Bug 807226 part 4. Allow event handlers to have a null nsIScriptContext if they won't have to compile from a string. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 09 Nov 2012 08:00:25 -0800
changeset 121487 f244d412e48500932c346c25de78ea2814cb1802
parent 121486 f64f70e0649c0b233e5973e675f65383406a496b
child 121488 741e28cf55ee20db857424ef8320921b948f74dc
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 4. Allow event handlers to have a null nsIScriptContext if they won't have to compile from a string. r=smaug
content/base/src/nsINode.cpp
content/events/src/nsDOMEventTargetHelper.cpp
content/events/src/nsEventListenerManager.cpp
content/events/src/nsEventListenerManager.h
content/xbl/src/nsXBLPrototypeHandler.cpp
dom/base/nsGlobalWindow.cpp
dom/base/nsIJSEventListener.h
dom/src/events/nsJSEventListener.cpp
--- a/content/base/src/nsINode.cpp
+++ b/content/base/src/nsINode.cpp
@@ -2030,18 +2030,17 @@ nsINode::SizeOfExcludingThis(nsMallocSiz
       return NS_ERROR_OUT_OF_MEMORY;                                         \
     }                                                                        \
                                                                              \
     JSObject *obj = GetWrapper();                                            \
     if (!obj) {                                                              \
       /* Just silently do nothing */                                         \
       return NS_OK;                                                          \
     }                                                                        \
-    return elm->SetEventHandlerToJsval(nsGkAtoms::on##name_, cx, obj, v,     \
-                                       true);                                \
+    return elm->SetEventHandlerToJsval(nsGkAtoms::on##name_, cx, obj, v);    \
 }
 #define TOUCH_EVENT EVENT
 #define DOCUMENT_ONLY_EVENT EVENT
 #include "nsEventNameList.h"
 #undef DOCUMENT_ONLY_EVENT
 #undef TOUCH_EVENT
 #undef EVENT
 
