Bug 807226 part 2. Change event handlers to store WebIDL callback functions. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 09 Nov 2012 08:00:25 -0800
changeset 121485 0ee3b6d2415aa35ae487dea95458a6cebeaade1d
parent 121484 e2727ec34a16a7a4d6b5afd4830eed0d17807e2e
child 121486 f64f70e0649c0b233e5973e675f65383406a496b
push id273
push userlsblakk@mozilla.com
push dateThu, 14 Feb 2013 23:19:38 +0000
treeherdermozilla-release@c5e807a3f8b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs807226
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 807226 part 2. Change event handlers to store WebIDL callback functions. r=smaug
content/events/src/nsEventListenerManager.cpp
content/events/src/nsEventListenerManager.h
content/events/src/nsEventListenerService.cpp
content/xbl/src/nsXBLPrototypeHandler.cpp
dom/base/nsIJSEventListener.h
dom/bindings/CallbackFunction.h
dom/src/events/nsJSEventListener.cpp
dom/src/events/nsJSEventListener.h
dom/webidl/EventHandler.webidl
--- a/content/events/src/nsEventListenerManager.cpp
+++ b/content/events/src/nsEventListenerManager.cpp
@@ -520,17 +520,17 @@ nsEventListenerManager::FindEventHandler
   return nullptr;
 }
 
 nsresult
 nsEventListenerManager::SetEventHandlerInternal(nsIScriptContext *aContext,
                                                 JSContext* aCx,
                                                 JSObject* aScopeObject,
                                                 nsIAtom* aName,
-                                                JSObject *aHandler,
+                                                const nsEventHandler& aHandler,
                                                 bool aPermitUntrustedEvents,
                                                 nsListenerStruct **aListenerStruct)
 {
   NS_ASSERTION(aContext || aCx, "Must have one or the other!");
 
   nsresult rv = NS_OK;
   uint32_t eventType = nsContentUtils::GetEventId(aName);
   nsListenerStruct* ls = FindEventHandler(eventType, aName);
@@ -546,18 +546,19 @@ nsEventListenerManager::SetEventHandlerI
       nsCOMPtr<nsIJSEventListener> scriptListener;
       rv = NS_NewJSEventListener(aContext, aScopeObject, mTarget, aName,
                                  aHandler, getter_AddRefs(scriptListener));
       listener = scriptListener.forget();
     } else {
       // If we don't have a script context, we're setting an event handler from
       // a component or other odd scope.  Ask XPConnect if it can make us an
       // nsIDOMEventListener.
+      MOZ_ASSERT(aHandler.HasEventHandler());
       rv = nsContentUtils::XPConnect()->WrapJS(aCx,
-                                               aHandler,
+                                               aHandler.Ptr()->Callable(),
                                                NS_GET_IID(nsIDOMEventListener),
                                                getter_AddRefs(listener));
     }
 
     if (NS_SUCCEEDED(rv)) {
       AddEventListener(listener, eventType, aName, flags, true);
 
       ls = FindEventHandler(eventType, aName);
@@ -569,30 +570,31 @@ nsEventListenerManager::SetEventHandlerI
     if ((!aContext && scriptListener) ||
         (aContext && !scriptListener)) {
       return NS_ERROR_FAILURE;
     }
 
     if (scriptListener) {
       scriptListener->SetHandler(aHandler);
     } else {
+      MOZ_ASSERT(aHandler.HasEventHandler());
       nsCOMPtr<nsIDOMEventListener> listener;
       rv = nsContentUtils::XPConnect()->WrapJS(aCx,
-                                               aHandler,
+                                               aHandler.Ptr()->Callable(),
                                                NS_GET_IID(nsIDOMEventListener),
                                                getter_AddRefs(listener));
       if (NS_SUCCEEDED(rv)) {
         ls->mListener = listener.forget();
       }
     }
   }
 
   if (NS_SUCCEEDED(rv) && ls) {
     // Set flag to indicate possible need for compilation later
-    ls->mHandlerIsString = !aHandler;
+    ls->mHandlerIsString = !aHandler.HasEventHandler();
     if (aPermitUntrustedEvents) {
       ls->mFlags |= NS_PRIV_EVENT_UNTRUSTED_PERMITTED;
     }
 
     *aListenerStruct = ls;
   } else {
     *aListenerStruct = nullptr;
   }
@@ -703,17 +705,17 @@ nsEventListenerManager::SetEventHandler(
   }
 
   nsIScriptContext* context = global->GetScriptContext();
   NS_ENSURE_TRUE(context, NS_ERROR_FAILURE);
 
   JSObject* scope = global->GetGlobalJSObject();
 
   nsListenerStruct *ls;
