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

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

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) {

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;
@@ -3723,6 +3724,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) {
@@ -3824,6 +3831,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;
@@ -3836,6 +3847,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;
@@ -3850,6 +3865,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);
@@ -3865,6 +3884,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

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

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

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

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

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