Bug 807226 part 7. Move creation of our event handlers out to the relevant API methods. r=smaug

Note that once we switch all these guys to WebIDL bindings they'll
automatically get the callback objects passed in from binding code.

content/base/src/nsINode.cpp

@@ -2018,10 +2018,13 @@ nsINode::SizeOfExcludingThis(nsMallocSizeOfFun aMallocSizeOf) const
   NS_IMETHODIMP nsINode::GetOn##name_(JSContext *cx, jsval *vp) {            \
     nsEventListenerManager *elm = GetListenerManager(false);                 \
     if (elm) {                                                               \
-      elm->GetEventHandler(nsGkAtoms::on##name_, vp);                        \
+      EventHandlerNonNull* h = elm->GetEventHandler(nsGkAtoms::on##name_);   \
-    } else {                                                                 \
+      if (h) {                                                               \
-      *vp = JSVAL_NULL;                                                      \
+        *vp = JS::ObjectValue(*h->Callable());                               \
+        return NS_OK;                                                        \
       }                                                                      \
+    }                                                                        \
+    *vp = JSVAL_NULL;                                                        \
     return NS_OK;                                                            \
   }                                                                          \
   NS_IMETHODIMP nsINode::SetOn##name_(JSContext *cx, const jsval &v) {       \
@@ -2035,7 +2038,17 @@ nsINode::SizeOfExcludingThis(nsMallocSizeOfFun aMallocSizeOf) const
       /* Just silently do nothing */                                         \
       return NS_OK;                                                          \
     }                                                                        \