-  rv = SetEventHandlerInternal(context, nullptr, scope, aName, nullptr,
+  rv = SetEventHandlerInternal(context, nullptr, scope, aName, nsEventHandler(),
                                aPermitUntrustedEvents, &ls);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!aDeferCompilation) {
     return CompileEventHandlerInternal(ls, true, &aBody);
   }
 
   return NS_OK;
@@ -740,21 +742,24 @@ nsEventListenerManager::CompileEventHand
   NS_PRECONDITION(aListenerStruct->GetJSListener(),
                   "Why do we not have a JS listener?");
   NS_PRECONDITION(aListenerStruct->mHandlerIsString,
                   "Why are we compiling a non-string JS listener?");
 
   nsresult result = NS_OK;
 
   nsIJSEventListener *listener = aListenerStruct->GetJSListener();
-  NS_ASSERTION(!listener->GetHandler(), "What is there to compile?");
+  NS_ASSERTION(!listener->GetHandler().HasEventHandler(),
+               "What is there to compile?");
 
   nsIScriptContext *context = listener->GetEventContext();
   nsScriptObjectHolder<JSObject> handler(context);
 
+  nsCOMPtr<nsPIDOMWindow> win; // Will end up non-null if mTarget is a window
+
   if (aListenerStruct->mHandlerIsString) {
     // OK, we didn't find an existing compiled event handler.  Flag us
     // as not a string so we don't keep trying to compile strings
     // which can't be compiled
     aListenerStruct->mHandlerIsString = false;
 
     // mTarget may not be an nsIContent if it's a window and we're
     // getting an inline event listener forwarded from <html:body> or
@@ -794,17 +799,17 @@ nsEventListenerManager::CompileEventHand
     }
 
     uint32_t lineNo = 0;
     nsAutoCString url (NS_LITERAL_CSTRING("-moz-evil:lying-event-listener"));
     nsCOMPtr<nsIDocument> doc;
     if (content) {
       doc = content->OwnerDoc();
     } else {
-      nsCOMPtr<nsPIDOMWindow> win = do_QueryInterface(mTarget);
+      win = do_QueryInterface(mTarget);
       if (win) {
         doc = do_QueryInterface(win->GetExtantDocument());
       }
     }
 
     if (doc) {
       nsIURI *uri = doc->GetDocumentURI();
       if (uri) {
@@ -844,17 +849,56 @@ nsEventListenerManager::CompileEventHand
     NS_ENSURE_SUCCESS(result, result);
   }
 
   if (handler) {
     // Bind it
     nsScriptObjectHolder<JSObject> boundHandler(context);
     context->BindCompiledEventHandler(mTarget, listener->GetEventScope(),
                                       handler.get(), boundHandler);
-    listener->SetHandler(boundHandler.get());
+    if (listener->EventName() == nsGkAtoms::onerror && win) {
+      bool ok;
+      JSAutoRequest ar(context->GetNativeContext());
+      nsRefPtr<OnErrorEventHandlerNonNull> handlerCallback =
+        new OnErrorEventHandlerNonNull(context->GetNativeContext(),
+                                       listener->GetEventScope(),
+                                       boundHandler.get(), &ok);
+      if (!ok) {
+        // JS_WrapObject failed, which means OOM allocating the JSObject.
+        return NS_ERROR_OUT_OF_MEMORY;
+      }
+      listener->SetHandler(handlerCallback);
+    } else if (listener->EventName() == nsGkAtoms::onbeforeunload) {
+      // XXXbz Should we really do the special beforeunload handler on
+      // non-Window objects?  Per spec, we shouldn't even be compiling the
+      // beforeunload content attribute on random elements!  See bug 807226.
+      bool ok;
+      JSAutoRequest ar(context->GetNativeContext());
+      nsRefPtr<BeforeUnloadEventHandlerNonNull> handlerCallback =
+        new BeforeUnloadEventHandlerNonNull(context->GetNativeContext(),
+                                            listener->GetEventScope(),
+                                            boundHandler.get(), &ok);
+      if (!ok) {
+        // JS_WrapObject failed, which means OOM allocating the JSObject.
+        return NS_ERROR_OUT_OF_MEMORY;
+      }
+      listener->SetHandler(handlerCallback);
+    } else {
+      bool ok;
+      JSAutoRequest ar(context->GetNativeContext());
+      nsRefPtr<EventHandlerNonNull> handlerCallback =
+        new EventHandlerNonNull(context->GetNativeContext(),
+                                listener->GetEventScope(),
+                                boundHandler.get(), &ok);
+      if (!ok) {
+        // JS_WrapObject failed, which means OOM allocating the JSObject.
+        return NS_ERROR_OUT_OF_MEMORY;
+      }
+      listener->SetHandler(handlerCallback);
+    }
   }
 
   return result;
 }
 
 nsresult
 nsEventListenerManager::HandleEventSubType(nsListenerStruct* aListenerStruct,
                                            nsIDOMEventListener* aListener,
@@ -1148,38 +1192,59 @@ nsEventListenerManager::HasUnloadListene
 
 nsresult
 nsEventListenerManager::SetEventHandlerToJsval(nsIAtom* aEventName,
                                                JSContext* cx,
                                                JSObject* aScope,
                                                const jsval& v,
                                                bool aExpectScriptContext)
 {
-  JSObject *handler;
+  JSObject *callable;
   if (JSVAL_IS_PRIMITIVE(v) ||
-      !JS_ObjectIsCallable(cx, handler = JSVAL_TO_OBJECT(v))) {
+      !JS_ObjectIsCallable(cx, callable = JSVAL_TO_OBJECT(v))) {
     RemoveEventHandler(aEventName);
     return NS_OK;
   }
 
-  // Now ensure that we're working in the compartment of aScope from now on.
-  JSAutoCompartment ac(cx, aScope);
-
-  // Rewrap the handler into the new compartment, if needed.
-  jsval tempVal = v;
-  if (!JS_WrapValue(cx, &tempVal)) {
-    return NS_ERROR_UNEXPECTED;
+  nsEventHandler handler;
+  nsCOMPtr<nsPIDOMWindow> win = do_QueryInterface(mTarget);
+  if (aEventName == nsGkAtoms::onerror && win) {
+    bool ok;
+    nsRefPtr<OnErrorEventHandlerNonNull> handlerCallback =
+      new OnErrorEventHandlerNonNull(cx, aScope, callable, &ok);
+    if (!ok) {
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+    handler.SetHandler(handlerCallback);
+  } else if (aEventName == nsGkAtoms::onbeforeunload) {
+    MOZ_ASSERT(win,
+               "Should not have onbeforeunload handlers on non-Window objects");
+    bool ok;
+    nsRefPtr<BeforeUnloadEventHandlerNonNull> handlerCallback =
+      new BeforeUnloadEventHandlerNonNull(cx, aScope, callable, &ok);
+    if (!ok) {
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+    handler.SetHandler(handlerCallback);
+  } else {
+    bool ok;
+    nsRefPtr<EventHandlerNonNull> handlerCallback =
+      new EventHandlerNonNull(cx, aScope, callable, &ok);
+    if (!ok) {
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+    handler.SetHandler(handlerCallback);
   }
-  handler = &tempVal.toObject();
 
   // We might not have a script context, e.g. if we're setting a listener
   // on a dead Window.
   nsIScriptContext *context = nsJSUtils::GetStaticScriptContext(aScope);
   NS_ENSURE_TRUE(context || !aExpectScriptContext, NS_ERROR_FAILURE);
 
+  JSAutoCompartment ac(cx, aScope);
   JSObject *scope = ::JS_GetGlobalForObject(cx, aScope);
   // Untrusted events are always permitted for non-chrome script
   // handlers.
   nsListenerStruct *ignored;
   return SetEventHandlerInternal(context, cx, scope, aEventName, handler,
                                  !nsContentUtils::IsCallerChrome(), &ignored);
 }
 
@@ -1196,17 +1261,22 @@ nsEventListenerManager::GetEventHandler(
   }
 
   nsIJSEventListener *listener = ls->GetJSListener();
     
   if (ls->mHandlerIsString) {
     CompileEventHandlerInternal(ls, true, nullptr);
   }
 
-  *vp = OBJECT_TO_JSVAL(listener->GetHandler());
+  const nsEventHandler& handler = listener->GetHandler();
+  if (handler.HasEventHandler()) {
+    *vp = OBJECT_TO_JSVAL(handler.Ptr()->Callable());
+  } else {
+    *vp = JS::NullValue();
+  }
 }
 
 size_t
 nsEventListenerManager::SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf)
   const
 {
   size_t n = aMallocSizeOf(this);
   n += mListeners.SizeOfExcludingThis(aMallocSizeOf);
@@ -1223,17 +1293,19 @@ nsEventListenerManager::SizeOfIncludingT
 void
 nsEventListenerManager::MarkForCC()
 {
   uint32_t count = mListeners.Length();
   for (uint32_t i = 0; i < count; ++i) {
     const nsListenerStruct& ls = mListeners.ElementAt(i);
     nsIJSEventListener* jsl = ls.GetJSListener();
     if (jsl) {
-      xpc_UnmarkGrayObject(jsl->GetHandler());
+      if (jsl->GetHandler().HasEventHandler()) {
+        xpc_UnmarkGrayObject(jsl->GetHandler().Ptr()->Callable());
+      }
       xpc_UnmarkGrayObject(jsl->GetEventScope());
     } else if (ls.mListenerType == eWrappedJSListener) {
       xpc_TryUnmarkWrappedGrayObject(ls.mListener);
     }
   }
   if (mRefCnt.IsPurple()) {
     mRefCnt.RemovePurple();
   }
--- a/content/events/src/nsEventListenerManager.h
+++ b/content/events/src/nsEventListenerManager.h
@@ -256,26 +256,26 @@ protected:
                                        const nsAString* aBody);
 
   /**
    * Find the nsListenerStruct for the "inline" event listener for aTypeAtom.
    */
   nsListenerStruct* FindEventHandler(uint32_t aEventType, nsIAtom* aTypeAtom);
 
   /**
-   * Set the "inline" event listener for aName to aHandler.  aHandler
-   * may be null to indicate that we should lazily get and compile the
-   * string for this listener.  The nsListenerStruct that results, if
-   * any, is returned in aListenerStruct.
+   * Set the "inline" event listener for aName to aHandler.  aHandler may be
+   * have no actual handler set to indicate that we should lazily get and
+   * compile the string for this listener.  The nsListenerStruct that results,
+   * if any, is returned in aListenerStruct.
    */
   nsresult SetEventHandlerInternal(nsIScriptContext *aContext,
                                    JSContext* aCx,
                                    JSObject* aScopeGlobal,
                                    nsIAtom* aName,
-                                   JSObject *aHandler,
+                                   const nsEventHandler& aHandler,
                                    bool aPermitUntrustedEvents,
                                    nsListenerStruct **aListenerStruct);
 
   bool IsDeviceType(uint32_t aType);
   void EnableDevice(uint32_t aType);
   void DisableDevice(uint32_t aType);
 
 public:
--- a/content/events/src/nsEventListenerService.cpp
+++ b/content/events/src/nsEventListenerService.cpp
@@ -78,17 +78,17 @@ nsEventListenerInfo::GetJSVal(JSContext*
     }
     aAc.construct(aCx, object);
     *aJSVal = OBJECT_TO_JSVAL(object);
     return true;
   }
 
   nsCOMPtr<nsIJSEventListener> jsl = do_QueryInterface(mListener);
   if (jsl) {
-    JSObject *handler = jsl->GetHandler();
+    JSObject *handler = jsl->GetHandler().Ptr()->Callable();
     if (handler) {
       aAc.construct(aCx, handler);
       *aJSVal = OBJECT_TO_JSVAL(handler);
       return true;
     }
   }
   return false;
 }
--- a/content/xbl/src/nsXBLPrototypeHandler.cpp
+++ b/content/xbl/src/nsXBLPrototypeHandler.cpp
@@ -40,18 +40,20 @@
 #include "nsIDOMScriptObjectFactory.h"
 #include "nsDOMCID.h"
 #include "nsUnicharUtils.h"
 #include "nsCRT.h"
 #include "nsXBLEventHandler.h"
 #include "nsXBLSerialize.h"
 #include "nsEventDispatcher.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/dom/EventHandlerBinding.h"
 
 using namespace mozilla;
+using namespace mozilla::dom;
 
 static NS_DEFINE_CID(kDOMScriptObjectFactoryCID,
                      NS_DOM_SCRIPT_OBJECT_FACTORY_CID);
 
 uint32_t nsXBLPrototypeHandler::gRefCnt = 0;
 
 int32_t nsXBLPrototypeHandler::kMenuAccessKey = -1;
 int32_t nsXBLPrototypeHandler::kAccelKey = -1;
@@ -289,21 +291,32 @@ nsXBLPrototypeHandler::ExecuteHandler(ns
 
   // Bind it to the bound element
   JSObject* scope = boundGlobal->GetGlobalJSObject();
   nsScriptObjectHolder<JSObject> boundHandler(boundContext);
   rv = boundContext->BindCompiledEventHandler(scriptTarget, scope,
                                               handler.get(), boundHandler);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  bool ok;
+  JSAutoRequest ar(boundContext->GetNativeContext());
+  nsRefPtr<EventHandlerNonNull> handlerCallback =
+    new EventHandlerNonNull(boundContext->GetNativeContext(),
+                            scope, boundHandler.get(), &ok);
+  if (!ok) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  nsEventHandler eventHandler(handlerCallback);
+
   // Execute it.
   nsCOMPtr<nsIJSEventListener> eventListener;
   rv = NS_NewJSEventListener(boundContext, scope,
                              scriptTarget, onEventAtom,
-                             boundHandler.get(),
+                             eventHandler,
                              getter_AddRefs(eventListener));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Handle the event.
   eventListener->HandleEvent(aEvent);
   eventListener->Disconnect();
   return NS_OK;
 }
--- a/dom/base/nsIJSEventListener.h
+++ b/dom/base/nsIJSEventListener.h
@@ -5,38 +5,177 @@
 
 #ifndef nsIJSEventListener_h__
 #define nsIJSEventListener_h__
 
 #include "nsIScriptContext.h"
 #include "jsapi.h"
 #include "xpcpublic.h"
 #include "nsIDOMEventListener.h"
-
-class nsIAtom;
+#include "nsIAtom.h"
+#include "mozilla/dom/EventHandlerBinding.h"
 
 #define NS_IJSEVENTLISTENER_IID \
 { 0x92f9212b, 0xa6aa, 0x4867, \
   { 0x93, 0x8a, 0x56, 0xbe, 0x17, 0x67, 0x4f, 0xd4 } }
 
+class nsEventHandler
+{
+public:
+  typedef mozilla::dom::EventHandlerNonNull EventHandlerNonNull;
+  typedef mozilla::dom::BeforeUnloadEventHandlerNonNull
+    BeforeUnloadEventHandlerNonNull;
+  typedef mozilla::dom::OnErrorEventHandlerNonNull OnErrorEventHandlerNonNull;
+  typedef mozilla::dom::CallbackFunction CallbackFunction;
+
+  enum HandlerType {
+    eUnset = 0,
+    eNormal = 0x1,
+    eOnError = 0x2,
+    eOnBeforeUnload = 0x3,
+    eTypeBits = 0x3
+  };
+
+  nsEventHandler() :
+    mBits(0)
+  {}
+
+  nsEventHandler(EventHandlerNonNull* aHandler)
+  {
+    Assign(aHandler, eNormal);
+  }
+
+  nsEventHandler(OnErrorEventHandlerNonNull* aHandler)
+  {
+    Assign(aHandler, eOnError);
+  }
+
+  nsEventHandler(BeforeUnloadEventHandlerNonNull* aHandler)
+  {
+    Assign(aHandler, eOnBeforeUnload);
+  }
+
+  nsEventHandler(const nsEventHandler& aOther)
+  {
+    if (aOther.HasEventHandler()) {
+      // Have to make sure we take our own ref
+      Assign(aOther.Ptr(), aOther.Type());
+    } else {
+      mBits = 0;
+    }
+  }
+
+  ~nsEventHandler()
+  {
+    ReleaseHandler();
+  }
+
+  HandlerType Type() const {
+    return HandlerType(mBits & eTypeBits);
+  }
+
+  bool HasEventHandler() const
+  {
+    return !!Ptr();
+  }
+
+  void SetHandler(const nsEventHandler& aHandler)
+  {
+    if (aHandler.HasEventHandler()) {
+      ReleaseHandler();
+      Assign(aHandler.Ptr(), aHandler.Type());
+    } else {
+      ForgetHandler();
+    }
+  }
+
+  EventHandlerNonNull* EventHandler() const
+  {
+    MOZ_ASSERT(Type() == eNormal && Ptr());
+    return reinterpret_cast<EventHandlerNonNull*>(Ptr());
+  }
+
+  void SetHandler(EventHandlerNonNull* aHandler)
+  {
+    ReleaseHandler();
+    Assign(aHandler, eNormal);
+  }
+
+  BeforeUnloadEventHandlerNonNull* BeforeUnloadEventHandler() const
+  {
+    MOZ_ASSERT(Type() == eOnBeforeUnload);
+    return reinterpret_cast<BeforeUnloadEventHandlerNonNull*>(Ptr());
+  }
+
+  void SetHandler(BeforeUnloadEventHandlerNonNull* aHandler)
+  {
+    ReleaseHandler();
+    Assign(aHandler, eOnBeforeUnload);
+  }
+
+  OnErrorEventHandlerNonNull* OnErrorEventHandler() const
+  {
+    MOZ_ASSERT(Type() == eOnError);
+    return reinterpret_cast<OnErrorEventHandlerNonNull*>(Ptr());
+  }
+
+  void SetHandler(OnErrorEventHandlerNonNull* aHandler)
+  {
+    ReleaseHandler();
+    Assign(aHandler, eOnError);
+  }
+
+  CallbackFunction* Ptr() const
+  {
+    // Have to cast eTypeBits so we don't have to worry about
+    // promotion issues after the bitflip.
+    return reinterpret_cast<CallbackFunction*>(mBits & ~uintptr_t(eTypeBits));
+  }
+
+  void ForgetHandler()
+  {
+    ReleaseHandler();
+    mBits = 0;
+  }
+
+private:
+  void operator=(const nsEventHandler&) MOZ_DELETE;
+
+  void ReleaseHandler()
+  {
+    nsISupports* ptr = Ptr();
+    NS_IF_RELEASE(ptr);
+  }
+
+  void Assign(nsISupports* aHandler, HandlerType aType) {
+    MOZ_ASSERT(aHandler, "Must have handler");
+    NS_ADDREF(aHandler);
+    mBits = uintptr_t(aHandler) | uintptr_t(aType);
+  }
+
+  uintptr_t mBits;
+};
+
 // Implemented by script event listeners. Used to retrieve the
 // script object corresponding to the event target and the handler itself.
 // (Note this interface is now used to store script objects for all
 // script languages, so is no longer JS specific)
 //
 // Note, mTarget is a raw pointer and the owner of the nsIJSEventListener object
 // is expected to call Disconnect()!
 class nsIJSEventListener : public nsIDOMEventListener
 {
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IJSEVENTLISTENER_IID)
 
   nsIJSEventListener(nsIScriptContext* aContext, JSObject* aScopeObject,
-                     nsISupports *aTarget, JSObject *aHandler)
-    : mContext(aContext), mScopeObject(aScopeObject), mHandler(aHandler)
+                     nsISupports *aTarget, nsIAtom* aType,
+                     const nsEventHandler& aHandler)
+  : mContext(aContext), mScopeObject(aScopeObject), mEventName(aType),
+    mHandler(aHandler)
   {
     nsCOMPtr<nsISupports> base = do_QueryInterface(aTarget);
     mTarget = base.get();
   }
 
   nsIScriptContext *GetEventContext() const
   {
     return mContext;
@@ -52,62 +191,79 @@ public:
     mTarget = nullptr;
   }
 
   JSObject* GetEventScope() const
   {
     return xpc_UnmarkGrayObject(mScopeObject);
   }
 
-  JSObject *GetHandler() const
+  const nsEventHandler& GetHandler() const
   {
-    return xpc_UnmarkGrayObject(mHandler);
+    return mHandler;
+  }
+
+  nsIAtom* EventName() const
+  {
+    return mEventName;
   }
 
-  // Set a handler for this event listener.  Must not be called if
-  // there is already a handler!  The handler must already be bound to
-  // the right target.
-  virtual void SetHandler(JSObject *aHandler) = 0;
+  // Set a handler for this event listener.  The handler must already
+  // be bound to the right target.
+  void SetHandler(const nsEventHandler& aHandler)
+  {
+    mHandler.SetHandler(aHandler);
+  }
+  void SetHandler(mozilla::dom::EventHandlerNonNull* aHandler)
+  {
+    mHandler.SetHandler(aHandler);
+  }
+  void SetHandler(mozilla::dom::BeforeUnloadEventHandlerNonNull* aHandler)
+  {
+    mHandler.SetHandler(aHandler);
+  }
+  void SetHandler(mozilla::dom::OnErrorEventHandlerNonNull* aHandler)
+  {
+    mHandler.SetHandler(aHandler);
+  }
 
-  // Among the sub-classes that inherit (directly or indirectly) from nsINode,
-  // measurement of the following members may be added later if DMD finds it is
-  // worthwhile:
-  // - nsIJSEventListener: mEventName
-  //
   virtual size_t SizeOfExcludingThis(nsMallocSizeOfFun aMallocSizeOf) const
   {
     return 0;
 
     // Measurement of the following members may be added later if DMD finds it
     // is worthwhile:
     // - mContext
     // - mTarget
     //
     // The following members are not measured:
-    // - mScopeObject, mHandler: because they're measured by the JS memory
+    // - mScopeObject: because they're measured by the JS memory
     //   reporters
+    // - mHandler: may be shared with others
+    // - mEventName: shared with others
   }
 
   virtual size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const
   {
     return aMallocSizeOf(this) + SizeOfExcludingThis(aMallocSizeOf);
   }
 
 protected:
   virtual ~nsIJSEventListener()
   {
     NS_ASSERTION(!mTarget, "Should have called Disconnect()!");
   }
   nsCOMPtr<nsIScriptContext> mContext;
   JSObject* mScopeObject;
   nsISupports* mTarget;
-  JSObject *mHandler;
+  nsCOMPtr<nsIAtom> mEventName;
+  nsEventHandler mHandler;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIJSEventListener, NS_IJSEVENTLISTENER_IID)
 
 /* factory function.  aHandler must already be bound to aTarget */
 nsresult NS_NewJSEventListener(nsIScriptContext *aContext,
                                JSObject* aScopeObject, nsISupports* aTarget,
-                               nsIAtom* aType, JSObject* aHandler,
+                               nsIAtom* aType, const nsEventHandler& aHandler,
                                nsIJSEventListener **aReturn);
 
 #endif // nsIJSEventListener_h__
