From d54e0e9a5f096d534e3231574b9e0b5a7152314c Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 21 Mar 2014 23:31:03 -0300 Subject: [PATCH] Bug 986304 - Remove the scope object machinery attached to nsIJSEventListener. r=bz,f=smaug --- dom/events/EventListenerManager.cpp | 32 ++++++------------- dom/events/EventListenerManager.h | 3 +- dom/events/nsIJSEventListener.h | 35 +++++---------------- dom/events/nsJSEventListener.cpp | 49 +++-------------------------- dom/events/nsJSEventListener.h | 10 ++---- dom/xbl/nsXBLPrototypeHandler.cpp | 3 +- 6 files changed, 26 insertions(+), 106 deletions(-) diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index 53bf0741f9dc..f7e5eafdf680 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -597,14 +597,11 @@ EventListenerManager::FindEventHandler(uint32_t aEventType, EventListenerManager::Listener* EventListenerManager::SetEventHandlerInternal( - JS::Handle 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 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()); - JS::Rooted scope(context->GetNativeContext(), - global->GetGlobalJSObject()); - - Listener* listener = SetEventHandlerInternal(scope, aName, + Listener* listener = SetEventHandlerInternal(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 scope(cx, jsListener->GetEventScope()); nsCOMPtr 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, - aTypeString, nsEventHandler(aHandler), + SetEventHandlerInternal(aEventName, 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, - EmptyString(), nsEventHandler(aHandler), + SetEventHandlerInternal(nsGkAtoms::onerror, EmptyString(), + nsEventHandler(aHandler), !nsContentUtils::IsCallerChrome()); } else { if (!aHandler) { @@ -1258,8 +1248,7 @@ EventListenerManager::SetEventHandler(OnErrorEventHandlerNonNull* aHandler) } // Untrusted events are always permitted. - SetEventHandlerInternal(JS::NullPtr(), nullptr, - NS_LITERAL_STRING("error"), + SetEventHandlerInternal(nullptr, 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, - EmptyString(), nsEventHandler(aHandler), + SetEventHandlerInternal(nsGkAtoms::onbeforeunload, EmptyString(), + 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) { diff --git a/dom/events/EventListenerManager.h b/dom/events/EventListenerManager.h index 64f48413dc62..9efdb3f52a5a 100644 --- a/dom/events/EventListenerManager.h +++ b/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 aScopeGlobal, - nsIAtom* aName, + Listener* SetEventHandlerInternal(nsIAtom* aName, const nsAString& aTypeString, const nsEventHandler& aHandler, bool aPermitUntrustedEvents); diff --git a/dom/events/nsIJSEventListener.h b/dom/events/nsIJSEventListener.h index 390b64beb036..d0f09bf56f0d 100644 --- a/dom/events/nsIJSEventListener.h +++ b/dom/events/nsIJSEventListener.h @@ -14,8 +14,8 @@ #include "mozilla/dom/EventHandlerBinding.h" #define NS_IJSEVENTLISTENER_IID \ -{ 0x5077b12a, 0x5a1f, 0x4583, \ - { 0xbb, 0xa7, 0x78, 0x84, 0x94, 0x0e, 0x5e, 0xff } } +{ 0x8f06b4af, 0xbd0d, 0x486b, \ + { 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, - nsISupports *aTarget, nsIAtom* aType, + nsIJSEventListener(nsISupports *aTarget, nsIAtom* aType, const nsEventHandler& aHandler) - : mScopeObject(aScopeObject), mEventName(aType), mHandler(aHandler) + : mEventName(aType), mHandler(aHandler) { nsCOMPtr 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, - JS::Handle aScopeObject) + void SetHandler(const nsEventHandler& aHandler) { 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 aScopeObject) = 0; - - JS::Heap mScopeObject; nsISupports* mTarget; nsCOMPtr 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, - nsIAtom* aType, const nsEventHandler& aHandler, +nsresult NS_NewJSEventListener(nsISupports* aTarget, nsIAtom* aType, + const nsEventHandler& aHandler, nsIJSEventListener **aReturn); #endif // nsIJSEventListener_h__ diff --git a/dom/events/nsJSEventListener.cpp b/dom/events/nsJSEventListener.cpp index 4f766590c0ca..ec5485fefec9 100644 --- a/dom/events/nsJSEventListener.cpp +++ b/dom/events/nsJSEventListener.cpp @@ -43,45 +43,16 @@ using namespace mozilla::dom; /* * nsJSEventListener implementation */ -nsJSEventListener::nsJSEventListener(JSObject* aScopeObject, - nsISupports *aTarget, +nsJSEventListener::nsJSEventListener(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 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)) && - (!mHandler.HasEventHandler() || - !mHandler.Ptr()->HasGrayCallable())) { - return true; - } - return false; + return !mHandler.HasEventHandler() || !mHandler.Ptr()->HasGrayCallable(); } nsresult @@ -274,14 +236,13 @@ nsJSEventListener::HandleEvent(nsIDOMEvent* aEvent) */ nsresult -NS_NewJSEventListener(JSObject* aScopeObject, - nsISupports*aTarget, nsIAtom* aEventType, +NS_NewJSEventListener(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; diff --git a/dom/events/nsJSEventListener.h b/dom/events/nsJSEventListener.h index 4238cc9cf457..8144bb080844 100644 --- a/dom/events/nsJSEventListener.h +++ b/dom/events/nsJSEventListener.h @@ -21,9 +21,8 @@ class nsJSEventListener : public nsIJSEventListener { public: - nsJSEventListener(JSObject* aScopeObject, nsISupports* aTarget, - nsIAtom* aType, const nsEventHandler& aHandler); - virtual ~nsJSEventListener(); + nsJSEventListener(nsISupports* aTarget, nsIAtom* aType, + const nsEventHandler& aHandler); NS_DECL_CYCLE_COLLECTING_ISUPPORTS @@ -37,10 +36,7 @@ public: return aMallocSizeOf(this) + SizeOfExcludingThis(aMallocSizeOf); } - NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS(nsJSEventListener) - -protected: - virtual void UpdateScopeObject(JS::Handle aScopeObject); + NS_DECL_CYCLE_COLLECTION_SKIPPABLE_CLASS(nsJSEventListener) bool IsBlackForCC(); }; diff --git a/dom/xbl/nsXBLPrototypeHandler.cpp b/dom/xbl/nsXBLPrototypeHandler.cpp index 6d32fbe49bbb..1a6923bf0b96 100644 --- a/dom/xbl/nsXBLPrototypeHandler.cpp +++ b/dom/xbl/nsXBLPrototypeHandler.cpp @@ -326,8 +326,7 @@ nsXBLPrototypeHandler::ExecuteHandler(EventTarget* aTarget, // Execute it. nsCOMPtr eventListener; - rv = NS_NewJSEventListener(globalObject, - scriptTarget, onEventAtom, + rv = NS_NewJSEventListener(scriptTarget, onEventAtom, eventHandler, getter_AddRefs(eventListener)); NS_ENSURE_SUCCESS(rv, rv);