--- a/content/events/src/nsDOMEventTargetHelper.cpp
+++ b/content/events/src/nsDOMEventTargetHelper.cpp
@@ -233,18 +233,17 @@ nsDOMEventTargetHelper::SetEventHandler(
 {
   nsEventListenerManager* elm = GetListenerManager(true);
 
   JSObject* obj = GetWrapper();
   if (!obj) {
     return NS_OK;
   }
   
-  return elm->SetEventHandlerToJsval(aType, aCx, obj, aValue,
-                                     HasOrHasHadOwner());
+  return elm->SetEventHandlerToJsval(aType, aCx, obj, aValue);
 }
 
 void
 nsDOMEventTargetHelper::GetEventHandler(nsIAtom* aType,
                                         JSContext* aCx,
                                         JS::Value* aValue)
 {
   *aValue = JSVAL_NULL;
--- a/content/events/src/nsEventListenerManager.cpp
+++ b/content/events/src/nsEventListenerManager.cpp
@@ -517,79 +517,49 @@ nsEventListenerManager::FindEventHandler
       return ls;
     }
   }
   return nullptr;
 }
 
 nsresult
 nsEventListenerManager::SetEventHandlerInternal(nsIScriptContext *aContext,
-                                                JSContext* aCx,
                                                 JSObject* aScopeObject,
                                                 nsIAtom* aName,
                                                 const nsEventHandler& aHandler,
                                                 bool aPermitUntrustedEvents,
                                                 nsListenerStruct **aListenerStruct)
 {
-  NS_ASSERTION(aContext || aCx, "Must have one or the other!");
+  NS_ASSERTION(aContext || aHandler.HasEventHandler(),
+               "Must have one or the other!");
 
   nsresult rv = NS_OK;
   uint32_t eventType = nsContentUtils::GetEventId(aName);
   nsListenerStruct* ls = FindEventHandler(eventType, aName);
 
   if (!ls) {
     // If we didn't find a script listener or no listeners existed
     // create and add a new one.
-    const uint32_t flags = NS_EVENT_FLAG_BUBBLE |
-                           (aContext ? NS_PRIV_EVENT_FLAG_SCRIPT : 0);
-    nsCOMPtr<nsIDOMEventListener> listener;
+    const uint32_t flags = NS_EVENT_FLAG_BUBBLE | NS_PRIV_EVENT_FLAG_SCRIPT;
 
-    if (aContext) {
-      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.Ptr()->Callable(),
-                                               NS_GET_IID(nsIDOMEventListener),
-                                               getter_AddRefs(listener));
-    }
+    nsCOMPtr<nsIJSEventListener> scriptListener;
+    rv = NS_NewJSEventListener(aContext, aScopeObject, mTarget, aName,
+                               aHandler, getter_AddRefs(scriptListener));
 
     if (NS_SUCCEEDED(rv)) {
-      AddEventListener(listener, eventType, aName, flags, true);
+      AddEventListener(scriptListener, eventType, aName, flags, true);
 
       ls = FindEventHandler(eventType, aName);
     }
   } else {
-    // Don't mix 'real' JS event handlers and 'fake' JS event handlers.
     nsIJSEventListener* scriptListener = ls->GetJSListener();
-
-    if ((!aContext && scriptListener) ||
-        (aContext && !scriptListener)) {
-      return NS_ERROR_FAILURE;
-    }
+    MOZ_ASSERT(scriptListener,
+               "How can we have an event handler with no nsIJSEventListener?");
 
-    if (scriptListener) {
-      scriptListener->SetHandler(aHandler);
-    } else {
-      MOZ_ASSERT(aHandler.HasEventHandler());
-      nsCOMPtr<nsIDOMEventListener> listener;
-      rv = nsContentUtils::XPConnect()->WrapJS(aCx,
-                                               aHandler.Ptr()->Callable(),
-                                               NS_GET_IID(nsIDOMEventListener),
-                                               getter_AddRefs(listener));
-      if (NS_SUCCEEDED(rv)) {
-        ls->mListener = listener.forget();
-      }
-    }
+    scriptListener->SetHandler(aHandler, aContext, aScopeObject);
   }
 
   if (NS_SUCCEEDED(rv) && ls) {
     // Set flag to indicate possible need for compilation later
     ls->mHandlerIsString = !aHandler.HasEventHandler();
     if (aPermitUntrustedEvents) {
       ls->mFlags |= NS_PRIV_EVENT_UNTRUSTED_PERMITTED;
     }
@@ -705,17 +675,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, nsEventHandler(),
+  rv = SetEventHandlerInternal(context, scope, aName, nsEventHandler(),
                                aPermitUntrustedEvents, &ls);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!aDeferCompilation) {
     return CompileEventHandlerInternal(ls, true, &aBody);
   }
 
   return NS_OK;
@@ -1189,18 +1159,17 @@ nsEventListenerManager::HasUnloadListene
   }
   return false;
 }
 
 nsresult
 nsEventListenerManager::SetEventHandlerToJsval(nsIAtom* aEventName,
                                                JSContext* cx,
                                                JSObject* aScope,
-                                               const jsval& v,
-                                               bool aExpectScriptContext)
+                                               const jsval& v)
 {
   JSObject *callable;
   if (JSVAL_IS_PRIMITIVE(v) ||
       !JS_ObjectIsCallable(cx, callable = JSVAL_TO_OBJECT(v))) {
     RemoveEventHandler(aEventName);
     return NS_OK;
   }
 
@@ -1229,27 +1198,22 @@ nsEventListenerManager::SetEventHandlerT
     nsRefPtr<EventHandlerNonNull> handlerCallback =
       new EventHandlerNonNull(cx, aScope, callable, &ok);
     if (!ok) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
     handler.SetHandler(handlerCallback);
   }
 