--- a/dom/bindings/CallbackFunction.h
+++ b/dom/bindings/CallbackFunction.h
@@ -73,16 +73,22 @@ public:
   }
 
   JSObject* Callable() const
   {
     xpc_UnmarkGrayObject(mCallable);
     return mCallable;
   }
 
+  bool HasGrayCallable() const
+  {
+    // Play it safe in case this gets called after unlink.
+    return mCallable && xpc_IsGrayGCThing(mCallable);
+  }
+
 protected:
   void DropCallback()
   {
     if (mCallable) {
       NS_DROP_JS_OBJECTS(this, CallbackFunction);
       mCallable = nullptr;
     }
   }
--- a/dom/src/events/nsJSEventListener.cpp
+++ b/dom/src/events/nsJSEventListener.cpp
@@ -33,26 +33,27 @@ class EventListenerCounter
 public:
   ~EventListenerCounter() {
   }
 };
 
 static EventListenerCounter sEventListenerCounter;
 #endif
 
+using namespace mozilla::dom;
+
 /*
  * nsJSEventListener implementation
  */
 nsJSEventListener::nsJSEventListener(nsIScriptContext *aContext,
                                      JSObject* aScopeObject,
                                      nsISupports *aTarget,
                                      nsIAtom* aType,
-                                     JSObject *aHandler)
-  : nsIJSEventListener(aContext, aScopeObject, aTarget, aHandler),
-    mEventName(aType)
+                                     const nsEventHandler& aHandler)
+  : nsIJSEventListener(aContext, aScopeObject, aTarget, aType, aHandler)
 {
   // aScopeObject is the inner window's JS object, which we need to lock
   // until we are done with it.
   NS_ASSERTION(aScopeObject && aContext,
                "EventListener with no context or scope?");
   NS_HOLD_JS_OBJECTS(this, nsJSEventListener);
 }
 
