Bug 1516425 - Hold a strong reference to TabParent from nsFrameLoader and make TabParent cycle collected. r=smaug a=pascalc
authorRyan Hunt <rhunt@eqrion.net>
Tue, 02 Apr 2019 10:35:51 -0400
changeset 526031 fceed60b10ce9ca9b022b73d769f6a070c859339
parent 526030 d8cb02604219e560b7d15f73667e3f0015ae02ea
child 526032 650cabd8d8a060bf3bf37daadbdccd4dea585755
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, pascalc
bugs1516425
milestone67.0
Bug 1516425 - Hold a strong reference to TabParent from nsFrameLoader and make TabParent cycle collected. r=smaug a=pascalc Currently TabParent is refcounted, but nsFrameLoader hold a weak pointer. The pointer should be cleared out when the TabParent is destroyed, but that's a bit of a footgun and it's not obvious that we always do this correctly. Because TabParent holds a reference to a nsFrameLoader and the frame element (which contains a nsFrameLoader), I think this means we need to cycle collect TabParent.
dom/base/nsFrameLoader.cpp
dom/base/nsFrameLoader.h
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -147,17 +147,17 @@ typedef ScrollableLayerGuid::ViewID View
 // keep number of shells to a reasonable number on accidental recursion with a
 // small (but not 1) branching factor.  With large branching factors the number
 // of shells can rapidly become huge and run us out of memory.  To solve that,
 // we'd need to re-institute a fixed version of bug 98158.
 #define MAX_DEPTH_CONTENT_FRAMES 10
 
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(nsFrameLoader, mDocShell, mMessageManager,
                                       mChildMessageManager, mOpener,
-                                      mParentSHistory)
+                                      mParentSHistory, mRemoteBrowser)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsFrameLoader)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsFrameLoader)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsFrameLoader)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY_CONCRETE(nsFrameLoader)
   NS_INTERFACE_MAP_ENTRY(nsIMutationObserver)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
--- a/dom/base/nsFrameLoader.h
+++ b/dom/base/nsFrameLoader.h
@@ -462,17 +462,17 @@ class nsFrameLoader final : public nsStu
   // presentation is detached and stored in mDetachedSubdocFrame. This
   // enables us to detect whether the frame has moved documents during
   // a reframe, so that we know not to restore the presentation.
   RefPtr<Document> mContainerDocWhileDetached;
 
   // An opener window which should be used when the docshell is created.
   nsCOMPtr<nsPIDOMWindowOuter> mOpener;
 
