Backed out 3 changesets (bug 1589054) for browser chrome failures on browser_crash_oopiframe.js CLOSED TREE

Backed out changeset 1a43032819e1 (bug 1589054)
Backed out changeset 91e4d5c6422a (bug 1589054)
Backed out changeset 03bc24aa3a2c (bug 1589054)

dom/base/nsFrameLoader.cpp

@@ -1905,7 +1905,6 @@ void nsFrameLoader::DestroyDocShell() {
   }
 
   mBrowsingContext = nullptr;
-  mDocShell = nullptr;
 
   if (mChildMessageManager) {
     // Stop handling events in the in-process frame script.
@@ -2015,7 +2014,6 @@ nsresult nsFrameLoader::MaybeCreateDocShell() {
   // context inside of nsDocShell::Create
   RefPtr<nsDocShell> docShell = nsDocShell::Create(mBrowsingContext);
   NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);
-  mDocShell = docShell;
 
   mBrowsingContext->SetEmbedderElement(mOwnerContent);
 

dom/base/nsFrameLoader.h

@@ -124,7 +124,9 @@ class nsFrameLoader final : public nsStubMutationObserver,
   void StartDestroy();
   void DestroyDocShell();
   void DestroyComplete();
-  nsIDocShell* GetExistingDocShell() const { return mDocShell; }
+  nsIDocShell* GetExistingDocShell() const {
+    return mBrowsingContext ? mBrowsingContext->GetDocShell() : nullptr;
+  }
   mozilla::dom::InProcessBrowserChildMessageManager*
   GetBrowserChildMessageManager() const {
     return mChildMessageManager;
@@ -427,7 +429,10 @@ class nsFrameLoader final : public nsStubMutationObserver,
   nsresult MaybeCreateDocShell();
   nsresult EnsureMessageManager();
   nsresult ReallyLoadFrameScripts();
-  nsDocShell* GetDocShell() const { return mDocShell; }
+  nsDocShell* GetDocShell() const {
+    return mBrowsingContext ? nsDocShell::Cast(mBrowsingContext->GetDocShell())
+                            : nullptr;
+  }
 
   // Updates the subdocument position and size. This gets called only
   // when we have our own in-process DocShell.
@@ -503,7 +508,6 @@ class nsFrameLoader final : public nsStubMutationObserver,
 
   uint64_t mChildID;
   RefPtr<mozilla::dom::RemoteBrowser> mRemoteBrowser;
-  RefPtr<nsDocShell> mDocShell;
 
   // Holds the last known size of the frame.
   mozilla::ScreenIntSize mLazySize;

dom/base/nsFrameLoaderOwner.cpp

@@ -90,42 +90,34 @@ void nsFrameLoaderOwner::ChangeRemotenessCommon(
   doc->BlockOnload();
   auto cleanup = MakeScopeExit([&]() { doc->UnblockOnload(false); });
 
-  {
-    // Introduce a script blocker to ensure no JS is executed during the
-    // nsFrameLoader teardown & recreation process. Unload listeners will be run
-    // for the previous document, and the load will be started for the new one,
-    // at the end of this block.
-    nsAutoScriptBlocker sb;
-
-    // If we already have a Frameloader, destroy it, possibly preserving its
-    // browsing context.
-    if (mFrameLoader) {
-      if (aPreserveContext) {
-        bc = mFrameLoader->GetBrowsingContext();
-        mFrameLoader->SkipBrowsingContextDetach();
-      }
-
-      // Preserve the networkCreated status, as nsDocShells created after a
-      // process swap may shouldn't change their dynamically-created status.
-      networkCreated = mFrameLoader->IsNetworkCreated();
-      mFrameLoader->Destroy();
-      mFrameLoader = nullptr;
+  // If we already have a Frameloader, destroy it, possibly preserving its
+  // browsing context.
+  if (mFrameLoader) {
+    if (aPreserveContext) {
+      bc = mFrameLoader->GetBrowsingContext();
+      mFrameLoader->SkipBrowsingContextDetach();
     }
 
-    mFrameLoader =
-        nsFrameLoader::Recreate(owner, bc, aRemoteType, networkCreated);
-    if (NS_WARN_IF(!mFrameLoader)) {
-      aRv.Throw(NS_ERROR_FAILURE);
-      return;
-    }
+    // Preserve the networkCreated status, as nsDocShells created after a
+    // process swap may shouldn't change their dynamically-created status.
+    networkCreated = mFrameLoader->IsNetworkCreated();
+    mFrameLoader->Destroy();
+    mFrameLoader = nullptr;
+  }
 
-    // Invoke the frame loader initialization callback to perform setup on our
-    // new nsFrameLoader. This may cause our ErrorResult to become errored, so
-    // double-check after calling.
-    aFrameLoaderInit();
-    if (NS_WARN_IF(aRv.Failed())) {
-      return;
-    }
+  mFrameLoader =
+      nsFrameLoader::Recreate(owner, bc, aRemoteType, networkCreated);
+  if (NS_WARN_IF(!mFrameLoader)) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+
+  // Invoke the frame loader initialization callback to perform setup on our new
+  // nsFrameLoader. This may cause our ErrorResult to become errored, so
+  // double-check after calling.
+  aFrameLoaderInit();
+  if (NS_WARN_IF(aRv.Failed())) {
+    return;
   }
 
   // Now that we've got a new FrameLoader, we need to reset our
@@ -190,8 +182,9 @@ void nsFrameLoaderOwner::ChangeRemoteness(
                          aOptions.mRemoteType, frameLoaderInit, rv);
 }
 
-void nsFrameLoaderOwner::ChangeRemotenessWithBridge(BrowserBridgeChild* aBridge,
-                                                    mozilla::ErrorResult& rv) {
+void nsFrameLoaderOwner::ChangeRemotenessWithBridge(
+    mozilla::ipc::ManagedEndpoint<mozilla::dom::PBrowserBridgeChild> aEndpoint,
+    uint64_t aTabId, mozilla::ErrorResult& rv) {
   MOZ_ASSERT(XRE_IsContentProcess());
   if (NS_WARN_IF(!mFrameLoader)) {
     rv.Throw(NS_ERROR_UNEXPECTED);
@@ -199,9 +192,24 @@ void nsFrameLoaderOwner::ChangeRemotenessWithBridge(BrowserBridgeChild* aBridge,
   }
 
   std::function<void()> frameLoaderInit = [&] {
-    RefPtr<BrowserBridgeHost> host = aBridge->FinishInit(mFrameLoader);
-    mFrameLoader->mBrowsingContext->SetEmbedderElement(
-        mFrameLoader->GetOwnerContent());
+    RefPtr<BrowsingContext> browsingContext = mFrameLoader->mBrowsingContext;
+    RefPtr<BrowserBridgeChild> bridge =
+        new BrowserBridgeChild(mFrameLoader, browsingContext, TabId(aTabId));
+    Document* ownerDoc = mFrameLoader->GetOwnerDoc();
+    if (NS_WARN_IF(!ownerDoc)) {
+      rv.Throw(NS_ERROR_UNEXPECTED);
+      return;
+    }
+
+    RefPtr<BrowserChild> browser =
+        BrowserChild::GetFrom(ownerDoc->GetDocShell());
+    if (!browser->BindPBrowserBridgeEndpoint(std::move(aEndpoint), bridge)) {
+      rv.Throw(NS_ERROR_UNEXPECTED);
+      return;
+    }
+
+    RefPtr<BrowserBridgeHost> host = bridge->FinishInit();
+    browsingContext->SetEmbedderElement(mFrameLoader->GetOwnerContent());
     mFrameLoader->mRemoteBrowser = host;
   };
 

dom/base/nsFrameLoaderOwner.h

@@ -14,9 +14,13 @@ namespace mozilla {
 class ErrorResult;
 namespace dom {
 class BrowsingContext;
-class BrowserBridgeChild;
+class PBrowserBridgeChild;
 struct RemotenessOptions;
 }  // namespace dom
+namespace ipc {
+template <typename T>
+class ManagedEndpoint;
+}  // namespace ipc
 }  // namespace mozilla
 
 // IID for the FrameLoaderOwner interface
@@ -53,8 +57,10 @@ class nsFrameLoaderOwner : public nsISupports {
   void ChangeRemoteness(const mozilla::dom::RemotenessOptions& aOptions,
                         mozilla::ErrorResult& rv);
 
-  void ChangeRemotenessWithBridge(mozilla::dom::BrowserBridgeChild* aBridge,
-                                  mozilla::ErrorResult& rv);
+  void ChangeRemotenessWithBridge(
+      mozilla::ipc::ManagedEndpoint<mozilla::dom::PBrowserBridgeChild>
+          aEndpoint,
+      uint64_t aTabId, mozilla::ErrorResult& rv);
 
  private:
   bool UseRemoteSubframes();

dom/ipc/BrowserBridgeChild.cpp

@@ -30,9 +30,13 @@ using namespace mozilla::ipc;
 namespace mozilla {
 namespace dom {
 
-BrowserBridgeChild::BrowserBridgeChild(BrowsingContext* aBrowsingContext,
+BrowserBridgeChild::BrowserBridgeChild(nsFrameLoader* aFrameLoader,
+                                       BrowsingContext* aBrowsingContext,
                                        TabId aId)
-    : mId{aId}, mLayersId{0}, mBrowsingContext(aBrowsingContext) {}
+    : mId{aId},
+      mLayersId{0},
+      mFrameLoader(aFrameLoader),
+      mBrowsingContext(aBrowsingContext) {}
 
 BrowserBridgeChild::~BrowserBridgeChild() {
 #if defined(ACCESSIBILITY) && defined(XP_WIN)
@@ -42,11 +46,7 @@ BrowserBridgeChild::~BrowserBridgeChild() {
 #endif
 }
 
-already_AddRefed<BrowserBridgeHost> BrowserBridgeChild::FinishInit(
-    nsFrameLoader* aFrameLoader) {
-  MOZ_DIAGNOSTIC_ASSERT(!mFrameLoader);
-  mFrameLoader = aFrameLoader;
-
+already_AddRefed<BrowserBridgeHost> BrowserBridgeChild::FinishInit() {
   RefPtr<Element> owner = mFrameLoader->GetOwnerContent();
   nsCOMPtr<nsIDocShell> docShell = do_GetInterface(owner->GetOwnerGlobal());
   MOZ_DIAGNOSTIC_ASSERT(docShell);

dom/ipc/BrowserBridgeChild.h

@@ -56,7 +56,7 @@ class BrowserBridgeChild : public PBrowserBridgeChild {
 
   void SetIsUnderHiddenEmbedderElement(bool aIsUnderHiddenEmbedderElement);
 
-  already_AddRefed<BrowserBridgeHost> FinishInit(nsFrameLoader* aFrameLoader);
+  already_AddRefed<BrowserBridgeHost> FinishInit();
 
 #if defined(ACCESSIBILITY) && defined(XP_WIN)
   a11y::RemoteIframeDocProxyAccessibleWrap* GetEmbeddedDocAccessible() {
@@ -68,7 +68,8 @@ class BrowserBridgeChild : public PBrowserBridgeChild {
 
   static BrowserBridgeChild* GetFrom(nsIContent* aContent);
 
-  BrowserBridgeChild(BrowsingContext* aBrowsingContext, TabId aId);
+  BrowserBridgeChild(nsFrameLoader* aFrameLoader,
+                     BrowsingContext* aBrowsingContext, TabId aId);
 
  protected:
   friend class ContentChild;

dom/ipc/ContentChild.cpp

@@ -2127,13 +2127,13 @@ already_AddRefed<RemoteBrowser> ContentChild::CreateBrowser(
 
   TabId tabId(nsContentUtils::GenerateTabId());
   RefPtr<BrowserBridgeChild> browserBridge =
-      new BrowserBridgeChild(aBrowsingContext, tabId);
+      new BrowserBridgeChild(aFrameLoader, aBrowsingContext, tabId);
 
   browserChild->SendPBrowserBridgeConstructor(
       browserBridge, PromiseFlatString(aContext.PresentationURL()), aRemoteType,
       aBrowsingContext, chromeFlags, tabId);
 
-  return browserBridge->FinishInit(aFrameLoader);
+  return browserBridge->FinishInit();
 }
 
 PScriptCacheChild* ContentChild::AllocPScriptCacheChild(

dom/ipc/WindowGlobalChild.cpp

@@ -281,11 +281,11 @@ mozilla::ipc::IPCResult WindowGlobalChild::RecvMakeFrameLocal(
 
   RefPtr<Element> embedderElt = aFrameContext->GetEmbedderElement();
   if (NS_WARN_IF(!embedderElt)) {
-    return IPC_OK();
+    return IPC_FAIL(this, "No embedder element in this process");
   }
 
   if (NS_WARN_IF(embedderElt->GetOwnerGlobal() != WindowGlobal())) {
-    return IPC_OK();
+    return IPC_FAIL(this, "Wrong actor");
   }
 
   RefPtr<nsFrameLoaderOwner> flo = do_QueryObject(embedderElt);
@@ -311,38 +311,20 @@ mozilla::ipc::IPCResult WindowGlobalChild::RecvMakeFrameRemote(
   // Immediately resolve the promise, acknowledging the request.
   aResolve(true);
 
-  // Immediately construct the BrowserBridgeChild so we can destroy it cleanly
-  // if the process switch fails.
-  RefPtr<BrowserBridgeChild> bridge =
-      new BrowserBridgeChild(aFrameContext, aTabId);
-  RefPtr<BrowserChild> manager = GetBrowserChild();
-  if (NS_WARN_IF(
-          !manager->BindPBrowserBridgeEndpoint(std::move(aEndpoint), bridge))) {
-    return IPC_OK();
-  }
-
   RefPtr<Element> embedderElt = aFrameContext->GetEmbedderElement();
   if (NS_WARN_IF(!embedderElt)) {
-    BrowserBridgeChild::Send__delete__(bridge);
-    return IPC_OK();
+    return IPC_FAIL(this, "No embedder element in this process");
   }
 
   if (NS_WARN_IF(embedderElt->GetOwnerGlobal() != WindowGlobal())) {
-    BrowserBridgeChild::Send__delete__(bridge);
-    return IPC_OK();
+    return IPC_FAIL(this, "Wrong actor");
   }
 
   RefPtr<nsFrameLoaderOwner> flo = do_QueryObject(embedderElt);
   MOZ_DIAGNOSTIC_ASSERT(flo, "Embedder must be a nsFrameLoaderOwner");
 
   // Trgger a process switch into the specified process.
-  IgnoredErrorResult rv;
-  flo->ChangeRemotenessWithBridge(bridge, rv);
-  if (NS_WARN_IF(rv.Failed())) {
-    BrowserBridgeChild::Send__delete__(bridge);
-    return IPC_OK();
-  }
-
+  flo->ChangeRemotenessWithBridge(std::move(aEndpoint), aTabId, IgnoreErrors());
   return IPC_OK();
 }