-    return elm->SetEventHandlerToJsval(nsGkAtoms::on##name_, cx, obj, v);    \
+    nsRefPtr<EventHandlerNonNull> handler;                                   \
+    JSObject *callable;                                                      \
+    if (v.isObject() &&                                                      \
+        JS_ObjectIsCallable(cx, callable = &v.toObject())) {                 \
+      bool ok;                                                               \
+      handler = new EventHandlerNonNull(cx, obj, callable, &ok);             \
+      if (!ok) {                                                             \
+        return NS_ERROR_OUT_OF_MEMORY;                                       \
+      }                                                                      \
+    }                                                                        \
+    return elm->SetEventHandler(nsGkAtoms::on##name_, handler);              \
   }
 #define TOUCH_EVENT EVENT
 #define DOCUMENT_ONLY_EVENT EVENT

content/events/src/nsDOMEventTargetHelper.cpp

@@ -15,6 +15,9 @@
 #include "nsDOMEvent.h"
 #include "mozilla/Likely.h"
 
+using namespace mozilla;
+using namespace mozilla::dom;
+
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsDOMEventTargetHelper)
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsDOMEventTargetHelper)
@@ -231,14 +234,24 @@ nsDOMEventTargetHelper::SetEventHandler(nsIAtom* aType,
                                         JSContext* aCx,
                                         const JS::Value& aValue)
 {
-  nsEventListenerManager* elm = GetListenerManager(true);
-
   JSObject* obj = GetWrapper();
   if (!obj) {
     return NS_OK;
   }
 
-  return elm->SetEventHandlerToJsval(aType, aCx, obj, aValue);
+  nsRefPtr<EventHandlerNonNull> handler;
+  JSObject* callable;
+  if (aValue.isObject() &&
+      JS_ObjectIsCallable(aCx, callable = &aValue.toObject())) {
+    bool ok;
+    handler = new EventHandlerNonNull(aCx, obj, callable, &ok);
+    if (!ok) {
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+  }
+  ErrorResult rv;
+  SetEventHandler(aType, handler, rv);
+  return rv.ErrorCode();
 }
 
 void
@@ -246,11 +259,11 @@ nsDOMEventTargetHelper::GetEventHandler(nsIAtom* aType,
                                         JSContext* aCx,
                                         JS::Value* aValue)
 {
-  *aValue = JSVAL_NULL;
+  EventHandlerNonNull* handler = GetEventHandler(aType);
-
+  if (handler) {
-  nsEventListenerManager* elm = GetListenerManager(false);
+    *aValue = JS::ObjectValue(*handler->Callable());
-  if (elm) {
+  } else {
-    elm->GetEventHandler(aType, aValue);
+    *aValue = JS::NullValue();
   }
 }
 

content/events/src/nsDOMEventTargetHelper.h

@@ -92,9 +92,20 @@ public:
   nsresult SetEventHandler(nsIAtom* aType,
                            JSContext* aCx,
                            const JS::Value& aValue);
+  void SetEventHandler(nsIAtom* aType,
+                       mozilla::dom::EventHandlerNonNull* aHandler,
+                       mozilla::ErrorResult& rv)
+  {
+    rv = GetListenerManager(true)->SetEventHandler(aType, aHandler);
+  }
   void GetEventHandler(nsIAtom* aType,
                        JSContext* aCx,
                        JS::Value* aValue);
+  mozilla::dom::EventHandlerNonNull* GetEventHandler(nsIAtom* aType)
+  {
+    nsEventListenerManager* elm = GetListenerManager(false);
+    return elm ? elm->GetEventHandler(aType) : nullptr;
+  }
 
   nsresult CheckInnerWindowCorrectness()
   {
@@ -162,9 +173,7 @@ private:
                             mozilla::dom::EventHandlerNonNull* aCallback, \
                             ErrorResult& aRv)                             \
   {                                                                       \
-    JSObject* callback = aCallback ? aCallback->Callable() : nullptr;     \
+    SetEventHandler(nsGkAtoms::on##_event, aCallback, aRv);               \
-    aRv = SetEventHandler(nsGkAtoms::on##_event, aCx,                     \
-                          JS::ObjectOrNullValue(callback));               \
   }
 
 /* Use this macro to declare functions that forward the behavior of this

content/events/src/nsEventListenerManager.cpp

@@ -1161,65 +1161,62 @@ nsEventListenerManager::HasUnloadListeners()
 }
 
 nsresult
-nsEventListenerManager::SetEventHandlerToJsval(nsIAtom* aEventName,
+nsEventListenerManager::SetEventHandler(nsIAtom* aEventName,
-                                               JSContext* cx,
+                                        EventHandlerNonNull* aHandler)
-                                               JSObject* aScope,
-                                               const jsval& v)
 {
-  JSObject *callable;
+  if (!aHandler) {
-  if (JSVAL_IS_PRIMITIVE(v) ||
-      !JS_ObjectIsCallable(cx, callable = JSVAL_TO_OBJECT(v))) {
     RemoveEventHandler(aEventName);
     return NS_OK;
   }
 
-  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);
-  }
-
   // Untrusted events are always permitted for non-chrome script
   // handlers.
   nsListenerStruct *ignored;
-  return SetEventHandlerInternal(nullptr, nullptr, aEventName, handler,
+  return SetEventHandlerInternal(nullptr, nullptr, aEventName,
+                                 nsEventHandler(aHandler),
                                  !nsContentUtils::IsCallerChrome(), &ignored);
 }
 
-void
+nsresult
-nsEventListenerManager::GetEventHandler(nsIAtom *aEventName, jsval *vp)
+nsEventListenerManager::SetEventHandler(OnErrorEventHandlerNonNull* aHandler)
+{
+  if (!aHandler) {
+    RemoveEventHandler(nsGkAtoms::onerror);
+    return NS_OK;
+  }
+
+  // Untrusted events are always permitted for non-chrome script
+  // handlers.
+  nsListenerStruct *ignored;
+  return SetEventHandlerInternal(nullptr, nullptr, nsGkAtoms::onerror,
+                                 nsEventHandler(aHandler),
+                                 !nsContentUtils::IsCallerChrome(), &ignored);
+}
+
+nsresult
+nsEventListenerManager::SetEventHandler(BeforeUnloadEventHandlerNonNull* aHandler)
+{
+  if (!aHandler) {
+    RemoveEventHandler(nsGkAtoms::onbeforeunload);
+    return NS_OK;
+  }
+
+  // Untrusted events are always permitted for non-chrome script
+  // handlers.
+  nsListenerStruct *ignored;
+  return SetEventHandlerInternal(nullptr, nullptr, nsGkAtoms::onbeforeunload,
+                                 nsEventHandler(aHandler),
+                                 !nsContentUtils::IsCallerChrome(), &ignored);
+}
+
+const nsEventHandler*
+nsEventListenerManager::GetEventHandlerInternal(nsIAtom *aEventName)
 {
   uint32_t eventType = nsContentUtils::GetEventId(aEventName);
   nsListenerStruct* ls = FindEventHandler(eventType, aEventName);
 
-  *vp = JSVAL_NULL;
-
   if (!ls) {
-    return;
+    return nullptr;
   }
 
   nsIJSEventListener *listener = ls->GetJSListener();
@@ -1230,10 +1227,10 @@ nsEventListenerManager::GetEventHandler(nsIAtom *aEventName, jsval *vp)
 
   const nsEventHandler& handler = listener->GetHandler();
   if (handler.HasEventHandler()) {
-    *vp = OBJECT_TO_JSVAL(handler.Ptr()->Callable());
+    return &handler;
-  } else {
-    *vp = JS::NullValue();
   }
+
+  return nullptr;
 }
 
 size_t

content/events/src/nsEventListenerManager.h

@@ -281,21 +281,47 @@ protected:
 
 public:
   /**
-   * Set the "inline" event listener for aEventName to |v|.  This
+   * Set the "inline" event listener for aEventName to aHandler.  If
-   * might actually remove the event listener, depending on the value
+   * aHandler is null, this will actually remove the event listener
-   * of |v|.  Note that on entry to this function cx and aScope might
-   * not be in the same compartment, though cx and v are guaranteed to
-   * be in the same compartment.
    */
-  nsresult SetEventHandlerToJsval(nsIAtom* aEventName, JSContext* cx,
+  nsresult SetEventHandler(nsIAtom* aEventName,
-                                  JSObject* aScope, const jsval& v);
+                           mozilla::dom::EventHandlerNonNull* aHandler);
+  nsresult SetEventHandler(mozilla::dom::OnErrorEventHandlerNonNull* aHandler);
+  nsresult SetEventHandler(mozilla::dom::BeforeUnloadEventHandlerNonNull* aHandler);
+
   /**
    * Get the value of the "inline" event listener for aEventName.
    * This may cause lazy compilation if the listener is uncompiled.
+   *
+   * Note: It's the caller's responsibility to make sure to call the right one
+   * of these methods.  In particular, "onerror" events use
+   * OnErrorEventHandlerNonNull for some event targets and EventHandlerNonNull
+   * for others.
    */
-  void GetEventHandler(nsIAtom *aEventName, jsval *vp);
+  mozilla::dom::EventHandlerNonNull* GetEventHandler(nsIAtom *aEventName)
+  {
+    const nsEventHandler* handler = GetEventHandlerInternal(aEventName);
+    return handler ? handler->EventHandler() : nullptr;
+  }
+  mozilla::dom::OnErrorEventHandlerNonNull* GetOnErrorEventHandler()
+  {
+    const nsEventHandler* handler = GetEventHandlerInternal(nsGkAtoms::onerror);
+    return handler ? handler->OnErrorEventHandler() : nullptr;
+  }
+  mozilla::dom::BeforeUnloadEventHandlerNonNull* GetOnBeforeUnloadEventHandler()
+  {
+    const nsEventHandler* handler =
+      GetEventHandlerInternal(nsGkAtoms::onbeforeunload);
+    return handler ? handler->BeforeUnloadEventHandler() : nullptr;
+  }
 
 protected:
+  /**
+   * Helper method for implementing the various Get*EventHandler above.  Will
+   * return null if we don't have an event handler for this event name.
+   */
+  const nsEventHandler* GetEventHandlerInternal(nsIAtom* aEventName);
+
   void AddEventListener(nsIDOMEventListener *aListener, 
                         uint32_t aType,
                         nsIAtom* aTypeAtom,

dom/base/nsGlobalWindow.cpp

@@ -11192,10 +11192,13 @@ nsGlobalWindow::DisableNetworkEvent(uint32_t aType)
                                              jsval *vp) {                    \
     nsEventListenerManager *elm = GetListenerManager(false);                 \
     if (elm) {                                                               \
-      elm->GetEventHandler(nsGkAtoms::on##name_, vp);                        \
+      EventHandlerNonNull* h = elm->GetEventHandler(nsGkAtoms::on##name_);   \
-    } else {                                                                 \
+      if (h) {                                                               \
-      *vp = JSVAL_NULL;                                                      \
+        *vp = JS::ObjectValue(*h->Callable());                               \
+        return NS_OK;                                                        \
       }                                                                      \
+    }                                                                        \
+    *vp = JSVAL_NULL;                                                        \
     return NS_OK;                                                            \
   }                                                                          \
   NS_IMETHODIMP nsGlobalWindow::SetOn##name_(JSContext *cx,                  \
@@ -11209,7 +11212,92 @@ nsGlobalWindow::DisableNetworkEvent(uint32_t aType)
     if (!obj) {                                                              \
       return NS_ERROR_UNEXPECTED;                                            \
     }                                                                        \
-    return elm->SetEventHandlerToJsval(nsGkAtoms::on##name_, cx, obj, v);    \
+    nsRefPtr<EventHandlerNonNull> handler;                                   \
+    JSObject *callable;                                                      \
+    if (v.isObject() &&                                                      \
+        JS_ObjectIsCallable(cx, callable = &v.toObject())) {                 \
+      bool ok;                                                               \
+      handler = new EventHandlerNonNull(cx, obj, callable, &ok);             \
+      if (!ok) {                                                             \
+        return NS_ERROR_OUT_OF_MEMORY;                                       \
+      }                                                                      \
+    }                                                                        \
+    return elm->SetEventHandler(nsGkAtoms::on##name_, handler);              \
+  }
+#define ERROR_EVENT(name_, id_, type_, struct_)                              \
+  NS_IMETHODIMP nsGlobalWindow::GetOn##name_(JSContext *cx,                  \
+                                             jsval *vp) {                    \
+    nsEventListenerManager *elm = GetListenerManager(false);                 \
+    if (elm) {                                                               \
+      OnErrorEventHandlerNonNull* h = elm->GetOnErrorEventHandler();         \
+      if (h) {                                                               \
+        *vp = JS::ObjectValue(*h->Callable());                               \
+        return NS_OK;                                                        \
+      }                                                                      \
+    }                                                                        \
+    *vp = JSVAL_NULL;                                                        \
+    return NS_OK;                                                            \
+  }                                                                          \
+  NS_IMETHODIMP nsGlobalWindow::SetOn##name_(JSContext *cx,                  \
+                                             const jsval &v) {               \
+    nsEventListenerManager *elm = GetListenerManager(true);                  \
+    if (!elm) {                                                              \
+      return NS_ERROR_OUT_OF_MEMORY;                                         \
+    }                                                                        \
+                                                                             \
+    JSObject *obj = mJSObject;                                               \
+    if (!obj) {                                                              \
+      return NS_ERROR_UNEXPECTED;                                            \
+    }                                                                        \
+    nsRefPtr<OnErrorEventHandlerNonNull> handler;                            \
+    JSObject *callable;                                                      \
+    if (v.isObject() &&                                                      \
+        JS_ObjectIsCallable(cx, callable = &v.toObject())) {                 \
+      bool ok;                                                               \
+      handler = new OnErrorEventHandlerNonNull(cx, obj, callable, &ok);      \
+      if (!ok) {                                                             \
+        return NS_ERROR_OUT_OF_MEMORY;                                       \
+      }                                                                      \
+    }                                                                        \
+    return elm->SetEventHandler(handler);                                    \
+  }
+#define BEFOREUNLOAD_EVENT(name_, id_, type_, struct_)                       \
+  NS_IMETHODIMP nsGlobalWindow::GetOn##name_(JSContext *cx,                  \
+                                             jsval *vp) {                    \
+    nsEventListenerManager *elm = GetListenerManager(false);                 \
+    if (elm) {                                                               \
+      BeforeUnloadEventHandlerNonNull* h =                                   \
+        elm->GetOnBeforeUnloadEventHandler();                                \
+      if (h) {                                                               \
+        *vp = JS::ObjectValue(*h->Callable());                               \
+        return NS_OK;                                                        \
+      }                                                                      \
+    }                                                                        \
+    *vp = JSVAL_NULL;                                                        \
+    return NS_OK;                                                            \
+  }                                                                          \
+  NS_IMETHODIMP nsGlobalWindow::SetOn##name_(JSContext *cx,                  \
+                                             const jsval &v) {               \
+    nsEventListenerManager *elm = GetListenerManager(true);                  \
+    if (!elm) {                                                              \
+      return NS_ERROR_OUT_OF_MEMORY;                                         \
+    }                                                                        \
+                                                                             \
+    JSObject *obj = mJSObject;                                               \
+    if (!obj) {                                                              \
+      return NS_ERROR_UNEXPECTED;                                            \
+    }                                                                        \
+    nsRefPtr<BeforeUnloadEventHandlerNonNull> handler;                       \
+    JSObject *callable;                                                      \
+    if (v.isObject() &&                                                      \
+        JS_ObjectIsCallable(cx, callable = &v.toObject())) {                 \
+      bool ok;                                                               \
+      handler = new BeforeUnloadEventHandlerNonNull(cx, obj, callable, &ok); \
+      if (!ok) {                                                             \
+        return NS_ERROR_OUT_OF_MEMORY;                                       \
+      }                                                                      \
+    }                                                                        \
+    return elm->SetEventHandler(handler);                                    \
   }
 #define WINDOW_ONLY_EVENT EVENT
 #define TOUCH_EVENT EVENT