-  // 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,
+  return SetEventHandlerInternal(nullptr, scope, aEventName, handler,
                                  !nsContentUtils::IsCallerChrome(), &ignored);
 }
 
 void
 nsEventListenerManager::GetEventHandler(nsIAtom *aEventName, jsval *vp)
 {
   uint32_t eventType = nsContentUtils::GetEventId(aEventName);
   nsListenerStruct* ls = FindEventHandler(eventType, aEventName);
--- a/content/events/src/nsEventListenerManager.h
+++ b/content/events/src/nsEventListenerManager.h
@@ -258,43 +258,41 @@ protected:
   /**
    * 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
    * 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.
+   * compile the string for this listener, but in that case aContext must be
+   * non-null.  Otherwise, aContext is allowed to be null.  The nsListenerStruct
+   * that results, if any, is returned in aListenerStruct.
    */
   nsresult SetEventHandlerInternal(nsIScriptContext *aContext,
-                                   JSContext* aCx,
                                    JSObject* aScopeGlobal,
                                    nsIAtom* aName,
                                    const nsEventHandler& aHandler,
                                    bool aPermitUntrustedEvents,
                                    nsListenerStruct **aListenerStruct);
 
   bool IsDeviceType(uint32_t aType);
   void EnableDevice(uint32_t aType);
   void DisableDevice(uint32_t aType);
 
 public:
   /**
    * Set the "inline" event listener for aEventName to |v|.  This
    * might actually remove the event listener, depending on the value
    * 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.  If aExpectScriptContext is false,
-   * not finding an nsIScriptContext does not cause failure.
+   * be in the same compartment.
    */
   nsresult SetEventHandlerToJsval(nsIAtom* aEventName, JSContext* cx,
-                                  JSObject* aScope, const jsval& v,
-                                  bool aExpectScriptContext);
+                                  JSObject* aScope, const jsval& v);
   /**
    * Get the value of the "inline" event listener for aEventName.
    * This may cause lazy compilation if the listener is uncompiled.
    */
   void GetEventHandler(nsIAtom *aEventName, jsval *vp);
 
 protected:
   void AddEventListener(nsIDOMEventListener *aListener, 
--- a/content/xbl/src/nsXBLPrototypeHandler.cpp
+++ b/content/xbl/src/nsXBLPrototypeHandler.cpp
@@ -304,17 +304,17 @@ nsXBLPrototypeHandler::ExecuteHandler(ns
   if (!ok) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   nsEventHandler eventHandler(handlerCallback);
 
   // Execute it.
   nsCOMPtr<nsIJSEventListener> eventListener;
-  rv = NS_NewJSEventListener(boundContext, scope,
+  rv = NS_NewJSEventListener(nullptr, scope,
                              scriptTarget, onEventAtom,
                              eventHandler,
                              getter_AddRefs(eventListener));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Handle the event.
   eventListener->HandleEvent(aEvent);
   eventListener->Disconnect();
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -11204,18 +11204,17 @@ nsGlobalWindow::DisableNetworkEvent(uint
     if (!elm) {                                                              \
       return NS_ERROR_OUT_OF_MEMORY;                                         \
     }                                                                        \
                                                                              \
     JSObject *obj = mJSObject;                                               \
     if (!obj) {                                                              \
       return NS_ERROR_UNEXPECTED;                                            \
     }                                                                        \
-    return elm->SetEventHandlerToJsval(nsGkAtoms::on##name_, cx, obj, v,     \
-                                       true);                                \
+    return elm->SetEventHandlerToJsval(nsGkAtoms::on##name_, cx, obj, v);    \
   }
 #define WINDOW_ONLY_EVENT EVENT
 #define TOUCH_EVENT EVENT
 #include "nsEventNameList.h"
 #undef TOUCH_EVENT
 #undef WINDOW_ONLY_EVENT
 #undef EVENT
 
--- a/dom/base/nsIJSEventListener.h
+++ b/dom/base/nsIJSEventListener.h
@@ -171,16 +171,17 @@ public:
                      const nsEventHandler& aHandler)
   : mContext(aContext), mScopeObject(aScopeObject), mEventName(aType),
     mHandler(aHandler)
   {
     nsCOMPtr<nsISupports> base = do_QueryInterface(aTarget);
     mTarget = base.get();
   }
 
+  // Can return null if we already have a handler.
   nsIScriptContext *GetEventContext() const
   {
     return mContext;
   }
 
   nsISupports *GetEventTarget() const
   {
     return mTarget;
@@ -203,19 +204,22 @@ public:
 
   nsIAtom* EventName() const
   {
     return mEventName;
   }
 
   // Set a handler for this event listener.  The handler must already
   // be bound to the right target.
-  void SetHandler(const nsEventHandler& aHandler)
+  void SetHandler(const nsEventHandler& aHandler, nsIScriptContext* aContext,
+                  JSObject* aScopeObject)
   {
     mHandler.SetHandler(aHandler);
+    mContext = aContext;
+    mScopeObject = aScopeObject;
   }
   void SetHandler(mozilla::dom::EventHandlerNonNull* aHandler)
   {
     mHandler.SetHandler(aHandler);
   }
   void SetHandler(mozilla::dom::BeforeUnloadEventHandlerNonNull* aHandler)
   {
     mHandler.SetHandler(aHandler);
@@ -255,15 +259,17 @@ protected:
   JSObject* mScopeObject;
   nsISupports* mTarget;
   nsCOMPtr<nsIAtom> mEventName;
   nsEventHandler mHandler;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIJSEventListener, NS_IJSEVENTLISTENER_IID)
 
-/* factory function.  aHandler must already be bound to aTarget */
+/* factory function.  aHandler must already be bound to aTarget.
+   aContext is allowed to be null if aHandler is already set up.
+ */
 nsresult NS_NewJSEventListener(nsIScriptContext *aContext,
                                JSObject* aScopeObject, nsISupports* aTarget,
                                nsIAtom* aType, const nsEventHandler& aHandler,
                                nsIJSEventListener **aReturn);
 
 #endif // nsIJSEventListener_h__
--- a/dom/src/events/nsJSEventListener.cpp
+++ b/dom/src/events/nsJSEventListener.cpp
@@ -50,31 +50,31 @@ nsJSEventListener::nsJSEventListener(nsI
                                      JSObject* aScopeObject,
                                      nsISupports *aTarget,
                                      nsIAtom* 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,
+  NS_ASSERTION(aScopeObject,
                "EventListener with no context or scope?");
   NS_HOLD_JS_OBJECTS(this, nsJSEventListener);
 }
 
 nsJSEventListener::~nsJSEventListener() 
 {
-  if (mContext) {
+  if (mScopeObject) {
     NS_DROP_JS_OBJECTS(this, nsJSEventListener);
   }
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSEventListener)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSEventListener)
-  if (tmp->mContext) {
+  if (tmp->mScopeObject) {
     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) {
@@ -129,32 +129,37 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 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)) &&
+  // We can claim to be black if all the things we reference are
+  // effectively black already.
+  if ((!mScopeObject || !xpc_IsGrayGCThing(mScopeObject)) &&
       (!mHandler.HasEventHandler() ||
        !mHandler.Ptr()->HasGrayCallable())) {
+    if (!mContext) {
+      // Well, we certainly won't be marking it, so move on!
+      return true;
+    }
     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.HasEventHandler())
+  if (!target || !mHandler.HasEventHandler())
     return NS_ERROR_FAILURE;
 
   if (mHandler.Type() == nsEventHandler::eOnError) {
     MOZ_ASSERT(mEventName == nsGkAtoms::onerror);
 
     nsString errorMsg, file;
     EventOrString msgOrEvent;
     Optional<nsAString> fileName;
@@ -251,16 +256,18 @@ nsJSEventListener::HandleEvent(nsIDOMEve
  */
 
 nsresult
 NS_NewJSEventListener(nsIScriptContext* aContext, JSObject* aScopeObject,
                       nsISupports*aTarget, nsIAtom* aEventType,
                       const nsEventHandler& aHandler,
                       nsIJSEventListener** aReturn)
 {
+  MOZ_ASSERT(aContext || aHandler.HasEventHandler(),
+             "Must have a handler if we don't have an nsIScriptContext");
   NS_ENSURE_ARG(aEventType);
   nsJSEventListener* it =
     new nsJSEventListener(aContext, aScopeObject, aTarget, aEventType,
                           aHandler);
   NS_ADDREF(*aReturn = it);
 
   return NS_OK;
 }