@@ -65,34 +66,35 @@ nsJSEventListener::~nsJSEventListener()
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSEventListener)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSEventListener)
   if (tmp->mContext) {
     NS_DROP_JS_OBJECTS(tmp, nsJSEventListener);
     tmp->mScopeObject = nullptr;
     NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mContext)
   }
+  tmp->mHandler.ForgetHandler();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsJSEventListener)
   if (MOZ_UNLIKELY(cb.WantDebugInfo()) && tmp->mEventName) {
     nsAutoCString name;
     name.AppendLiteral("nsJSEventListener handlerName=");
     name.Append(
       NS_ConvertUTF16toUTF8(nsDependentAtomString(tmp->mEventName)).get());
     cb.DescribeRefCountedNode(tmp->mRefCnt.get(), name.get());
   } else {
     NS_IMPL_CYCLE_COLLECTION_DESCRIBE(nsJSEventListener, tmp->mRefCnt.get())
   }
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mContext)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(mHandler.Ptr())
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsJSEventListener)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mScopeObject)
-  NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mHandler)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_BEGIN(nsJSEventListener)
   if (tmp->IsBlackForCC()) {
     return true;
   }
   // If we have a target, it is the one which has tmp as onfoo handler.
   if (tmp->mTarget) {
@@ -126,29 +128,30 @@ NS_INTERFACE_MAP_END
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsJSEventListener)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsJSEventListener)
 
 bool
 nsJSEventListener::IsBlackForCC()
 {
   if (mContext &&
       (!mScopeObject || !xpc_IsGrayGCThing(mScopeObject)) &&
-      (!mHandler || !xpc_IsGrayGCThing(mHandler))) {
+      (!mHandler.HasEventHandler() ||
+       !mHandler.Ptr()->HasGrayCallable())) {
     nsIScriptGlobalObject* sgo =
       static_cast<nsJSContext*>(mContext.get())->GetCachedGlobalObject();
     return sgo && sgo->IsBlackForCC();
   }
   return false;
 }
 
 nsresult
 nsJSEventListener::HandleEvent(nsIDOMEvent* aEvent)
 {
   nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(mTarget);
-  if (!target || !mContext || !mHandler)
+  if (!target || !mContext || !mHandler.HasEventHandler())
     return NS_ERROR_FAILURE;
 
   nsresult rv;
   nsCOMPtr<nsIMutableArray> iargv;
 
   bool handledScriptError = false;
   if (mEventName == nsGkAtoms::onerror) {
     NS_ENSURE_TRUE(aEvent, NS_ERROR_UNEXPECTED);
@@ -203,19 +206,19 @@ nsJSEventListener::HandleEvent(nsIDOMEve
   nsCOMPtr<nsIJSContextStack> stack =
     do_GetService("@mozilla.org/js/xpc/ContextStack;1");
   NS_ASSERTION(stack && NS_SUCCEEDED(stack->Peek(&cx)) && cx &&
                GetScriptContextFromJSContext(cx) == mContext,
                "JSEventListener has wrong script context?");
 #endif
   nsCOMPtr<nsIVariant> vrv;
   xpc_UnmarkGrayObject(mScopeObject);
-  xpc_UnmarkGrayObject(mHandler);
-  rv = mContext->CallEventHandler(mTarget, mScopeObject, mHandler, iargv,
-                                  getter_AddRefs(vrv));
+  rv = mContext->CallEventHandler(mTarget, mScopeObject,
+                                  mHandler.Ptr()->Callable(),
+                                  iargv, getter_AddRefs(vrv));
 
   if (NS_SUCCEEDED(rv)) {
     uint16_t dataType = nsIDataType::VTYPE_VOID;
     if (vrv)
       vrv->GetDataType(&dataType);
 
     if (mEventName == nsGkAtoms::onbeforeunload) {
       nsCOMPtr<nsIDOMBeforeUnloadEvent> beforeUnload = do_QueryInterface(aEvent);
@@ -252,36 +255,25 @@ nsJSEventListener::HandleEvent(nsIDOMEve
         aEvent->PreventDefault();
       }
     }
   }
 
   return rv;
 }
 
-/* virtual */ void
-nsJSEventListener::SetHandler(JSObject *aHandler)
-{
-  // Technically we should drop the old mHandler and hold the new
-  // one... except for JS this is a no-op, and we're really not
-  // pretending very hard to support anything else.  And since we
-  // can't in fact only drop one script object (we'd have to drop
-  // mScope too, and then re-hold it), let's just not worry about it
-  // all.
-  mHandler = aHandler;
-}
-
 /*
  * Factory functions
  */
 
 nsresult
 NS_NewJSEventListener(nsIScriptContext* aContext, JSObject* aScopeObject,
                       nsISupports*aTarget, nsIAtom* aEventType,
-                      JSObject* aHandler, nsIJSEventListener** aReturn)
+                      const nsEventHandler& aHandler,
+                      nsIJSEventListener** aReturn)
 {
   NS_ENSURE_ARG(aEventType);
   nsJSEventListener* it =
     new nsJSEventListener(aContext, aScopeObject, aTarget, aEventType,
                           aHandler);
   NS_ADDREF(*aReturn = it);
 
   return NS_OK;
--- a/dom/src/events/nsJSEventListener.h
+++ b/dom/src/events/nsJSEventListener.h
@@ -16,34 +16,32 @@
 #include "nsCycleCollectionParticipant.h"
 
 // nsJSEventListener interface
 // misnamed - JS no longer has exclusive rights over this interface!
 class nsJSEventListener : public nsIJSEventListener
 {
 public:
   nsJSEventListener(nsIScriptContext* aContext, JSObject* aScopeObject,
-                    nsISupports* aTarget, nsIAtom* aType, JSObject* aHandler);
+                    nsISupports* aTarget, nsIAtom* aType,
+                    const nsEventHandler& aHandler);
   virtual ~nsJSEventListener();
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
 
   // nsIDOMEventListener interface
   NS_DECL_NSIDOMEVENTLISTENER
 
   // nsIJSEventListener
-  virtual void SetHandler(JSObject *aHandler);
 
   virtual size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const
   {
     return aMallocSizeOf(this) + SizeOfExcludingThis(aMallocSizeOf);
   }
 
   NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS(nsJSEventListener)
 
 protected:
   bool IsBlackForCC();
-
-  nsCOMPtr<nsIAtom> mEventName;
 };
 
 #endif //nsJSEventListener_h__
 
--- a/dom/webidl/EventHandler.webidl
+++ b/dom/webidl/EventHandler.webidl
@@ -10,10 +10,14 @@
  * Opera Software ASA. You are granted a license to use, reproduce
  * and create derivative works of this document.
  */
 [TreatNonCallableAsNull]
 callback EventHandlerNonNull = any (Event event);
 typedef EventHandlerNonNull? EventHandler;
 
 [TreatNonCallableAsNull]
-callback OnErrorEventHandlerNonNull = any ((Event or DOMString) event, optional DOMString source, optional unsigned long lineno, optional unsigned long column);
+callback BeforeUnloadEventHandlerNonNull = DOMString? (Event event);
+typedef BeforeUnloadEventHandlerNonNull? BeforeUnloadEventHandler;
+
+[TreatNonCallableAsNull]
+callback OnErrorEventHandlerNonNull = boolean ((Event or DOMString) event, optional DOMString source, optional unsigned long lineno, optional unsigned long column);
 typedef OnErrorEventHandlerNonNull? OnErrorEventHandler;