Bug 1438541 P1 Track DOMEventTargetHelper objects in the nsIGlobalObject base class and always call DisconnectFromOwner() on them. r=smaug
authorBen Kelly <ben@wanderview.com>
Wed, 21 Feb 2018 10:53:22 -0800
changeset 404782 a1d30eced092547c59253e12383fee05c0284b04
parent 404703 ba1d6f897a923b561bd9458f77bd3f7f63b99573
child 404783 9a2fc66acc13134ec5025fa65ed226e196e30a2e
push id33490
push userdluca@mozilla.com
push dateThu, 22 Feb 2018 10:00:20 +0000
treeherdermozilla-central@ea3da643422c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1438541
milestone60.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 1438541 P1 Track DOMEventTargetHelper objects in the nsIGlobalObject base class and always call DisconnectFromOwner() on them. r=smaug
dom/base/nsGlobalWindowInner.cpp
dom/base/nsGlobalWindowInner.h
dom/base/nsIGlobalObject.cpp
dom/base/nsIGlobalObject.h
dom/events/DOMEventTargetHelper.cpp
dom/events/DOMEventTargetHelper.h
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -1097,39 +1097,16 @@ nsGlobalWindowInner::~nsGlobalWindowInne
 
   nsCOMPtr<nsIDeviceSensors> ac = do_GetService(NS_DEVICE_SENSORS_CONTRACTID);
   if (ac)
     ac->RemoveWindowAsListener(this);
 
   nsLayoutStatics::Release();
 }
 
-void
-nsGlobalWindowInner::AddEventTargetObject(DOMEventTargetHelper* aObject)
-{
-  mEventTargetObjects.PutEntry(aObject);
-}
-
-void
-nsGlobalWindowInner::RemoveEventTargetObject(DOMEventTargetHelper* aObject)
-{
-  mEventTargetObjects.RemoveEntry(aObject);
-}
-
-void
-nsGlobalWindowInner::DisconnectEventTargetObjects()
-{
-  for (auto iter = mEventTargetObjects.ConstIter(); !iter.Done();
-       iter.Next()) {
-    RefPtr<DOMEventTargetHelper> target = iter.Get()->GetKey();
-    target->DisconnectFromOwner();
-  }
-  mEventTargetObjects.Clear();
-}
-
 // static
 void
 nsGlobalWindowInner::ShutDown()
 {
   AssertIsOnMainThread();
 
   if (gDumpFile && gDumpFile != stdout) {
     fclose(gDumpFile);
@@ -6902,16 +6879,18 @@ nsGlobalWindowInner::IsVRContentPresenti
   }
   return false;
 }
 
 void
 nsGlobalWindowInner::AddSizeOfIncludingThis(nsWindowSizes& aWindowSizes) const
 {
   aWindowSizes.mDOMOtherSize += aWindowSizes.mState.mMallocSizeOf(this);
+  aWindowSizes.mDOMOtherSize +=
+    nsIGlobalObject::ShallowSizeOfExcludingThis(aWindowSizes.mState.mMallocSizeOf);
 
   EventListenerManager* elm = GetExistingListenerManager();
   if (elm) {
     aWindowSizes.mDOMOtherSize +=
       elm->SizeOfIncludingThis(aWindowSizes.mState.mMallocSizeOf);
     aWindowSizes.mDOMEventListenersCount += elm->ListenerCount();
   }
   if (mDoc) {
@@ -6924,32 +6903,27 @@ nsGlobalWindowInner::AddSizeOfIncludingT
     }
   }
 
   if (mNavigator) {
     aWindowSizes.mDOMOtherSize +=
       mNavigator->SizeOfIncludingThis(aWindowSizes.mState.mMallocSizeOf);
   }
 
