From 48a2bc08bb89ac66bfc08888b64dbb54bc24c781 Mon Sep 17 00:00:00 2001 From: Barret Rennie Date: Thu, 2 May 2019 23:35:02 +0000 Subject: [PATCH] Bug 1510569 - Keep track of whether we are navigating to a new URI in nsDocShell r=mconley,kmag,qdot Previously the `WebNavigationChild` would keep track of when triggering its `nsIWebNavigation`, `goForward`, `goBack`, `gotoIndex`, and `loadURI` methods. It's `nsIWebNavigation` instance is always an `nsIDocShell` and as part of porting `OnStateChange` and `OnLocationChange` events from `WebProgressChild`/`RemoteWebProgress` to `BrowserChild`/`BrowserParent`, this informations needs to be available from the `BrowserChild`. As it stands, it is currently an expando property on the `WebProgressChild`. Instead of introducing yet another XPCOM interface for the WebProgressChild, we now store this information directly on the `nsDocShell`. Furthermore, instead of having the `WebNavigationChild` manage this part of the `nsDocShell`'s state, we can have the `nsDocShell` manage this state itself so it is always consistent. Differential Revision: https://phabricator.services.mozilla.com/D28124 --- browser/base/content/browser.js | 4 +-- browser/base/content/tabbrowser.js | 6 ++--- docshell/base/nsDocShell.cpp | 25 ++++++++++++++++++- docshell/base/nsDocShell.h | 4 +++ docshell/base/nsIDocShell.idl | 13 ++++++++++ toolkit/actors/WebNavigationChild.jsm | 2 -- .../content/widgets/browser-custom-element.js | 6 ++--- toolkit/modules/RemoteWebProgress.jsm | 6 ++--- toolkit/modules/WebProgressChild.jsm | 6 ++--- 9 files changed, 54 insertions(+), 18 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index fac54aff09c4..a9f32a336047 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -1208,7 +1208,7 @@ function _loadURI(browser, uri, params = {}) { // !requiredRemoteType means we're loading in the parent/this process. if (!requiredRemoteType) { - browser.inLoadURI = true; + browser.isNavigating = true; } let loadURIOptions = { triggeringPrincipal, @@ -1278,7 +1278,7 @@ function _loadURI(browser, uri, params = {}) { } } finally { if (!requiredRemoteType) { - browser.inLoadURI = false; + browser.isNavigating = false; } } } diff --git a/browser/base/content/tabbrowser.js b/browser/base/content/tabbrowser.js index 0a2a91328d8c..fb955b6cb2d6 100644 --- a/browser/base/content/tabbrowser.js +++ b/browser/base/content/tabbrowser.js @@ -5027,8 +5027,8 @@ class TabProgressListener { this.mBrowser.userTypedValue = null; - let inLoadURI = this.mBrowser.inLoadURI; - if (this.mTab.selected && gURLBar && !inLoadURI) { + let isNavigating = this.mBrowser.isNavigating; + if (this.mTab.selected && gURLBar && !isNavigating) { URLBarSetURI(); } } else if (isSuccessful) { @@ -5101,7 +5101,7 @@ class TabProgressListener { // and the user cleared the URL manually. if (this.mBrowser.didStartLoadSinceLastUserTyping() || (isErrorPage && aLocation.spec != "about:blank") || - (isSameDocument && this.mBrowser.inLoadURI) || + (isSameDocument && this.mBrowser.isNavigating) || (isSameDocument && !this.mBrowser.userTypedValue)) { this.mBrowser.userTypedValue = null; } diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 1acbad18d125..b3a5b3adef41 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -385,7 +385,8 @@ nsDocShell::nsDocShell(BrowsingContext* aBrowsingContext) mHasLoadedNonBlankURI(false), mBlankTiming(false), mTitleValidForCurrentURI(false), - mIsFrame(false) { + mIsFrame(false), + mIsNavigating(false) { mHistoryID.m0 = 0; mHistoryID.m1 = 0; mHistoryID.m2 = 0; @@ -3725,6 +3726,12 @@ nsDocShell::GetContentBlockingLog(Promise** aPromise) { return NS_OK; } +NS_IMETHODIMP +nsDocShell::GetIsNavigating(bool* aOut) { + *aOut = mIsNavigating; + return NS_OK; +} + NS_IMETHODIMP nsDocShell::SetDeviceSizeIsPageSize(bool aValue) { if (mDeviceSizeIsPageSize != aValue) { @@ -3826,6 +3833,10 @@ nsDocShell::GoBack() { if (!IsNavigationAllowed()) { return NS_OK; // JS may not handle returning of an error code } + + auto cleanupIsNavigating = MakeScopeExit([&]() { mIsNavigating = false; }); + mIsNavigating = true; + RefPtr rootSH = GetRootSessionHistory(); NS_ENSURE_TRUE(rootSH, NS_ERROR_FAILURE); ErrorResult rv; @@ -3838,6 +3849,10 @@ nsDocShell::GoForward() { if (!IsNavigationAllowed()) { return NS_OK; // JS may not handle returning of an error code } + + auto cleanupIsNavigating = MakeScopeExit([&]() { mIsNavigating = false; }); + mIsNavigating = true; + RefPtr rootSH = GetRootSessionHistory(); NS_ENSURE_TRUE(rootSH, NS_ERROR_FAILURE); ErrorResult rv; @@ -3852,6 +3867,10 @@ nsDocShell::GotoIndex(int32_t aIndex) { if (!IsNavigationAllowed()) { return NS_OK; // JS may not handle returning of an error code } + + auto cleanupIsNavigating = MakeScopeExit([&]() { mIsNavigating = false; }); + mIsNavigating = true; + RefPtr rootSH = GetRootSessionHistory(); NS_ENSURE_TRUE(rootSH, NS_ERROR_FAILURE); return rootSH->LegacySHistory()->GotoIndex(aIndex); @@ -3867,6 +3886,10 @@ nsresult nsDocShell::LoadURI(const nsAString& aURI, if (!IsNavigationAllowed()) { return NS_OK; // JS may not handle returning of an error code } + + auto cleanupIsNavigating = MakeScopeExit([&]() { mIsNavigating = false; }); + mIsNavigating = true; + nsCOMPtr uri; nsCOMPtr postData(aLoadURIOptions.mPostData); nsresult rv = NS_OK; diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index be40c31b89e1..a41bf9b9bd3c 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -1205,6 +1205,10 @@ class nsDocShell final : public nsDocLoader, bool mTitleValidForCurrentURI : 1; bool mIsFrame : 1; + + // This flag indicates whether or not the DocShell is currently executing an + // nsIWebNavigation navigation method. + bool mIsNavigating : 1; }; #endif /* nsDocShell_h__ */ diff --git a/docshell/base/nsIDocShell.idl b/docshell/base/nsIDocShell.idl index 5574480ae6df..6f72f27739ce 100644 --- a/docshell/base/nsIDocShell.idl +++ b/docshell/base/nsIDocShell.idl @@ -1167,4 +1167,17 @@ interface nsIDocShell : nsIDocShellTreeItem * sense that's relevant to document.open. */ [notxpcom, nostdcall] readonly attribute boolean isAttemptingToNavigate; + + /** + * Whether or not this docshell is executing a nsIWebNavigation navigation + * method. + * + * This will be true when the following methods are executing: + * nsIWebNavigation.binaryLoadURI + * nsIWebNavigation.goBack + * nsIWebNavigation.goForward + * nsIWebNavigation.gotoIndex + * nsIWebNavigation.loadURI + */ + [infallible] readonly attribute boolean isNavigating; }; diff --git a/toolkit/actors/WebNavigationChild.jsm b/toolkit/actors/WebNavigationChild.jsm index b024850732ef..9ae4ee09209b 100644 --- a/toolkit/actors/WebNavigationChild.jsm +++ b/toolkit/actors/WebNavigationChild.jsm @@ -56,11 +56,9 @@ class WebNavigationChild extends ActorChild { } _wrapURIChangeCall(fn) { - this.mm.WebProgress.inLoadURI = true; try { fn(); } finally { - this.mm.WebProgress.inLoadURI = false; this.mm.WebProgress.sendLoadCallResult(); } } diff --git a/toolkit/content/widgets/browser-custom-element.js b/toolkit/content/widgets/browser-custom-element.js index 53d84f703b1b..2682f54dc159 100644 --- a/toolkit/content/widgets/browser-custom-element.js +++ b/toolkit/content/widgets/browser-custom-element.js @@ -759,11 +759,11 @@ class MozBrowser extends MozElements.MozElementMixin(XULFrameElement) { _wrapURIChangeCall(fn) { if (!this.isRemoteBrowser) { - this.inLoadURI = true; + this.isNavigating = true; try { fn(); } finally { - this.inLoadURI = false; + this.isNavigating = false; } } else { fn(); @@ -1022,7 +1022,7 @@ class MozBrowser extends MozElements.MozElementMixin(XULFrameElement) { } didStartLoadSinceLastUserTyping() { - return !this.inLoadURI && + return !this.isNavigating && this.urlbarChangeTracker._startedLoadSinceLastUserTyping; } diff --git a/toolkit/modules/RemoteWebProgress.jsm b/toolkit/modules/RemoteWebProgress.jsm index 2ff6c228fdae..ceb7b786ad5a 100644 --- a/toolkit/modules/RemoteWebProgress.jsm +++ b/toolkit/modules/RemoteWebProgress.jsm @@ -158,7 +158,7 @@ class RemoteWebProgressManager { // It shouldn't go through the same processing as all the forwarded // webprogresslistener messages. if (aMessage.name == "Content:LoadURIResult") { - this._browser.inLoadURI = false; + this._browser.isNavigating = false; return; } @@ -193,8 +193,8 @@ class RemoteWebProgressManager { if (json.documentContentType !== null) { this._browser._documentContentType = json.documentContentType; } - if (typeof json.inLoadURI != "undefined") { - this._browser.inLoadURI = json.inLoadURI; + if (typeof json.isNavigating != "undefined") { + this._browser.isNavigating = json.isNavigating; } if (json.charset) { this._browser._characterSet = json.charset; diff --git a/toolkit/modules/WebProgressChild.jsm b/toolkit/modules/WebProgressChild.jsm index 2d856f546097..591fc85a3c31 100644 --- a/toolkit/modules/WebProgressChild.jsm +++ b/toolkit/modules/WebProgressChild.jsm @@ -25,8 +25,6 @@ class WebProgressChild { constructor(mm) { this.mm = mm; - this.inLoadURI = false; - // NOTIFY_PROGRESS, NOTIFY_STATUS, NOTIFY_REFRESH, and // NOTIFY_CONTENT_BLOCKING are handled by PBrowser. let notifyCode = Ci.nsIWebProgress.NOTIFY_ALL & @@ -115,7 +113,7 @@ class WebProgressChild { json.documentURI = this.mm.content.document.documentURIObject.spec; json.charset = this.mm.content.document.characterSet; json.mayEnableCharacterEncodingMenu = this.mm.docShell.mayEnableCharacterEncodingMenu; - json.inLoadURI = this.inLoadURI; + json.isNavigating = this.mm.docShell.isNavigating; } this._send("Content:StateChange", json); @@ -143,7 +141,7 @@ class WebProgressChild { let csp = this.mm.content.document.nodePrincipal.csp; json.csp = E10SUtils.serializeCSP(csp); json.synthetic = this.mm.content.document.mozSyntheticDocument; - json.inLoadURI = this.inLoadURI; + json.isNavigating = this.mm.docShell.isNavigating; json.requestContextID = this.mm.content.document.documentLoadGroup ? this.mm.content.document.documentLoadGroup.requestContextID : null;