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

@@ -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;
     }
   }
 }

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;
       }

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<ChildSHistory> 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<ChildSHistory> 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<ChildSHistory> 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<nsIURI> uri;
   nsCOMPtr<nsIInputStream> postData(aLoadURIOptions.mPostData);
   nsresult rv = NS_OK;

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__ */

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;
 };

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();
     }
   }

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;
   }
 

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;

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;