diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index ab90f90a4518..7abb3bb1c9c9 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -91,6 +91,13 @@ void CanonicalBrowsingContext::SetOwnerProcessId(uint64_t aProcessId) { mProcessId = aProcessId; } +void CanonicalBrowsingContext::SetInFlightProcessId(uint64_t aProcessId) { + // We can't handle more than one in-flight process change at a time. + MOZ_ASSERT_IF(aProcessId, mInFlightProcessId == 0); + + mInFlightProcessId = aProcessId; +} + void CanonicalBrowsingContext::GetWindowGlobals( nsTArray>& aWindows) { aWindows.SetCapacity(mWindowGlobals.Count()); diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index d85472ad82e4..aad3b9948d3a 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -41,6 +41,9 @@ class CanonicalBrowsingContext final : public BrowsingContext { void SetOwnerProcessId(uint64_t aProcessId); + void SetInFlightProcessId(uint64_t aProcessId); + uint64_t GetInFlightProcessId() const { return mInFlightProcessId; } + void GetWindowGlobals(nsTArray>& aWindows); // Called by WindowGlobalParent to register and unregister window globals. @@ -93,6 +96,10 @@ class CanonicalBrowsingContext final : public BrowsingContext { // Indicates which process owns the docshell. uint64_t mProcessId; + // The ID of the former owner process during an ownership change, which may + // have in-flight messages that assume it is still the owner. + uint64_t mInFlightProcessId = 0; + // All live window globals within this browsing context. nsTHashtable> mWindowGlobals; RefPtr mCurrentWindowGlobal; diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 9687d89a9f26..0b6a4b29b117 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -3425,7 +3425,29 @@ void nsFrameLoader::SkipBrowsingContextDetach() { if (IsRemoteFrame()) { // OOP Browser - Go directly over Browser Parent if (auto* browserParent = GetBrowserParent()) { - Unused << browserParent->SendSkipBrowsingContextDetach(); + RefPtr bc(GetBrowsingContext()); + // We're going to be synchronously changing the owner of the + // BrowsingContext in the parent process while the current owner may still + // have in-flight requests which only the owner is allowed to make. Those + // requests will typically trigger assertions if they come from a child + // other than the owner. + // + // To work around this, we record the previous owner at the start of the + // process switch, and clear it when we've received a reply from the + // child, treating ownership mismatches as warnings in the interim. + // + // In the future, this sort of issue will probably need to be handled + // using ownership epochs, which should be more both flexible and + // resilient. For the moment, though, the surrounding process switch code + // is enough in flux that we're better off with a workable interim + // solution. + bc->Canonical()->SetInFlightProcessId( + browserParent->Manager()->ChildID()); + browserParent->SendSkipBrowsingContextDetach( + [bc](bool aSuccess) { bc->Canonical()->SetInFlightProcessId(0); }, + [bc](mozilla::ipc::ResponseRejectReason aReason) { + bc->Canonical()->SetInFlightProcessId(0); + }); } // OOP IFrame - Through Browser Bridge Parent, set on browser child else if (auto* browserBridgeChild = GetBrowserBridgeChild()) { diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 72cd8d717a2c..0a629bca0323 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -1057,14 +1057,13 @@ BrowserChild::~BrowserChild() { mozilla::DropJSObjects(this); } -mozilla::ipc::IPCResult BrowserChild::RecvSkipBrowsingContextDetach() { - nsCOMPtr docShell = do_GetInterface(WebNavigation()); - if (!docShell) { - return IPC_OK(); +mozilla::ipc::IPCResult BrowserChild::RecvSkipBrowsingContextDetach( + SkipBrowsingContextDetachResolver&& aResolve) { + if (nsCOMPtr docShell = do_GetInterface(WebNavigation())) { + RefPtr docshell = nsDocShell::Cast(docShell); + docshell->SkipBrowsingContextDetach(); } - RefPtr docshell = nsDocShell::Cast(docShell); - MOZ_ASSERT(docshell); - docshell->SkipBrowsingContextDetach(); + aResolve(true); return IPC_OK(); } diff --git a/dom/ipc/BrowserChild.h b/dom/ipc/BrowserChild.h index 38cc4c79ecc5..4175651fcdc8 100644 --- a/dom/ipc/BrowserChild.h +++ b/dom/ipc/BrowserChild.h @@ -519,7 +519,8 @@ class BrowserChild final : public nsMessageManagerScriptExecutor, mozilla::ipc::IPCResult RecvUpdateNativeWindowHandle( const uintptr_t& aNewHandle); - mozilla::ipc::IPCResult RecvSkipBrowsingContextDetach(); + mozilla::ipc::IPCResult RecvSkipBrowsingContextDetach( + SkipBrowsingContextDetachResolver&& aResolve); /** * Native widget remoting protocol for use with windowed plugins with e10s. */ diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 260e0642864b..26990152af7d 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5838,7 +5838,7 @@ mozilla::ipc::IPCResult ContentParent::RecvAttachBrowsingContext( bool ContentParent::CheckBrowsingContextOwnership( BrowsingContext* aBC, const char* aOperation) const { if (!aBC->Canonical()->IsOwnedByProcess(ChildID())) { - MOZ_DIAGNOSTIC_ASSERT(false, + MOZ_DIAGNOSTIC_ASSERT(ChildID() == aBC->Canonical()->GetInFlightProcessId(), "Attempt to modify a BrowsingContext from a child " "which doesn't own it"); diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index bb3b481123b6..24bbe744f30b 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -1026,7 +1026,7 @@ child: */ async GetContentBlockingLog() returns(IPCStream ipcStream, bool success); - async SkipBrowsingContextDetach(); + async SkipBrowsingContextDetach() returns (bool success); parent: /** Records a history visit. */ async VisitURI(URIParams aURI, URIParams? aLastVisitedURI,