-  aWindowSizes.mDOMEventTargetsSize +=
-    mEventTargetObjects.ShallowSizeOfExcludingThis(
-      aWindowSizes.mState.mMallocSizeOf);
-
-  for (auto iter = mEventTargetObjects.ConstIter(); !iter.Done(); iter.Next()) {
-    DOMEventTargetHelper* et = iter.Get()->GetKey();
+  ForEachEventTargetObject([&] (DOMEventTargetHelper* et) {
     if (nsCOMPtr<nsISizeOfEventTarget> iSizeOf = do_QueryObject(et)) {
       aWindowSizes.mDOMEventTargetsSize +=
         iSizeOf->SizeOfEventTargetIncludingThis(
           aWindowSizes.mState.mMallocSizeOf);
     }
     if (EventListenerManager* elm = et->GetExistingListenerManager()) {
       aWindowSizes.mDOMEventListenersCount += elm->ListenerCount();
     }
     ++aWindowSizes.mDOMEventTargetsCount;
-  }
+  });
 
   if (mPerformance) {
     aWindowSizes.mDOMPerformanceUserEntries =
       mPerformance->SizeOfUserEntries(aWindowSizes.mState.mMallocSizeOf);
     aWindowSizes.mDOMPerformanceResourceEntries =
       mPerformance->SizeOfResourceEntries(aWindowSizes.mState.mMallocSizeOf);
   }
 
--- a/dom/base/nsGlobalWindowInner.h
+++ b/dom/base/nsGlobalWindowInner.h
@@ -4,17 +4,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsGlobalWindowInner_h___
 #define nsGlobalWindowInner_h___
 
 #include "nsPIDOMWindow.h"
 
-#include "nsTHashtable.h"
 #include "nsHashKeys.h"
 #include "nsRefPtrHashtable.h"
 #include "nsInterfaceHashtable.h"
 
 // Local Includes
 // Helper Classes
 #include "nsCOMPtr.h"
 #include "nsAutoPtr.h"
