Backed out 3 changesets (bug 1589054) for browser chrome failures on browser_crash_oopiframe.js CLOSED TREE
authorCosmin Sabou <csabou@mozilla.com>
Mon, 21 Oct 2019 19:29:20 +0300
changeset 498397 7537be6fe6bf885aa89388881e781648ca8f89f0
parent 498396 01028fb65d10c40684a5fa6b0f0da6fea5f0bd25
child 498398 eefafe971a89a0060761d5fda11b44b4d0b65bab
push id36717
push usernbeleuzu@mozilla.com
push dateMon, 21 Oct 2019 21:51:55 +0000
treeherdermozilla-central@563f437f24b9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1589054
milestone71.0a1
backs out1a43032819e160acfec3ccefc3fe9aa0fbc265b1
91e4d5c6422a0f6d47bd16618938b22a100da7fd
03bc24aa3a2c434e579cb44ba840b691d77cf841
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
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
dom/base/nsFrameLoader.h
dom/base/nsFrameLoaderOwner.cpp
dom/base/nsFrameLoaderOwner.h
dom/ipc/BrowserBridgeChild.cpp
dom/ipc/BrowserBridgeChild.h
dom/ipc/ContentChild.cpp
dom/ipc/WindowGlobalChild.cpp
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -1900,17 +1900,16 @@ void nsFrameLoader::DestroyDocShell() {
   }
 
   // Destroy the docshell.
   if (GetDocShell()) {
     GetDocShell()->Destroy();
   }
 
   mBrowsingContext = nullptr;
-  mDocShell = nullptr;
 
   if (mChildMessageManager) {
     // Stop handling events in the in-process frame script.
     mChildMessageManager->DisconnectEventListeners();
   }
 }
 
 void nsFrameLoader::DestroyComplete() {
@@ -2010,17 +2009,16 @@ nsresult nsFrameLoader::MaybeCreateDocSh
   if (NS_WARN_IF(!parentDocShell)) {
     return NS_ERROR_UNEXPECTED;
   }
 
   // nsDocShell::Create will attach itself to the passed browsing
   // context inside of nsDocShell::Create
   RefPtr<nsDocShell> docShell = nsDocShell::Create(mBrowsingContext);
   NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);
-  mDocShell = docShell;
 
   mBrowsingContext->SetEmbedderElement(mOwnerContent);
 
   mIsTopLevelContent =
       mBrowsingContext->IsContent() && !mBrowsingContext->GetParent();
   if (!mNetworkCreated && !mIsTopLevelContent) {
     docShell->SetCreatedDynamically(true);
   }
--- a/dom/base/nsFrameLoader.h
+++ b/dom/base/nsFrameLoader.h
@@ -119,17 +119,19 @@ class nsFrameLoader final : public nsStu
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(nsFrameLoader)
 
   NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
   nsresult CheckForRecursiveLoad(nsIURI* aURI);
   nsresult ReallyStartLoading();
   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;
   }
   nsresult UpdatePositionAndSize(nsSubDocumentFrame* aIFrame);
   void SendIsUnderHiddenEmbedderElement(bool aIsUnderHiddenEmbedderElement);
 
   // When creating a nsFrameLoader which is a static clone, two methods are
@@ -422,17 +424,20 @@ class nsFrameLoader final : public nsStu
 
   /**
    * If we are an IPC frame, set mRemoteFrame. Otherwise, create and
    * initialize mDocShell.
    */
   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.
   void UpdateBaseWindowPositionAndSize(nsSubDocumentFrame* aIFrame);
 
   /**
    * Checks whether a load of the given URI should be allowed, and returns an
    * error result if it should not.
@@ -498,17 +503,16 @@ class nsFrameLoader final : public nsStu
 
   // When performing a process switch, this value is used rather than mURIToLoad
   // to identify the process-switching load which should be resumed in the
   // target process.
   uint64_t mPendingSwitchID;
 
   uint64_t mChildID;
   RefPtr<mozilla::dom::RemoteBrowser> mRemoteBrowser;
-  RefPtr<nsDocShell> mDocShell;
 
   // Holds the last known size of the frame.
   mozilla::ScreenIntSize mLazySize;
 
   RefPtr<mozilla::dom::ParentSHistory> mParentSHistory;
 
   RefPtr<mozilla::dom::TabListener> mSessionStoreListener;
 
--- a/dom/base/nsFrameLoaderOwner.cpp
+++ b/dom/base/nsFrameLoaderOwner.cpp
@@ -85,52 +85,44 @@ void nsFrameLoaderOwner::ChangeRemotenes
   // document's load event, and immediately trigger the load event if there are
   // no other blockers. Since we're going to be adding a new blocker as soon as
   // we recreate the frame loader, this is not what we want, so add our own
   // blocker until the process is complete.
   Document* doc = owner->OwnerDoc();
   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
   // nsSubDocumentFrame to use the new FrameLoader.
   if (nsSubDocumentFrame* ourFrame = do_QueryFrame(owner->GetPrimaryFrame())) {
     ourFrame->ResetFrameLoader();
   }
 
@@ -185,28 +177,44 @@ void nsFrameLoaderOwner::ChangeRemotenes
       mFrameLoader->LoadFrame(false);
     }
   };
 
   ChangeRemotenessCommon(ShouldPreserveBrowsingContext(aOptions),
                          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);
     return;
   }
 
   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;
   };
 
   // NOTE: We always use the DEFAULT_REMOTE_TYPE here, because we don't actually
   // know the real remote type, and don't need to, as we're a content process.
   ChangeRemotenessCommon(
       /* preserve */ true, NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE),
       frameLoaderInit, rv);
