Bug 986304 - Remove the scope object machinery attached to nsIJSEventListener. r=bz,f=smaug

dom/events/EventListenerManager.cpp

@@ -597,14 +597,11 @@ EventListenerManager::FindEventHandler(uint32_t aEventType,
 
 EventListenerManager::Listener*
 EventListenerManager::SetEventHandlerInternal(
-                        JS::Handle<JSObject*> aScopeObject,
                         nsIAtom* aName,
                         const nsAString& aTypeString,
                         const nsEventHandler& aHandler,
                         bool aPermitUntrustedEvents)
 {
-  MOZ_ASSERT(aScopeObject || aHandler.HasEventHandler(),
-             "Must have one or the other!");
   MOZ_ASSERT(aName || !aTypeString.IsEmpty());
 
   uint32_t eventType = nsContentUtils::GetEventId(aName);
@@ -617,7 +614,7 @@ EventListenerManager::SetEventHandlerInternal(
     flags.mListenerIsJSListener = true;
 
     nsCOMPtr<nsIJSEventListener> jsListener;
-    NS_NewJSEventListener(aScopeObject, mTarget, aName,
+    NS_NewJSEventListener(mTarget, aName,
                           aHandler, getter_AddRefs(jsListener));
     EventListenerHolder listenerHolder(jsListener);
     AddEventListenerInternal(listenerHolder, eventType, aName, aTypeString,
@@ -631,7 +628,7 @@ EventListenerManager::SetEventHandlerInternal(
 
     bool same = jsListener->GetHandler() == aHandler;
     // Possibly the same listener, but update still the context and scope.
-    jsListener->SetHandler(aHandler, aScopeObject);
+    jsListener->SetHandler(aHandler);
     if (mTarget && !same && aName) {
       mTarget->EventListenerRemoved(aName);
       mTarget->EventListenerAdded(aName);
@@ -746,14 +743,9 @@ EventListenerManager::SetEventHandler(nsIAtom* aName,
 
   nsIScriptContext* context = global->GetScriptContext();
   NS_ENSURE_TRUE(context, NS_ERROR_FAILURE);
-
   NS_ENSURE_STATE(global->GetGlobalJSObject());
 
-  JSAutoRequest ar(context->GetNativeContext());
+  Listener* listener = SetEventHandlerInternal(aName,
-  JS::Rooted<JSObject*> scope(context->GetNativeContext(),
-                              global->GetGlobalJSObject());
-
-  Listener* listener = SetEventHandlerInternal(scope, aName,
                                                EmptyString(),
                                                nsEventHandler(),
                                                aPermitUntrustedEvents);
@@ -807,7 +799,6 @@ EventListenerManager::CompileEventHandlerInternal(Listener* aListener,
 
   // Push a context to make sure exceptions are reported in the right place.
   AutoPushJSContextForErrorReporting cx(context->GetNativeContext());
-  JS::Rooted<JSObject*> scope(cx, jsListener->GetEventScope());
 
   nsCOMPtr<nsIAtom> typeAtom = aListener->mTypeAtom;
   nsIAtom* attrName = typeAtom;
@@ -1231,8 +1222,7 @@ EventListenerManager::SetEventHandler(nsIAtom* aEventName,
 
   // Untrusted events are always permitted for non-chrome script
   // handlers.
-  SetEventHandlerInternal(JS::NullPtr(), aEventName,
+  SetEventHandlerInternal(aEventName, aTypeString, nsEventHandler(aHandler),
-                          aTypeString, nsEventHandler(aHandler),
                           !mIsMainThreadELM ||
                           !nsContentUtils::IsCallerChrome());
 }
@@ -1248,8 +1238,8 @@ EventListenerManager::SetEventHandler(OnErrorEventHandlerNonNull* aHandler)
 
     // Untrusted events are always permitted for non-chrome script
     // handlers.
-    SetEventHandlerInternal(JS::NullPtr(), nsGkAtoms::onerror,
+    SetEventHandlerInternal(nsGkAtoms::onerror, EmptyString(),
-                            EmptyString(), nsEventHandler(aHandler),
+                            nsEventHandler(aHandler),
                             !nsContentUtils::IsCallerChrome());
   } else {
     if (!aHandler) {
@@ -1258,8 +1248,7 @@ EventListenerManager::SetEventHandler(OnErrorEventHandlerNonNull* aHandler)
     }
 
     // Untrusted events are always permitted.
-    SetEventHandlerInternal(JS::NullPtr(), nullptr,
+    SetEventHandlerInternal(nullptr, NS_LITERAL_STRING("error"),
-                            NS_LITERAL_STRING("error"),
                             nsEventHandler(aHandler), true);
   }
 }
@@ -1275,8 +1264,8 @@ EventListenerManager::SetEventHandler(
 
   // Untrusted events are always permitted for non-chrome script
   // handlers.
-  SetEventHandlerInternal(JS::NullPtr(), nsGkAtoms::onbeforeunload,
+  SetEventHandlerInternal(nsGkAtoms::onbeforeunload, EmptyString(),
-                          EmptyString(), nsEventHandler(aHandler),
+                          nsEventHandler(aHandler),
                           !mIsMainThreadELM ||
                           !nsContentUtils::IsCallerChrome());
 }
@@ -1332,9 +1321,6 @@ EventListenerManager::MarkForCC()
       if (jsListener->GetHandler().HasEventHandler()) {
         JS::ExposeObjectToActiveJS(jsListener->GetHandler().Ptr()->Callable());
       }
-      if (JSObject* scope = jsListener->GetEventScope()) {
-        JS::ExposeObjectToActiveJS(scope);
-      }
     } else if (listener.mListenerType == Listener::eWrappedJSListener) {
       xpc_TryUnmarkWrappedGrayObject(listener.mListener.GetXPCOMCallback());
     } else if (listener.mListenerType == Listener::eWebIDLListener) {

dom/events/EventListenerManager.h

@@ -447,8 +447,7 @@ protected:
    * aScopeGlobal must be non-null.  Otherwise, aContext and aScopeGlobal are
    * allowed to be null.
    */
-  Listener* SetEventHandlerInternal(JS::Handle<JSObject*> aScopeGlobal,
+  Listener* SetEventHandlerInternal(nsIAtom* aName,
-                                    nsIAtom* aName,
                                     const nsAString& aTypeString,
                                     const nsEventHandler& aHandler,
                                     bool aPermitUntrustedEvents);

dom/events/nsIJSEventListener.h

@@ -14,8 +14,8 @@
 #include "mozilla/dom/EventHandlerBinding.h"
 
 #define NS_IJSEVENTLISTENER_IID \
-{ 0x5077b12a, 0x5a1f, 0x4583, \
+{ 0x8f06b4af, 0xbd0d, 0x486b, \
-  { 0xbb, 0xa7, 0x78, 0x84, 0x94, 0x0e, 0x5e, 0xff } }
+  { 0x81, 0xc8, 0x20, 0x42, 0x40, 0x2b, 0xf1, 0xef } }
 
 class nsEventHandler
 {
@@ -172,10 +172,9 @@ class nsIJSEventListener : public nsIDOMEventListener
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IJSEVENTLISTENER_IID)
 
-  nsIJSEventListener(JSObject* aScopeObject,
+  nsIJSEventListener(nsISupports *aTarget, nsIAtom* aType,
-                     nsISupports *aTarget, nsIAtom* aType,
                      const nsEventHandler& aHandler)
-  : mScopeObject(aScopeObject), mEventName(aType), mHandler(aHandler)
+  : mEventName(aType), mHandler(aHandler)
   {
     nsCOMPtr<nsISupports> base = do_QueryInterface(aTarget);
     mTarget = base.get();
@@ -191,17 +190,6 @@ public:
     mTarget = nullptr;
   }
 
-  // Can return null if we already have a handler.
-  JSObject* GetEventScope() const
-  {
-    if (!mScopeObject) {
-      return nullptr;
-    }
-
-    JS::ExposeObjectToActiveJS(mScopeObject);
-    return mScopeObject;
-  }
-
   const nsEventHandler& GetHandler() const
   {
     return mHandler;
@@ -219,11 +207,9 @@ public:
 
   // 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)
-                  JS::Handle<JSObject*> aScopeObject)
   {
     mHandler.SetHandler(aHandler);
-    UpdateScopeObject(aScopeObject);
   }
   void SetHandler(mozilla::dom::EventHandlerNonNull* aHandler)
   {
@@ -247,8 +233,6 @@ public:
     // - mTarget
     //
     // The following members are not measured:
-    // - mScopeObject: because they're measured by the JS memory
-    //   reporters
     // - mHandler: may be shared with others
     // - mEventName: shared with others
   }
@@ -264,11 +248,6 @@ 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(JS::Handle<JSObject*> aScopeObject) = 0;
-
-  JS::Heap<JSObject*> mScopeObject;
   nsISupports* mTarget;
   nsCOMPtr<nsIAtom> mEventName;
   nsEventHandler mHandler;
@@ -279,8 +258,8 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsIJSEventListener, NS_IJSEVENTLISTENER_IID)
 /* factory function.  aHandler must already be bound to aTarget.
    aContext is allowed to be null if aHandler is already set up.
  */
-nsresult NS_NewJSEventListener(JSObject* aScopeObject, nsISupports* aTarget,
+nsresult NS_NewJSEventListener(nsISupports* aTarget, nsIAtom* aType,
-                               nsIAtom* aType, const nsEventHandler& aHandler,
+                               const nsEventHandler& aHandler,
                                nsIJSEventListener **aReturn);
 
 #endif // nsIJSEventListener_h__

dom/events/nsJSEventListener.cpp

@@ -43,45 +43,16 @@ using namespace mozilla::dom;
 /*
  * nsJSEventListener implementation
  */
-nsJSEventListener::nsJSEventListener(JSObject* aScopeObject,
+nsJSEventListener::nsJSEventListener(nsISupports *aTarget,
-                                     nsISupports *aTarget,
                                      nsIAtom* aType,
                                      const nsEventHandler& aHandler)
-  : nsIJSEventListener(aScopeObject, aTarget, aType, aHandler)
+  : nsIJSEventListener(aTarget, aType, aHandler)
 {
-  if (mScopeObject) {
-    mozilla::HoldJSObjects(this);
-  }
-}
-
-nsJSEventListener::~nsJSEventListener() 
-{
-  if (mScopeObject) {
-    mScopeObject = nullptr;
-    mozilla::DropJSObjects(this);
-  }
-}
-
-/* virtual */
-void
-nsJSEventListener::UpdateScopeObject(JS::Handle<JSObject*> aScopeObject)
-{
-  if (mScopeObject && !aScopeObject) {
-    mScopeObject = nullptr;
-    mozilla::DropJSObjects(this);
-  } else if (aScopeObject && !mScopeObject) {
-    mozilla::HoldJSObjects(this);
-  }
-  mScopeObject = aScopeObject;
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSEventListener)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSEventListener)
-  if (tmp->mScopeObject) {
-    tmp->mScopeObject = nullptr;
-    mozilla::DropJSObjects(tmp);
-  }
   tmp->mHandler.ForgetHandler();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsJSEventListener)
@@ -98,10 +69,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsJSEventListener)
   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_END
-
 NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_BEGIN(nsJSEventListener)
   if (tmp->IsBlackForCC()) {
     return true;
@@ -143,12 +110,7 @@ nsJSEventListener::IsBlackForCC()
 {
   // We can claim to be black if all the things we reference are
   // effectively black already.
-  if ((!mScopeObject || !xpc_IsGrayGCThing(mScopeObject)) &&
+  return !mHandler.HasEventHandler() || !mHandler.Ptr()->HasGrayCallable();
-      (!mHandler.HasEventHandler() ||
-       !mHandler.Ptr()->HasGrayCallable())) {
-    return true;
-  }
-  return false;
 }
 
 nsresult
@@ -274,14 +236,13 @@ nsJSEventListener::HandleEvent(nsIDOMEvent* aEvent)
  */
 
 nsresult
-NS_NewJSEventListener(JSObject* aScopeObject,
+NS_NewJSEventListener(nsISupports*aTarget, nsIAtom* aEventType,
-                      nsISupports*aTarget, nsIAtom* aEventType,
                       const nsEventHandler& aHandler,
                       nsIJSEventListener** aReturn)
 {
   NS_ENSURE_ARG(aEventType || !NS_IsMainThread());
   nsJSEventListener* it =
-    new nsJSEventListener(aScopeObject, aTarget, aEventType, aHandler);
+    new nsJSEventListener(aTarget, aEventType, aHandler);
   NS_ADDREF(*aReturn = it);
 
   return NS_OK;

dom/events/nsJSEventListener.h

@@ -21,9 +21,8 @@
 class nsJSEventListener : public nsIJSEventListener
 {
 public:
-  nsJSEventListener(JSObject* aScopeObject, nsISupports* aTarget,
+  nsJSEventListener(nsISupports* aTarget, nsIAtom* aType,
-                    nsIAtom* aType, const nsEventHandler& aHandler);
+                    const nsEventHandler& aHandler);
-  virtual ~nsJSEventListener();
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
 
@@ -37,10 +36,7 @@ public:
     return aMallocSizeOf(this) + SizeOfExcludingThis(aMallocSizeOf);
   }
 
-  NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS(nsJSEventListener)
+  NS_DECL_CYCLE_COLLECTION_SKIPPABLE_CLASS(nsJSEventListener)
-
-protected:
-  virtual void UpdateScopeObject(JS::Handle<JSObject*> aScopeObject);
 
   bool IsBlackForCC();
 };

dom/xbl/nsXBLPrototypeHandler.cpp

@@ -326,8 +326,7 @@ nsXBLPrototypeHandler::ExecuteHandler(EventTarget* aTarget,
 
   // Execute it.
   nsCOMPtr<nsIJSEventListener> eventListener;
-  rv = NS_NewJSEventListener(globalObject,
+  rv = NS_NewJSEventListener(scriptTarget, onEventAtom,
-                             scriptTarget, onEventAtom,
                              eventHandler,
                              getter_AddRefs(eventListener));
   NS_ENSURE_SUCCESS(rv, rv);