Bug 1525720, part 17 - Ignore nsIRemoteTab methods after we have destroyed the browser. r=nika

It's possible for front-end references to nsIRemoteTab to outlive the IPDL actor. When
this happens, we should ignore methods and property accesses.

The one special case is that some code expects to be able to
access the TabId after the browser has been destroyed. For this
we can just cache the ID.

Differential Revision: https://phabricator.services.mozilla.com/D31449

--HG--
extra : rebase_source : 06791db921203a5dfc6cc386e420bfa0de113941
extra : histedit_source : 4617c237d14e01cdbfff66d391069bcdf2267c51

dom/ipc/BrowserHost.cpp

@@ -23,7 +23,8 @@ NS_IMPL_CYCLE_COLLECTION(BrowserHost, mRoot)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(BrowserHost)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(BrowserHost)
 
-BrowserHost::BrowserHost(BrowserParent* aParent) : mRoot(aParent) {
+BrowserHost::BrowserHost(BrowserParent* aParent)
+    : mId(aParent->GetTabId()), mRoot(aParent) {
   mRoot->SetBrowserHost(this);
 }
 
@@ -77,12 +78,19 @@ void BrowserHost::UpdateDimensions(const nsIntRect& aRect,
 /* attribute boolean docShellIsActive; */
 NS_IMETHODIMP
 BrowserHost::GetDocShellIsActive(bool* aDocShellIsActive) {
+  if (!mRoot) {
+    *aDocShellIsActive = false;
+    return NS_OK;
+  }
   *aDocShellIsActive = mRoot->GetDocShellIsActive();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BrowserHost::SetDocShellIsActive(bool aDocShellIsActive) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   VisitAll([&](BrowserParent* aBrowserParent) {
     aBrowserParent->SetDocShellIsActive(aDocShellIsActive);
   });
@@ -92,12 +100,19 @@ BrowserHost::SetDocShellIsActive(bool aDocShellIsActive) {
 /* attribute boolean renderLayers; */
 NS_IMETHODIMP
 BrowserHost::GetRenderLayers(bool* aRenderLayers) {
+  if (!mRoot) {
+    *aRenderLayers = false;
+    return NS_OK;
+  }
   *aRenderLayers = mRoot->GetRenderLayers();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BrowserHost::SetRenderLayers(bool aRenderLayers) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   VisitAll([&](BrowserParent* aBrowserParent) {
     aBrowserParent->SetRenderLayers(aRenderLayers);
   });
@@ -107,6 +122,10 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) {
 /* readonly attribute boolean hasLayers; */
 NS_IMETHODIMP
 BrowserHost::GetHasLayers(bool* aHasLayers) {
+  if (!mRoot) {
+    *aHasLayers = false;
+    return NS_OK;
+  }
   *aHasLayers = mRoot->GetHasLayers();
   return NS_OK;
 }
@@ -114,6 +133,9 @@ BrowserHost::GetHasLayers(bool* aHasLayers) {
 /* void forceRepaint (); */
 NS_IMETHODIMP
 BrowserHost::ForceRepaint(void) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   VisitAll(
       [](BrowserParent* aBrowserParent) { aBrowserParent->ForceRepaint(); });
   return NS_OK;
@@ -122,6 +144,9 @@ BrowserHost::ForceRepaint(void) {
 /* void resolutionChanged (); */
 NS_IMETHODIMP
 BrowserHost::NotifyResolutionChanged(void) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   VisitAll([](BrowserParent* aBrowserParent) {
     aBrowserParent->NotifyResolutionChanged();
   });
@@ -131,6 +156,9 @@ BrowserHost::NotifyResolutionChanged(void) {
 /* void deprioritize (); */
 NS_IMETHODIMP
 BrowserHost::Deprioritize(void) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   VisitAll(
       [](BrowserParent* aBrowserParent) { aBrowserParent->Deprioritize(); });
   return NS_OK;
@@ -139,6 +167,9 @@ BrowserHost::Deprioritize(void) {
 /* void preserveLayers (in boolean aPreserveLayers); */
 NS_IMETHODIMP
 BrowserHost::PreserveLayers(bool aPreserveLayers) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   VisitAll([&](BrowserParent* aBrowserParent) {
     aBrowserParent->PreserveLayers(aPreserveLayers);
   });
@@ -148,13 +179,17 @@ BrowserHost::PreserveLayers(bool aPreserveLayers) {
 /* readonly attribute uint64_t tabId; */
 NS_IMETHODIMP
 BrowserHost::GetTabId(uint64_t* aTabId) {
-  *aTabId = mRoot->GetTabId();
+  *aTabId = mId;
   return NS_OK;
 }
 
 /* readonly attribute uint64_t contentProcessId; */
 NS_IMETHODIMP
 BrowserHost::GetContentProcessId(uint64_t* aContentProcessId) {
+  if (!mRoot) {
+    *aContentProcessId = 0;
+    return NS_OK;
+  }
   *aContentProcessId = GetContentParent()->ChildID();
   return NS_OK;
 }
@@ -162,6 +197,10 @@ BrowserHost::GetContentProcessId(uint64_t* aContentProcessId) {
 /* readonly attribute int32_t osPid; */
 NS_IMETHODIMP
 BrowserHost::GetOsPid(int32_t* aOsPid) {
+  if (!mRoot) {
+    *aOsPid = 0;
+    return NS_OK;
+  }
   *aOsPid = GetContentParent()->Pid();
   return NS_OK;
 }
@@ -169,6 +208,10 @@ BrowserHost::GetOsPid(int32_t* aOsPid) {
 /* readonly attribute boolean hasContentOpener; */
 NS_IMETHODIMP
 BrowserHost::GetHasContentOpener(bool* aHasContentOpener) {
+  if (!mRoot) {
+    *aHasContentOpener = false;
+    return NS_OK;
+  }
   *aHasContentOpener = mRoot->GetHasContentOpener();
   return NS_OK;
 }
@@ -176,6 +219,10 @@ BrowserHost::GetHasContentOpener(bool* aHasContentOpener) {
 /* readonly attribute boolean hasPresented; */
 NS_IMETHODIMP
 BrowserHost::GetHasPresented(bool* aHasPresented) {
+  if (!mRoot) {
+    *aHasPresented = false;
+    return NS_OK;
+  }
   *aHasPresented = mRoot->GetHasPresented();
   return NS_OK;
 }
@@ -183,6 +230,10 @@ BrowserHost::GetHasPresented(bool* aHasPresented) {
 NS_IMETHODIMP
 BrowserHost::GetWindowGlobalParents(
     nsTArray<RefPtr<WindowGlobalParent>>& aWindowGlobalParents) {
+  if (!mRoot) {
+    aWindowGlobalParents = nsTArray<RefPtr<WindowGlobalParent>>();
+    return NS_OK;
+  }
   VisitAll([&](BrowserParent* aBrowser) {
     const auto& windowGlobalParents = aBrowser->ManagedPWindowGlobalParent();
     for (auto iter = windowGlobalParents.ConstIter(); !iter.Done();
@@ -198,12 +249,19 @@ BrowserHost::GetWindowGlobalParents(
 /* void transmitPermissionsForPrincipal (in nsIPrincipal aPrincipal); */
 NS_IMETHODIMP
 BrowserHost::TransmitPermissionsForPrincipal(nsIPrincipal* aPrincipal) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   return GetContentParent()->TransmitPermissionsForPrincipal(aPrincipal);
 }
 
 /* readonly attribute boolean hasBeforeUnload; */
 NS_IMETHODIMP
 BrowserHost::GetHasBeforeUnload(bool* aHasBeforeUnload) {
+  if (!mRoot) {
+    *aHasBeforeUnload = false;
+    return NS_OK;
+  }
   *aHasBeforeUnload = mRoot->GetHasBeforeUnload();
   return NS_OK;
 }
@@ -211,6 +269,10 @@ BrowserHost::GetHasBeforeUnload(bool* aHasBeforeUnload) {
 /* readonly attribute Element ownerElement; */
 NS_IMETHODIMP
 BrowserHost::GetOwnerElement(mozilla::dom::Element** aOwnerElement) {
+  if (!mRoot) {
+    *aOwnerElement = nullptr;
+    return NS_OK;
+  }
   *aOwnerElement = mRoot->GetOwnerElement();
   return NS_OK;
 }
@@ -221,6 +283,9 @@ NS_IMETHODIMP
 BrowserHost::StartApzAutoscroll(float aAnchorX, float aAnchorY,
                                 nsViewID aScrollId, uint32_t aPresShellId,
                                 bool* _retval) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   *_retval =
       mRoot->StartApzAutoscroll(aAnchorX, aAnchorY, aScrollId, aPresShellId);
   return NS_OK;
@@ -229,6 +294,9 @@ BrowserHost::StartApzAutoscroll(float aAnchorX, float aAnchorY,
 /* void stopApzAutoscroll (in nsViewID aScrollId, in uint32_t aPresShellId); */
 NS_IMETHODIMP
 BrowserHost::StopApzAutoscroll(nsViewID aScrollId, uint32_t aPresShellId) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   mRoot->StopApzAutoscroll(aScrollId, aPresShellId);
   return NS_OK;
 }
@@ -236,6 +304,9 @@ BrowserHost::StopApzAutoscroll(nsViewID aScrollId, uint32_t aPresShellId) {
 /* bool saveRecording (in AString aFileName); */
 NS_IMETHODIMP
 BrowserHost::SaveRecording(const nsAString& aFileName, bool* _retval) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   nsCOMPtr<nsIFile> file;
   nsresult rv = NS_NewLocalFile(aFileName, false, getter_AddRefs(file));
   if (NS_FAILED(rv)) {
@@ -247,6 +318,10 @@ BrowserHost::SaveRecording(const nsAString& aFileName, bool* _retval) {
 /* Promise getContentBlockingLog (); */
 NS_IMETHODIMP
 BrowserHost::GetContentBlockingLog(::mozilla::dom::Promise** aPromise) {
+  if (!mRoot) {
+    *aPromise = nullptr;
+    return NS_OK;
+  }
   NS_ENSURE_ARG_POINTER(aPromise);
 
   *aPromise = nullptr;
@@ -286,6 +361,9 @@ NS_IMETHODIMP
 BrowserHost::MaybeCancelContentJSExecutionFromScript(
     nsIRemoteTab::NavigationType aNavigationType,
     JS::Handle<JS::Value> aCancelContentJSOptions, JSContext* aCx) {
+  if (!mRoot) {
+    return NS_OK;
+  }
   dom::CancelContentJSOptions cancelContentJSOptions;
   if (!cancelContentJSOptions.Init(aCx, aCancelContentJSOptions)) {
     return NS_ERROR_INVALID_ARG;

dom/ipc/BrowserHost.h

@@ -89,6 +89,9 @@ class BrowserHost : public RemoteBrowser,
  private:
   virtual ~BrowserHost() = default;
 
+  // The TabID for the root BrowserParent, we cache this so that we can access
+  // it after the remote browser has been destroyed
+  TabId mId;
   // The root BrowserParent of this remote browser
   RefPtr<BrowserParent> mRoot;
 };