--- a/dom/base/nsFrameLoaderOwner.h
+++ b/dom/base/nsFrameLoaderOwner.h
@@ -9,19 +9,23 @@
 
 #include "nsISupports.h"
 
 class nsFrameLoader;
 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
 #define NS_FRAMELOADEROWNER_IID                      \
   {                                                  \
     0x1b4fd25c, 0x2e57, 0x11e9, {                    \
       0x9e, 0x5a, 0x5b, 0x86, 0xe9, 0x89, 0xa5, 0xc0 \
     }                                                \
@@ -48,18 +52,20 @@ class nsFrameLoaderOwner : public nsISup
   // remoteness requirements. This should follow the same path as
   // tabbrowser.js's updateBrowserRemoteness, including running the same logic
   // and firing the same events as unbinding a XULBrowserElement from the tree.
   // However, this method is available from backend and does not manipulate the
   // DOM.
   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();
   bool ShouldPreserveBrowsingContext(
       const mozilla::dom::RemotenessOptions& aOptions);
   void ChangeRemotenessCommon(bool aPreserveContext,
                               const nsAString& aRemoteType,
                               std::function<void()>& aFrameLoaderInit,
--- a/dom/ipc/BrowserBridgeChild.cpp
+++ b/dom/ipc/BrowserBridgeChild.cpp
@@ -25,33 +25,33 @@
 #include "nsSubDocumentFrame.h"
 #include "nsView.h"
 
 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)
   if (mEmbeddedDocAccessible) {
     mEmbeddedDocAccessible->Shutdown();
   }
 #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);
 
   nsDocShell::Cast(docShell)->OOPChildLoadStarted(this);
 
 #if defined(ACCESSIBILITY)
   if (a11y::DocAccessible* docAcc =
--- a/dom/ipc/BrowserBridgeChild.h
+++ b/dom/ipc/BrowserBridgeChild.h
@@ -51,29 +51,30 @@ class BrowserBridgeChild : public PBrows
   void NavigateByKey(bool aForward, bool aForDocumentNavigation);
 
   void Activate();
 
   void Deactivate(bool aWindowLowering);
 
   void SetIsUnderHiddenEmbedderElement(bool aIsUnderHiddenEmbedderElement);
 
-  already_AddRefed<BrowserBridgeHost> FinishInit(nsFrameLoader* aFrameLoader);
+  already_AddRefed<BrowserBridgeHost> FinishInit();
 
 #if defined(ACCESSIBILITY) && defined(XP_WIN)
   a11y::RemoteIframeDocProxyAccessibleWrap* GetEmbeddedDocAccessible() {
     return mEmbeddedDocAccessible;
   }
 #endif
 
   static BrowserBridgeChild* GetFrom(nsFrameLoader* aFrameLoader);
 
   static BrowserBridgeChild* GetFrom(nsIContent* aContent);
 
-  BrowserBridgeChild(BrowsingContext* aBrowsingContext, TabId aId);
+  BrowserBridgeChild(nsFrameLoader* aFrameLoader,
+                     BrowsingContext* aBrowsingContext, TabId aId);
 
  protected:
   friend class ContentChild;
   friend class PBrowserBridgeChild;
 
   mozilla::ipc::IPCResult RecvSetLayersId(
       const mozilla::layers::LayersId& aLayersId);
 
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -2122,23 +2122,23 @@ already_AddRefed<RemoteBrowser> ContentC
     chromeFlags |= nsIWebBrowserChrome::CHROME_FISSION_WINDOW;
   }
   if (docShell->GetAffectPrivateSessionLifetime()) {
     chromeFlags |= nsIWebBrowserChrome::CHROME_PRIVATE_LIFETIME;
   }
 
   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(
     const FileDescOrError& cacheFile, const bool& wantCacheData) {
   return new loader::ScriptCacheChild();
 }
 
 bool ContentChild::DeallocPScriptCacheChild(PScriptCacheChild* cache) {
--- a/dom/ipc/WindowGlobalChild.cpp
+++ b/dom/ipc/WindowGlobalChild.cpp
@@ -276,21 +276,21 @@ mozilla::ipc::IPCResult WindowGlobalChil
     dom::BrowsingContext* aFrameContext, uint64_t aPendingSwitchId) {
   MOZ_DIAGNOSTIC_ASSERT(XRE_IsContentProcess());
 
   MOZ_LOG(aFrameContext->GetLog(), LogLevel::Debug,
           ("RecvMakeFrameLocal ID=%" PRIx64, aFrameContext->Id()));
 
   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);
   MOZ_DIAGNOSTIC_ASSERT(flo, "Embedder must be a nsFrameLoaderOwner");
 
   // Trigger a process switch into the current process.
   RemotenessOptions options;
   options.mRemoteType.Assign(VoidString());
@@ -306,48 +306,30 @@ mozilla::ipc::IPCResult WindowGlobalChil
   MOZ_DIAGNOSTIC_ASSERT(XRE_IsContentProcess());
 
   MOZ_LOG(aFrameContext->GetLog(), LogLevel::Debug,
           ("RecvMakeFrameRemote ID=%" PRIx64, aFrameContext->Id()));
 
   // 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();
 }
 
 mozilla::ipc::IPCResult WindowGlobalChild::RecvDrawSnapshot(
     const Maybe<IntRect>& aRect, const float& aScale,
     const nscolor& aBackgroundColor, const uint32_t& aFlags,
     DrawSnapshotResolver&& aResolve) {
   nsCOMPtr<nsIDocShell> docShell = BrowsingContext()->GetDocShell();