-  TabParent* mRemoteBrowser;
+  RefPtr<TabParent> mRemoteBrowser;
   uint64_t mChildID;
 
   // This is used when this refers to a remote sub frame
   RefPtr<mozilla::dom::BrowserBridgeChild> mBrowserBridgeChild;
 
   // Holds the last known size of the frame.
   mozilla::ScreenIntSize mLazySize;
 
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -140,18 +140,25 @@ LazyLogModule gBrowserFocusLog("BrowserF
 // from the ones registered by webProgressListeners.
 #define NOTIFY_FLAG_SHIFT 16
 
 namespace mozilla {
 namespace dom {
 
 TabParent::LayerToTabParentTable* TabParent::sLayerToTabParentTable = nullptr;
 
-NS_IMPL_ISUPPORTS(TabParent, nsITabParent, nsIAuthPromptProvider,
-                  nsISupportsWeakReference)
+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(TabParent)
+  NS_INTERFACE_MAP_ENTRY(nsITabParent)
+  NS_INTERFACE_MAP_ENTRY(nsIAuthPromptProvider)
+  NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsITabParent)
+NS_INTERFACE_MAP_END
+NS_IMPL_CYCLE_COLLECTION(TabParent, mFrameElement, mBrowserDOMWindow, mLoadContext, mFrameLoader, mBrowsingContext)
+NS_IMPL_CYCLE_COLLECTING_ADDREF(TabParent)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(TabParent)
 
 TabParent::TabParent(ContentParent* aManager, const TabId& aTabId,
                      const TabContext& aContext,
                      CanonicalBrowsingContext* aBrowsingContext,
                      uint32_t aChromeFlags,
                      BrowserBridgeParent* aBrowserBridgeParent)
     : TabContext(aContext),
       mFrameElement(nullptr),
@@ -1930,17 +1937,17 @@ mozilla::ipc::IPCResult TabParent::RecvR
 
   if (!mFrameElement || !mFrameElement->OwnerDoc()) {
     return IPC_OK();
   }
 
   uint32_t flags = nsIFocusManager::FLAG_NOSCROLL;
   if (aCanRaise) flags |= nsIFocusManager::FLAG_RAISE;
 
-  RefPtr<Element> element = mFrameElement;
+  nsCOMPtr<Element> element = mFrameElement;
   fm->SetFocus(element, flags);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult TabParent::RecvEnableDisableCommands(
     const nsString& aAction, nsTArray<nsCString>&& aEnabledCommands,
     nsTArray<nsCString>&& aDisabledCommands) {
   nsCOMPtr<nsIBrowser> browser =
@@ -2538,17 +2545,18 @@ already_AddRefed<nsFrameLoader> TabParen
   if (mIsDestroyed && !aUseCachedFrameLoaderAfterDestroy) {
     return nullptr;
   }
 
   if (mFrameLoader) {
     RefPtr<nsFrameLoader> fl = mFrameLoader;
     return fl.forget();
   }
-  RefPtr<nsFrameLoaderOwner> frameLoaderOwner = do_QueryObject(mFrameElement);
+  nsCOMPtr<Element> frameElement(mFrameElement);
+  RefPtr<nsFrameLoaderOwner> frameLoaderOwner = do_QueryObject(frameElement);
   return frameLoaderOwner ? frameLoaderOwner->GetFrameLoader() : nullptr;
 }
 
 void TabParent::TryCacheDPIAndScale() {
   if (mDPI > 0) {
     return;
   }
 
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -92,21 +92,25 @@ class TabParent final : public PBrowserP
   friend class BrowserBridgeParent;  // for clearing mBrowserBridgeParent
 
   virtual ~TabParent();
 
  public:
   // Helper class for ContentParent::RecvCreateWindow.
   struct AutoUseNewTab;
 
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+  NS_DECL_NSIAUTHPROMPTPROVIDER
   // nsITabParent
   NS_DECL_NSITABPARENT
   // nsIDOMEventListener interfaces
   NS_DECL_NSIDOMEVENTLISTENER
 
+  NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(TabParent, nsITabParent)
+
   TabParent(ContentParent* aManager, const TabId& aTabId,
             const TabContext& aContext,
             CanonicalBrowsingContext* aBrowsingContext, uint32_t aChromeFlags,
             BrowserBridgeParent* aBrowserBridgeParent = nullptr);
 
   Element* GetOwnerElement() const { return mFrameElement; }
   already_AddRefed<nsPIDOMWindowOuter> GetParentWindowOuter();
 
@@ -453,19 +457,16 @@ class TabParent final : public PBrowserP
       PIndexedDBPermissionRequestParent* aActor,
       const Principal& aPrincipal) override;
 
   bool DeallocPIndexedDBPermissionRequestParent(
       PIndexedDBPermissionRequestParent* aActor);
 
   bool GetGlobalJSObject(JSContext* cx, JSObject** globalp);
 
-  NS_DECL_ISUPPORTS
-  NS_DECL_NSIAUTHPROMPTPROVIDER
-
   void StartPersistence(uint64_t aOuterWindowID,
                         nsIWebBrowserPersistDocumentReceiver* aRecv,
                         ErrorResult& aRv);
 
   bool HandleQueryContentEvent(mozilla::WidgetQueryContentEvent& aEvent);
 
   bool SendPasteTransferable(const IPCDataTransfer& aDataTransfer,
                              const bool& aIsPrivateData,
@@ -606,17 +607,17 @@ class TabParent final : public PBrowserP
   mozilla::ipc::IPCResult RecvAsyncAuthPrompt(const nsCString& aUri,
                                               const nsString& aRealm,
                                               const uint64_t& aCallbackId);
 
   virtual mozilla::ipc::IPCResult Recv__delete__() override;
 
   virtual void ActorDestroy(ActorDestroyReason why) override;
 
-  Element* mFrameElement;
+  nsCOMPtr<Element> mFrameElement;
   nsCOMPtr<nsIBrowserDOMWindow> mBrowserDOMWindow;
 
   mozilla::ipc::IPCResult RecvRemotePaintIsReady();
 
   mozilla::ipc::IPCResult RecvNotifyCompositorTransaction();
 
   mozilla::ipc::IPCResult RecvRemoteIsReadyToHandleInputEvents();