From 034508353204d30880cd4e9da833c0ab5559c37a Mon Sep 17 00:00:00 2001 From: Barret Rennie Date: Thu, 23 May 2019 18:48:48 +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 --HG-- extra : moz-landing-system : lando --- 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 212f85b33c90..4a5653a8e625 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -1229,7 +1229,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, @@ -1299,7 +1299,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 3e34e958ac6d..b0e13020112b 100644 --- a/browser/base/content/tabbrowser.js +++ b/browser/base/content/tabbrowser.js @@ -5048,8 +5048,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) { @@ -5122,7 +5122,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 a1003de72bed..175e3222d94d 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -387,7 +387,8 @@ nsDocShell::nsDocShell(BrowsingContext* aBrowsingContext) mTitleValidForCurrentURI(false), mIsFrame(false), mSkipBrowsingContextDetachOnDestroy(false), - mWatchedByDevtools(false) { + mWatchedByDevtools(false), + mIsNavigating(false) { mHistoryID.m0 = 0; mHistoryID.m1 = 0; mHistoryID.m2 = 0; @@ -3727,6 +3728,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) { @@ -3828,6 +3835,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; @@ -3840,6 +3851,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; @@ -3854,6 +3869,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); @@ -3869,6 +3888,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 44e23bb5e5ca..d9bbc4ad5dbb 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -1250,6 +1250,10 @@ class nsDocShell final : public nsDocLoader, // Set when activity in this docshell is being watched by the developer tools. bool mWatchedByDevtools : 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 cee8319e4766..484dc90d1b85 100644 --- a/docshell/base/nsIDocShell.idl +++ b/docshell/base/nsIDocShell.idl @@ -1192,4 +1192,17 @@ interface nsIDocShell : nsIDocShellTreeItem * Whether developer tools are watching activity in this docshell. */ [infallible] attribute boolean watchedByDevtools; + + /* + * 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 a422b376b86f..cceed5fbad89 100644 --- a/toolkit/actors/WebNavigationChild.jsm +++ b/toolkit/actors/WebNavigationChild.jsm @@ -51,11 +51,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 612cbff6e0da..d451173f9ae8 100644 --- a/toolkit/content/widgets/browser-custom-element.js +++ b/toolkit/content/widgets/browser-custom-element.js @@ -757,11 +757,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(); @@ -1020,7 +1020,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 97ab638b9087..100fb288001c 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); @@ -141,7 +139,7 @@ class WebProgressChild { let csp = this.mm.content.document.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;