@@ -87,17 +86,16 @@ struct nsRect;
 class nsWindowSizes;
 
 class IdleRequestExecutor;
 
 class DialogValueHolder;
 
 namespace mozilla {
 class AbstractThread;
-class DOMEventTargetHelper;
 class ThrottledEventQueue;
 namespace dom {
 class BarProp;
 struct ChannelPixelLayout;
 class ClientSource;
 class Console;
 class Crypto;
 class CustomElementRegistry;
@@ -512,20 +510,16 @@ public:
   }
 
   virtual uint32_t GetSerial() override {
     return mSerial;
   }
 
   void AddSizeOfIncludingThis(nsWindowSizes& aWindowSizes) const;
 
-  // Inner windows only.
-  void AddEventTargetObject(mozilla::DOMEventTargetHelper* aObject);
-  void RemoveEventTargetObject(mozilla::DOMEventTargetHelper* aObject);
-
   void NotifyIdleObserver(IdleObserverHolder* aIdleObserverHolder,
                           bool aCallOnidle);
   nsresult HandleIdleActiveEvent();
   bool ContainsIdleObserver(nsIIdleObserver* aIdleObserver, uint32_t timeInS);
   void HandleIdleObserverCallback();
 
   enum SlowScriptResponse {
     ContinueSlowScript = 0,
@@ -1254,18 +1248,16 @@ protected:
                       JS::Handle<JS::Value> aTransfer,
                       nsIPrincipal& aSubjectPrincipal,
                       mozilla::ErrorResult& aError);
 
 private:
   // Fire the JS engine's onNewGlobalObject hook.  Only used on inner windows.
   void FireOnNewGlobalObject();
 
-  void DisconnectEventTargetObjects();
-
   // nsPIDOMWindow{Inner,Outer} should be able to see these helper methods.
   friend class nsPIDOMWindowInner;
   friend class nsPIDOMWindowOuter;
 
   mozilla::dom::TabGroup* TabGroupInner();
 
   bool IsBackgroundInternal() const;
 
@@ -1423,18 +1415,16 @@ protected:
   // dom.successive_dialog_time_limit, we show a checkbox or confirmation prompt
   // to allow disabling of further dialogs from this window.
   TimeStamp                     mLastDialogQuitTime;
 
   // This flag keeps track of whether dialogs are
   // currently enabled on this window.
   bool                          mAreDialogsEnabled;
 
-  nsTHashtable<nsPtrHashKey<mozilla::DOMEventTargetHelper> > mEventTargetObjects;
-
   nsTArray<uint32_t> mEnabledSensors;
 
 #if defined(MOZ_WIDGET_ANDROID)
   mozilla::UniquePtr<mozilla::dom::WindowOrientationObserver> mOrientationChangeObserver;
 #endif
 
 #ifdef MOZ_WEBSPEECH
   RefPtr<mozilla::dom::SpeechSynthesis> mSpeechSynthesis;
--- a/dom/base/nsIGlobalObject.cpp
+++ b/dom/base/nsIGlobalObject.cpp
@@ -6,24 +6,28 @@
 
 #include "nsIGlobalObject.h"
 
 #include "mozilla/dom/ServiceWorker.h"
 #include "nsContentUtils.h"
 #include "nsThreadUtils.h"
 #include "nsHostObjectProtocolHandler.h"
 
+using mozilla::MallocSizeOf;
 using mozilla::Maybe;
+using mozilla::DOMEventTargetHelper;
 using mozilla::dom::ClientInfo;
 using mozilla::dom::ServiceWorker;
 using mozilla::dom::ServiceWorkerDescriptor;
 
 nsIGlobalObject::~nsIGlobalObject()
 {
   UnlinkHostObjectURIs();
+  DisconnectEventTargetObjects();
+  MOZ_DIAGNOSTIC_ASSERT(mEventTargetObjects.IsEmpty());
 }
 
 nsIPrincipal*
 nsIGlobalObject::PrincipalOrNull()
 {
   JSObject *global = GetGlobalJSObject();
   if (NS_WARN_IF(!global))
     return nullptr;
@@ -115,16 +119,61 @@ nsIGlobalObject::TraverseHostObjectURIs(
     return;
   }
 
   for (uint32_t index = 0; index < mHostObjectURIs.Length(); ++index) {
     nsHostObjectProtocolHandler::Traverse(mHostObjectURIs[index], aCb);
   }
 }
 
+void
+nsIGlobalObject::AddEventTargetObject(DOMEventTargetHelper* aObject)
+{
+  MOZ_DIAGNOSTIC_ASSERT(aObject);
+  MOZ_ASSERT(!mEventTargetObjects.Contains(aObject));
+  mEventTargetObjects.PutEntry(aObject);
+}
+
+void
+nsIGlobalObject::RemoveEventTargetObject(DOMEventTargetHelper* aObject)
+{
+  MOZ_DIAGNOSTIC_ASSERT(aObject);
+  MOZ_ASSERT(mEventTargetObjects.Contains(aObject));
+  mEventTargetObjects.RemoveEntry(aObject);
+}
+
+void
+nsIGlobalObject::ForEachEventTargetObject(const std::function<void(DOMEventTargetHelper*)>& aFunc) const
+{
+  // Protect against the function call triggering a mutation of the hash table
+  // while we are iterating by copying the DETH references to a temporary
+  // list.
+  AutoTArray<DOMEventTargetHelper*, 64> targetList;
+  for (auto iter = mEventTargetObjects.ConstIter(); !iter.Done(); iter.Next()) {
+    targetList.AppendElement(iter.Get()->GetKey());
+  }
+
+  // Iterate the target list and call the function on each one.
+  for (auto target : targetList) {
+    aFunc(target);
+  }
+}
+
+void
+nsIGlobalObject::DisconnectEventTargetObjects()
+{
+  ForEachEventTargetObject([&] (DOMEventTargetHelper* aTarget) {
+    aTarget->DisconnectFromOwner();
+
+    // Calling DisconnectFromOwner() should result in
+    // RemoveEventTargetObject() being called.
+    MOZ_DIAGNOSTIC_ASSERT(!mEventTargetObjects.Contains(aTarget));
+  });
+}
+
 Maybe<ClientInfo>
 nsIGlobalObject::GetClientInfo() const
 {
   // By default globals do not expose themselves as a client.  Only real
   // window and worker globals are currently considered clients.
   return Maybe<ClientInfo>();
 }
 
@@ -149,8 +198,16 @@ nsIGlobalObject::AddServiceWorker(Servic
   MOZ_DIAGNOSTIC_ASSERT(false, "this global should not have any service workers");
 }
 
 void
 nsIGlobalObject::RemoveServiceWorker(ServiceWorker* aServiceWorker)
 {
   MOZ_DIAGNOSTIC_ASSERT(false, "this global should not have any service workers");
 }
+
+size_t
+nsIGlobalObject::ShallowSizeOfExcludingThis(MallocSizeOf aSizeOf) const
+{
+  size_t rtn = mHostObjectURIs.ShallowSizeOfExcludingThis(aSizeOf);
+  rtn += mEventTargetObjects.ShallowSizeOfExcludingThis(aSizeOf);
+  return rtn;
+}
--- a/dom/base/nsIGlobalObject.h
+++ b/dom/base/nsIGlobalObject.h
@@ -6,39 +6,49 @@
 
 #ifndef nsIGlobalObject_h__
 #define nsIGlobalObject_h__
 
 #include "mozilla/Maybe.h"
 #include "mozilla/dom/ClientInfo.h"
 #include "mozilla/dom/DispatcherTrait.h"
 #include "mozilla/dom/ServiceWorkerDescriptor.h"
+#include "nsHashKeys.h"
 #include "nsISupports.h"
 #include "nsStringFwd.h"
 #include "nsTArray.h"
+#include "nsTHashtable.h"
 #include "js/TypeDecls.h"
 
 // Must be kept in sync with xpcom/rust/xpcom/src/interfaces/nonidl.rs
 #define NS_IGLOBALOBJECT_IID \
 { 0x11afa8be, 0xd997, 0x4e07, \
 { 0xa6, 0xa3, 0x6f, 0x87, 0x2e, 0xc3, 0xee, 0x7f } }
 
 class nsCycleCollectionTraversalCallback;
 class nsIPrincipal;
 
 namespace mozilla {
+class DOMEventTargetHelper;
 namespace dom {
 class ServiceWorker;
 } // namespace dom
 } // namespace mozilla
 
 class nsIGlobalObject : public nsISupports,
                         public mozilla::dom::DispatcherTrait
 {
   nsTArray<nsCString> mHostObjectURIs;
+
+  // Raw pointers to bound DETH objects.  These are added by
+  // AddEventTargetObject().  The DETH object must call
+  // RemoveEventTargetObject() before its destroyed to clear
+  // its raw pointer here.
+  nsTHashtable<nsPtrHashKey<mozilla::DOMEventTargetHelper>> mEventTargetObjects;
+
   bool mIsDying;
 
 protected:
   nsIGlobalObject()
    : mIsDying(false)
   {}
 
 public:
@@ -79,16 +89,28 @@ public:
 
   void UnregisterHostObjectURI(const nsACString& aURI);
 
   // Any CC class inheriting nsIGlobalObject should call these 2 methods if it
   // exposes the URL API.
   void UnlinkHostObjectURIs();
   void TraverseHostObjectURIs(nsCycleCollectionTraversalCallback &aCb);
 
+  // DETH objects must register themselves on the global when they
+  // bind to it in order to get the DisconnectFromOwner() method
+  // called correctly.  RemoveEventTargetObject() must be called
+  // before the DETH object is destroyed.
+  void AddEventTargetObject(mozilla::DOMEventTargetHelper* aObject);
+  void RemoveEventTargetObject(mozilla::DOMEventTargetHelper* aObject);
+
+  // Iterate the registered DETH objects and call the given function
+  // for each one.
+  void
+  ForEachEventTargetObject(const std::function<void(mozilla::DOMEventTargetHelper*)>& aFunc) const;
+
   virtual bool IsInSyncOperation() { return false; }
 
   virtual mozilla::Maybe<mozilla::dom::ClientInfo>
   GetClientInfo() const;
 
   virtual mozilla::Maybe<mozilla::dom::ServiceWorkerDescriptor>
   GetController() const;
 
@@ -110,14 +132,20 @@ public:
 protected:
   virtual ~nsIGlobalObject();
 
   void
   StartDying()
   {
     mIsDying = true;
   }
+
+  void
+  DisconnectEventTargetObjects();
+
+  size_t
+  ShallowSizeOfExcludingThis(mozilla::MallocSizeOf aSizeOf) const;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIGlobalObject,
                               NS_IGLOBALOBJECT_IID)
 
 #endif // nsIGlobalObject_h__
--- a/dom/events/DOMEventTargetHelper.cpp
+++ b/dom/events/DOMEventTargetHelper.cpp
@@ -84,18 +84,18 @@ NS_INTERFACE_MAP_END
 NS_IMPL_CYCLE_COLLECTING_ADDREF(DOMEventTargetHelper)
 NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(DOMEventTargetHelper,
                                                    LastRelease())
 
 NS_IMPL_DOMTARGET_DEFAULTS(DOMEventTargetHelper)
 
 DOMEventTargetHelper::~DOMEventTargetHelper()
 {
-  if (nsPIDOMWindowInner* owner = GetOwner()) {
-    nsGlobalWindowInner::Cast(owner)->RemoveEventTargetObject(this);
+  if (mParentObject) {
+    mParentObject->RemoveEventTargetObject(this);
   }
   if (mListenerManager) {
     mListenerManager->Disconnect();
   }
   ReleaseWrapper(this);
 }
 
 void
@@ -103,64 +103,66 @@ DOMEventTargetHelper::BindToOwner(nsPIDO
 {
   nsCOMPtr<nsIGlobalObject> glob = do_QueryInterface(aOwner);
   BindToOwner(glob);
 }
 
 void
 DOMEventTargetHelper::BindToOwner(nsIGlobalObject* aOwner)
 {
-  nsCOMPtr<nsIGlobalObject> parentObject = do_QueryReferent(mParentObject);
-  if (parentObject) {
+  if (mParentObject) {
+    mParentObject->RemoveEventTargetObject(this);
     if (mOwnerWindow) {
-      nsGlobalWindowInner::Cast(mOwnerWindow)->RemoveEventTargetObject(this);
       mOwnerWindow = nullptr;
     }
     mParentObject = nullptr;
     mHasOrHasHadOwnerWindow = false;
   }
   if (aOwner) {
-    mParentObject = do_GetWeakReference(aOwner);
-    MOZ_ASSERT(mParentObject, "All nsIGlobalObjects must support nsISupportsWeakReference");
+    mParentObject = aOwner;
+    aOwner->AddEventTargetObject(this);
     // Let's cache the result of this QI for fast access and off main thread usage
     mOwnerWindow = nsCOMPtr<nsPIDOMWindowInner>(do_QueryInterface(aOwner)).get();
     if (mOwnerWindow) {
       mHasOrHasHadOwnerWindow = true;
-      nsGlobalWindowInner::Cast(mOwnerWindow)->AddEventTargetObject(this);
     }
   }
 }
 
 void
 DOMEventTargetHelper::BindToOwner(DOMEventTargetHelper* aOther)
 {
+  if (mParentObject) {
+    mParentObject->RemoveEventTargetObject(this);
+  }
   if (mOwnerWindow) {
-    nsGlobalWindowInner::Cast(mOwnerWindow)->RemoveEventTargetObject(this);
     mOwnerWindow = nullptr;
     mParentObject = nullptr;
     mHasOrHasHadOwnerWindow = false;
   }
   if (aOther) {
     mHasOrHasHadOwnerWindow = aOther->HasOrHasHadOwner();
-    if (aOther->GetParentObject()) {
-      mParentObject = do_GetWeakReference(aOther->GetParentObject());
-      MOZ_ASSERT(mParentObject, "All nsIGlobalObjects must support nsISupportsWeakReference");
+    mParentObject = aOther->GetParentObject();
+    if (mParentObject) {
+      mParentObject->AddEventTargetObject(this);
       // Let's cache the result of this QI for fast access and off main thread usage
-      mOwnerWindow = nsCOMPtr<nsPIDOMWindowInner>(do_QueryInterface(aOther->GetParentObject())).get();
+      mOwnerWindow = nsCOMPtr<nsPIDOMWindowInner>(do_QueryInterface(mParentObject)).get();
       if (mOwnerWindow) {
         mHasOrHasHadOwnerWindow = true;
-        nsGlobalWindowInner::Cast(mOwnerWindow)->AddEventTargetObject(this);
       }
     }
   }
 }
 
 void
 DOMEventTargetHelper::DisconnectFromOwner()
 {
+  if (mParentObject) {
+    mParentObject->RemoveEventTargetObject(this);
+  }
   mOwnerWindow = nullptr;
   mParentObject = nullptr;
   // Event listeners can't be handled anymore, so we can release them here.
   if (mListenerManager) {
     mListenerManager->Disconnect();
     mListenerManager = nullptr;
   }
 
--- a/dom/events/DOMEventTargetHelper.h
+++ b/dom/events/DOMEventTargetHelper.h
@@ -141,18 +141,17 @@ public:
   nsIDocument* GetDocumentIfCurrent() const;
   void BindToOwner(nsIGlobalObject* aOwner);
   void BindToOwner(nsPIDOMWindowInner* aOwner);
   void BindToOwner(DOMEventTargetHelper* aOther);
   virtual void DisconnectFromOwner();
   using EventTarget::GetParentObject;
   virtual nsIGlobalObject* GetOwnerGlobal() const override
   {
-    nsCOMPtr<nsIGlobalObject> parentObject = do_QueryReferent(mParentObject);
-    return parentObject;
+    return mParentObject;
   }
   bool HasOrHasHadOwner() { return mHasOrHasHadOwnerWindow; }
 
   virtual void EventListenerAdded(nsAtom* aType) override;
   virtual void EventListenerAdded(const nsAString& aType) override;
 
   virtual void EventListenerRemoved(nsAtom* aType) override;
   virtual void EventListenerRemoved(const nsAString& aType) override;
@@ -190,18 +189,19 @@ protected:
 
   void KeepAliveIfHasListenersFor(const nsAString& aType);
   void KeepAliveIfHasListenersFor(nsAtom* aType);
 
   void IgnoreKeepAliveIfHasListenersFor(const nsAString& aType);
   void IgnoreKeepAliveIfHasListenersFor(nsAtom* aType);
 
 private:
-  // Inner window or sandbox.
-  nsWeakPtr                  mParentObject;
+  // The parent global object.  The global will clear this when
+  // it is destroyed by calling DisconnectFromOwner().
+  nsIGlobalObject* MOZ_NON_OWNING_REF mParentObject;
   // mParentObject pre QI-ed and cached (inner window)
   // (it is needed for off main thread access)
   // It is obtained in BindToOwner and reset in DisconnectFromOwner.
   nsPIDOMWindowInner* MOZ_NON_OWNING_REF mOwnerWindow;
   bool                       mHasOrHasHadOwnerWindow;
 
   struct {
     nsTArray<nsString> mStrings;