Bug 807226 part 5. Allow event handlers to have a null scope object if they don't have to compile from a string. r=smaug

content/events/src/nsEventListenerManager.cpp

@@ -528,7 +528,7 @@ nsEventListenerManager::SetEventHandlerInternal(nsIScriptContext *aContext,
                                                 bool aPermitUntrustedEvents,
                                                 nsListenerStruct **aListenerStruct)
 {
-  NS_ASSERTION(aContext || aHandler.HasEventHandler(),
+  NS_ASSERTION((aContext && aScopeObject) || aHandler.HasEventHandler(),
                "Must have one or the other!");
 
   nsresult rv = NS_OK;
@@ -1203,12 +1203,10 @@ nsEventListenerManager::SetEventHandlerToJsval(nsIAtom* aEventName,
     handler.SetHandler(handlerCallback);
   }
 
-  JSAutoCompartment ac(cx, aScope);
-  JSObject *scope = ::JS_GetGlobalForObject(cx, aScope);
   // Untrusted events are always permitted for non-chrome script
   // handlers.
   nsListenerStruct *ignored;
-  return SetEventHandlerInternal(nullptr, scope, aEventName, handler,
+  return SetEventHandlerInternal(nullptr, nullptr, aEventName, handler,
                                  !nsContentUtils::IsCallerChrome(), &ignored);
 }
 

content/events/src/nsEventListenerManager.h

@@ -263,9 +263,10 @@ protected:
   /**
    * 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, 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.
+   * compile the string for this listener, but in that case aContext and
+   * aScopeGlobal must be non-null.  Otherwise, aContext and aScopeGlobal are
+   * allowed to be null.  The nsListenerStruct that results, if any, is returned
+   * in aListenerStruct.
    */
   nsresult SetEventHandlerInternal(nsIScriptContext *aContext,
                                    JSObject* aScopeGlobal,

dom/base/nsIJSEventListener.h

@@ -192,6 +192,7 @@ public:
     mTarget = nullptr;
   }
 
+  // Can return null if we already have a handler.
   JSObject* GetEventScope() const
   {
     return xpc_UnmarkGrayObject(mScopeObject);
@@ -214,7 +215,7 @@ public:
   {
     mHandler.SetHandler(aHandler);
     mContext = aContext;
-    mScopeObject = aScopeObject;
+    UpdateScopeObject(aScopeObject);
   }
   void SetHandler(mozilla::dom::EventHandlerNonNull* aHandler)
   {
@@ -255,6 +256,11 @@ protected:
   {
     NS_ASSERTION(!mTarget, "Should have called Disconnect()!");
   }
+
+  // Update our mScopeObject; we have to make sure we properly handle
+  // the hold/drop stuff, so have to do it in nsJSEventListener.
+  virtual void UpdateScopeObject(JSObject* aScopeObject) = 0;
+
   nsCOMPtr<nsIScriptContext> mContext;
   JSObject* mScopeObject;
   nsISupports* mTarget;

dom/src/events/nsJSEventListener.cpp

@@ -53,11 +53,9 @@ nsJSEventListener::nsJSEventListener(nsIScriptContext *aContext,
                                      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,
-               "EventListener with no context or scope?");
-  NS_HOLD_JS_OBJECTS(this, nsJSEventListener);
+  if (mScopeObject) {
+    NS_HOLD_JS_OBJECTS(this, nsJSEventListener);
+  }
 }
 
 nsJSEventListener::~nsJSEventListener() 
@@ -67,6 +65,18 @@ nsJSEventListener::~nsJSEventListener()
   }
 }
 
+/* virtual */
+void
+nsJSEventListener::UpdateScopeObject(JSObject* aScopeObject)
+{
+  if (mScopeObject && !aScopeObject) {
+    NS_DROP_JS_OBJECTS(this, nsJSEventListener);
+  } else if (aScopeObject && !mScopeObject) {
+    NS_HOLD_JS_OBJECTS(this, nsJSEventListener);
+  }
+  mScopeObject = aScopeObject;
+}
+
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSEventListener)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSEventListener)
   if (tmp->mScopeObject) {

dom/src/events/nsJSEventListener.h

@@ -40,6 +40,8 @@ public:
   NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS(nsJSEventListener)
 
 protected:
+  virtual void UpdateScopeObject(JSObject* aScopeObject);
+
   bool IsBlackForCC();
 };