Bug 1536405 - Cycle-collect through ChromeEventHandler. r=bz a=pascalc
authorNika Layzell <nika@thelayzells.com>
Thu, 21 Mar 2019 20:37:06 +0100
changeset 523030 def85b90d5e4d7fd846cbba7c96ee02989abad9d
parent 523029 97b3d94e492d245ce71546550af6a6b6e54917b2
child 523031 1cdd67f491bf9e8de3a8bfa052c72ed6f0e9654f
push id11031
push userarchaeopteryx@coole-files.de
push dateMon, 08 Apr 2019 10:37:16 +0000
treeherdermozilla-beta@fceed60b10ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, pascalc
bugs1536405
milestone67.0
Bug 1536405 - Cycle-collect through ChromeEventHandler. r=bz a=pascalc Differential Revision: https://phabricator.services.mozilla.com//D24982
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -311,17 +311,16 @@ static void DecreasePrivateDocShellCount
 
 nsDocShell::nsDocShell(BrowsingContext* aBrowsingContext)
     : nsDocLoader(),
       mContentWindowID(NextWindowID()),
       mBrowsingContext(aBrowsingContext),
       mForcedCharset(nullptr),
       mParentCharset(nullptr),
       mTreeOwner(nullptr),
-      mChromeEventHandler(nullptr),
       mDefaultScrollbarPref(Scrollbar_Auto, Scrollbar_Auto),
       mCharsetReloadState(eCharsetReloadInit),
       mOrientationLock(hal::eScreenOrientation_None),
       mParentCharsetSource(0),
       mMarginWidth(-1),
       mMarginHeight(-1),
       mItemType(aBrowsingContext->IsContent() ? typeContent : typeChrome),
       mPreviousEntryIndex(-1),
@@ -536,17 +535,17 @@ void nsDocShell::DestroyChildren() {
   }
 
   nsDocLoader::DestroyChildren();
 }
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(nsDocShell, nsDocLoader,
                                    mSessionStorageManager, mScriptGlobal,
                                    mInitialClientSource, mSessionHistory,
-                                   mBrowsingContext)
+                                   mBrowsingContext, mChromeEventHandler)
 
 NS_IMPL_ADDREF_INHERITED(nsDocShell, nsDocLoader)
 NS_IMPL_RELEASE_INHERITED(nsDocShell, nsDocLoader)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsDocShell)
   NS_INTERFACE_MAP_ENTRY(nsIDocShell)
   NS_INTERFACE_MAP_ENTRY(nsIDocShellTreeItem)
   NS_INTERFACE_MAP_ENTRY(nsIWebNavigation)
@@ -1127,30 +1126,29 @@ nsDocShell::GetContentViewer(nsIContentV
 NS_IMETHODIMP
 nsDocShell::GetOuterWindowID(uint64_t* aWindowID) {
   *aWindowID = mContentWindowID;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::SetChromeEventHandler(EventTarget* aChromeEventHandler) {
-  // Weak reference. Don't addref.
   mChromeEventHandler = aChromeEventHandler;
 
   if (mScriptGlobal) {
     mScriptGlobal->SetChromeEventHandler(mChromeEventHandler);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::GetChromeEventHandler(EventTarget** aChromeEventHandler) {
   NS_ENSURE_ARG_POINTER(aChromeEventHandler);
-  nsCOMPtr<EventTarget> handler = mChromeEventHandler;
+  RefPtr<EventTarget> handler = mChromeEventHandler;
   handler.forget(aChromeEventHandler);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::SetCurrentURI(nsIURI* aURI) {
   // Note that securityUI will set STATE_IS_INSECURE, even if
   // the scheme of |aURI| is "https".
@@ -4185,17 +4183,17 @@ nsDocShell::DisplayLoadError(nsresult aE
       Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UI_EVENTS, bucketId);
     }
 
     cssClass.AssignLiteral("blacklist");
   } else if (NS_ERROR_CONTENT_CRASHED == aError) {
     errorPage.AssignLiteral("tabcrashed");
     error = "tabcrashed";
 
-    nsCOMPtr<EventTarget> handler = mChromeEventHandler;
+    RefPtr<EventTarget> handler = mChromeEventHandler;
     if (handler) {
       nsCOMPtr<Element> element = do_QueryInterface(handler);
       element->GetAttribute(NS_LITERAL_STRING("crashedPageTitle"), messageStr);
     }
 
     // DisplayLoadError requires a non-empty messageStr to proceed and call
     // LoadErrorPage. If the page doesn't have a title, we will use a blank
     // space which will be trimmed and thus treated as empty by the front-end.
@@ -4979,16 +4977,18 @@ nsDocShell::Destroy() {
   }
 
   mBrowsingContext->Detach();
 
   SetTreeOwner(nullptr);
 
   mTabChild = nullptr;
 
+  mChromeEventHandler = nullptr;
+
   mOnePermittedSandboxedNavigator = nullptr;
 
   // required to break ref cycle
   mSecurityUI = nullptr;
 
   // Cancel any timers that were set for this docshell; this is needed
   // to break the cycle between us and the timers.
   CancelRefreshURITimers();
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -1031,18 +1031,19 @@ class nsDocShell final : public nsDocLoa
 
   const mozilla::Encoding* mForcedCharset;
   const mozilla::Encoding* mParentCharset;
 
   // WEAK REFERENCES BELOW HERE.
   // Note these are intentionally not addrefd. Doing so will create a cycle.
   // For that reasons don't use nsCOMPtr.
 
-  nsIDocShellTreeOwner* mTreeOwner;                // Weak Reference
-  mozilla::dom::EventTarget* mChromeEventHandler;  // Weak Reference
+  nsIDocShellTreeOwner* mTreeOwner;  // Weak Reference
+
+  RefPtr<mozilla::dom::EventTarget> mChromeEventHandler;
 
   nsIntPoint mDefaultScrollbarPref;  // persistent across doc loads
 
   eCharsetReloadState mCharsetReloadState;
 
   mozilla::hal::ScreenOrientation mOrientationLock;
 
   int32_t mParentCharsetSource;