From c4e3ed85c0047ca375e3f1fea62fd36d8ec0f5eb Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 1 Jul 2020 13:28:13 -0700 Subject: [PATCH] browser(firefox): handle the case when inner window is restored from history (#2791) When innerWindow is restored from the history state, we do not receive content-document-global-created notification, but would still like to know that window is now using a different inner window to reset the state. This introduces a new notification juggler-dom-window-reused. At the same time, goBack()/goForward() sometimes do not initiate navigation synchronously, so our check for pendingNaivgationId() does not work. Instead, we rely on canGoBack, and assume that client will not need the navigationId synchronously. --- browser_patches/firefox/BUILD_NUMBER | 2 +- .../firefox/juggler/content/FrameTree.js | 1 + .../firefox/juggler/content/PageAgent.js | 11 ++-- .../firefox/juggler/protocol/Protocol.js | 14 ++-- .../firefox/patches/bootstrap.diff | 66 ++++++++++++++++++- 5 files changed, 76 insertions(+), 18 deletions(-) diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER index 42ba96426b..822fe6349f 100644 --- a/browser_patches/firefox/BUILD_NUMBER +++ b/browser_patches/firefox/BUILD_NUMBER @@ -1 +1 @@ -1116 +1117 diff --git a/browser_patches/firefox/juggler/content/FrameTree.js b/browser_patches/firefox/juggler/content/FrameTree.js index 7852aabd1d..46adfa7279 100644 --- a/browser_patches/firefox/juggler/content/FrameTree.js +++ b/browser_patches/firefox/juggler/content/FrameTree.js @@ -53,6 +53,7 @@ class FrameTree { Ci.nsIWebProgress.NOTIFY_FRAME_LOCATION; this._eventListeners = [ helper.addObserver(this._onDOMWindowCreated.bind(this), 'content-document-global-created'), + helper.addObserver(this._onDOMWindowCreated.bind(this), 'juggler-dom-window-reused'), helper.addObserver(subject => this._onDocShellCreated(subject.QueryInterface(Ci.nsIDocShell)), 'webnavigation-create'), helper.addObserver(subject => this._onDocShellDestroyed(subject.QueryInterface(Ci.nsIDocShell)), 'webnavigation-destroy'), helper.addProgressListener(webProgress, this, flags), diff --git a/browser_patches/firefox/juggler/content/PageAgent.js b/browser_patches/firefox/juggler/content/PageAgent.js index 278b5da4ce..a453e86a16 100644 --- a/browser_patches/firefox/juggler/content/PageAgent.js +++ b/browser_patches/firefox/juggler/content/PageAgent.js @@ -378,6 +378,8 @@ class PageAgent { } _onDOMContentLoaded(event) { + if (!event.target.ownerGlobal) + return; const docShell = event.target.ownerGlobal.docShell; const frame = this._frameTree.frameForDocShell(docShell); if (!frame) @@ -528,25 +530,24 @@ class PageAgent { const frame = this._frameTree.frame(frameId); const docShell = frame.docShell().QueryInterface(Ci.nsIWebNavigation); docShell.reload(Ci.nsIWebNavigation.LOAD_FLAGS_NONE); - return {navigationId: frame.pendingNavigationId(), navigationURL: frame.pendingNavigationURL()}; } async _goBack({frameId, url}) { const frame = this._frameTree.frame(frameId); const docShell = frame.docShell(); if (!docShell.canGoBack) - return {navigationId: null, navigationURL: null}; + return {success: false}; docShell.goBack(); - return {navigationId: frame.pendingNavigationId(), navigationURL: frame.pendingNavigationURL()}; + return {success: true}; } async _goForward({frameId, url}) { const frame = this._frameTree.frame(frameId); const docShell = frame.docShell(); if (!docShell.canGoForward) - return {navigationId: null, navigationURL: null}; + return {success: false}; docShell.goForward(); - return {navigationId: frame.pendingNavigationId(), navigationURL: frame.pendingNavigationURL()}; + return {success: true}; } async _adoptNode({frameId, objectId, executionContextId}) { diff --git a/browser_patches/firefox/juggler/protocol/Protocol.js b/browser_patches/firefox/juggler/protocol/Protocol.js index 87a3e8d866..1db1f8c835 100644 --- a/browser_patches/firefox/juggler/protocol/Protocol.js +++ b/browser_patches/firefox/juggler/protocol/Protocol.js @@ -712,27 +712,21 @@ const Page = { frameId: t.String, }, returns: { - navigationId: t.Nullable(t.String), - navigationURL: t.Nullable(t.String), - } + success: t.Boolean, + }, }, 'goForward': { params: { frameId: t.String, }, returns: { - navigationId: t.Nullable(t.String), - navigationURL: t.Nullable(t.String), - } + success: t.Boolean, + }, }, 'reload': { params: { frameId: t.String, }, - returns: { - navigationId: t.String, - navigationURL: t.String, - } }, 'getBoundingBox': { params: { diff --git a/browser_patches/firefox/patches/bootstrap.diff b/browser_patches/firefox/patches/bootstrap.diff index 1ad82d874d..1235889714 100644 --- a/browser_patches/firefox/patches/bootstrap.diff +++ b/browser_patches/firefox/patches/bootstrap.diff @@ -652,10 +652,60 @@ index e268e2bbe8add1b43f6e4d6507cc7810d707a344..a34a7a292a02ea8d94042475a43ae3a0 dom::MediaCapabilities* MediaCapabilities(); dom::MediaSession* MediaSession(); diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp -index da9d56e843a2c762dc7d5527712cdd3d30418f7f..ea9d962513dfc02682d5dfd543dd72ba78ab1745 100644 +index da9d56e843a2c762dc7d5527712cdd3d30418f7f..aa3d2d6022b60ac3a263c2d1df4eb12714d8e728 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp -@@ -3861,6 +3861,14 @@ Maybe nsGlobalWindowOuter::GetRDMDeviceSize( +@@ -2430,7 +2430,7 @@ nsresult nsGlobalWindowOuter::SetNewDocument(Document* aDocument, + &nsGlobalWindowInner::FireOnNewGlobalObject)); + } + +- if (newInnerWindow && !newInnerWindow->mHasNotifiedGlobalCreated && mDoc) { ++ if (newInnerWindow && mDoc) { + // We should probably notify. However if this is the, arguably bad, + // situation when we're creating a temporary non-chrome-about-blank + // document in a chrome docshell, don't notify just yet. Instead wait +@@ -2449,10 +2449,16 @@ nsresult nsGlobalWindowOuter::SetNewDocument(Document* aDocument, + }(); + + if (!isContentAboutBlankInChromeDocshell) { +- newInnerWindow->mHasNotifiedGlobalCreated = true; +- nsContentUtils::AddScriptRunner(NewRunnableMethod( +- "nsGlobalWindowOuter::DispatchDOMWindowCreated", this, +- &nsGlobalWindowOuter::DispatchDOMWindowCreated)); ++ if (!newInnerWindow->mHasNotifiedGlobalCreated) { ++ newInnerWindow->mHasNotifiedGlobalCreated = true; ++ nsContentUtils::AddScriptRunner(NewRunnableMethod( ++ "nsGlobalWindowOuter::DispatchDOMWindowCreated", this, ++ &nsGlobalWindowOuter::DispatchDOMWindowCreated)); ++ } else if (!reUseInnerWindow) { ++ nsContentUtils::AddScriptRunner(NewRunnableMethod( ++ "nsGlobalWindowOuter::JugglerDispatchDOMWindowReused", this, ++ &nsGlobalWindowOuter::JugglerDispatchDOMWindowReused)); ++ } + } + } + +@@ -2609,6 +2615,19 @@ void nsGlobalWindowOuter::DispatchDOMWindowCreated() { + } + } + ++void nsGlobalWindowOuter::JugglerDispatchDOMWindowReused() { ++ nsCOMPtr observerService = ++ mozilla::services::GetObserverService(); ++ if (observerService && mDoc) { ++ nsIPrincipal* principal = mDoc->NodePrincipal(); ++ if (!principal->IsSystemPrincipal()) { ++ observerService->NotifyObservers(static_cast(this), ++ "juggler-dom-window-reused", ++ nullptr); ++ } ++ } ++} ++ + void nsGlobalWindowOuter::ClearStatus() { SetStatusOuter(EmptyString()); } + + void nsGlobalWindowOuter::SetDocShell(nsDocShell* aDocShell) { +@@ -3861,6 +3880,14 @@ Maybe nsGlobalWindowOuter::GetRDMDeviceSize( } } } @@ -670,6 +720,18 @@ index da9d56e843a2c762dc7d5527712cdd3d30418f7f..ea9d962513dfc02682d5dfd543dd72ba return Nothing(); } +diff --git a/dom/base/nsGlobalWindowOuter.h b/dom/base/nsGlobalWindowOuter.h +index d2a8f0219f21800ef7800ed637c1f61d454a99a6..7e88989437613cec54d6bbf551afd572e28c4221 100644 +--- a/dom/base/nsGlobalWindowOuter.h ++++ b/dom/base/nsGlobalWindowOuter.h +@@ -319,6 +319,7 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget, + + // Outer windows only. + void DispatchDOMWindowCreated(); ++ void JugglerDispatchDOMWindowReused(); + + // Outer windows only. + virtual void EnsureSizeAndPositionUpToDate() override; diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp index 7c0d4bd261bf053c7cab4091522dc1e52118eba1..e320e6bfb0402eb07ae53272264278bad9638519 100644 --- a/dom/base/nsINode.cpp