Bug 798249 - track when we're 'inside' a loadURI call when dealing with Stop() calls resulting from the same, r?mconley,mak draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 16 Feb 2016 14:08:14 +0000
changeset 331210 27a6aee6fe9ed6be771a40ea9de5abed602410ef
parent 331209 b2d75ac5ba0fc90ab7e58e81caff2e4b474f7ca9
child 514331 8c690399b03cf0c57474b734c8d089be2bcf226d
push id10931
push usergijskruitbosch@gmail.com
push dateTue, 16 Feb 2016 15:04:42 +0000
reviewersmconley, mak
bugs798249
milestone47.0a1
Bug 798249 - track when we're 'inside' a loadURI call when dealing with Stop() calls resulting from the same, r?mconley,mak MozReview-Commit-ID: 48xQn03HtjZ
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/urlbar/browser.ini
browser/base/content/test/urlbar/browser_urlbar_stop_pending.js
browser/base/content/test/urlbar/slow-page.sjs
browser/base/moz.build
toolkit/content/browser-child.js
toolkit/modules/RemoteWebProgress.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -825,20 +825,26 @@ function _loadURIWithFlags(browser, uri,
                         Ci.nsIHttpChannel.REFERRER_POLICY_DEFAULT);
   let charset = params.charset;
   let postData = params.postData;
 
   if (!(flags & browser.webNavigation.LOAD_FLAGS_FROM_EXTERNAL)) {
     browser.userTypedClear++;
   }
 
+  let wasRemote = browser.isRemoteBrowser;
+
   let process = browser.isRemoteBrowser ? Ci.nsIXULRuntime.PROCESS_TYPE_CONTENT
                                         : Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT;
   let mustChangeProcess = gMultiProcessBrowser &&
                           !E10SUtils.canLoadURIInProcess(uri, process);
+  if ((!wasRemote && !mustChangeProcess) ||
+      (wasRemote && mustChangeProcess)) {
+    browser.inLoadURI = true;
+  }
   try {
     if (!mustChangeProcess) {
       browser.webNavigation.loadURIWithOptions(uri, flags,
                                                referrer, referrerPolicy,
                                                postData, null, null);
     } else {
       if (postData) {
         postData = NetUtil.readInputStreamToString(postData, postData.available());
@@ -862,16 +868,17 @@ function _loadURIWithFlags(browser, uri,
       Cu.reportError(e);
       gBrowser.updateBrowserRemotenessByURL(browser, uri);
       browser.webNavigation.loadURIWithOptions(uri, flags, referrer, referrerPolicy,
                                                postData, null, null);
     } else {
       throw e;
     }
   } finally {
+    browser.inLoadURI = false;
     if (browser.userTypedClear) {
       browser.userTypedClear--;
     }
   }
 }
 
 // Starts a new load in the browser first switching the browser to the correct
 // process
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -679,18 +679,27 @@
                   if (!Components.isSuccessCode(aStatus) &&
                       !isTabEmpty(this.mTab)) {
                     // Restore the current document's location in case the
                     // request was stopped (possibly from a content script)
                     // before the location changed.
 
                     this.mBrowser.userTypedValue = null;
 
-                    if (this.mTab.selected && gURLBar)
+                    let inLoadURI = false;
+                    if (gMultiProcessBrowser) {
+                      inLoadURI = !!(aWebProgress.wrappedJSObject &&
+                                     aWebProgress.wrappedJSObject.inLoadURI);
+                    } else {
+                      inLoadURI = this.mBrowser.inLoadURI;
+                    }
+
+                    if (this.mTab.selected && gURLBar && !inLoadURI) {
                       URLBarSetURI();
+                    }
                   } else {
                     // The document is done loading, we no longer want the
                     // value cleared.
 
                     if (this.mBrowser.userTypedClear > 1)
                       this.mBrowser.userTypedClear -= 2;
                     else if (this.mBrowser.userTypedClear > 0)
                       this.mBrowser.userTypedClear--;
@@ -1310,17 +1319,17 @@
         A newly created tab has position Infinity in the cache.
         If a tab is closed, it has no effect on the position of other
         tabs in the cache since we assume that closing a tab doesn't
         cause us to load in any other tabs.
 
         We ignore the effect of dragging tabs between windows.
       -->
       <method name="_recordTabAccess">
-	<parameter name="aTab"/>
+        <parameter name="aTab"/>
         <body><![CDATA[
           if (!Services.telemetry.canRecordExtended) {
             return;
           }
 
           let tabs = Array.from(this.visibleTabs);
 
           let pos = aTab.cachePosition;
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/urlbar/browser.ini
@@ -0,0 +1,6 @@
+[DEFAULT]
+
+[browser_urlbar_stop_pending.js]
+support-files =
+  slow-page.sjs
+
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/urlbar/browser_urlbar_stop_pending.js
@@ -0,0 +1,32 @@
+"use strict";
+
+const SLOW_PAGE = "http://www.example.com/browser/browser/base/content/test/urlbar/slow-page.sjs";
+const SLOW_PAGE2 = "http://mochi.test:8888/browser/browser/base/content/test/urlbar/slow-page.sjs?faster";
+
+add_task(function*() {
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "about:home", true);
+
+  let expectedURLBarChange = SLOW_PAGE;
+  let handler = e => {
+    is(gURLBar.value, expectedURLBarChange, "Should not change URL bar value!");
+  };
+
+  let obs = new MutationObserver(handler);
+
+  obs.observe(gURLBar, {attributes: true});
+  gURLBar.value = SLOW_PAGE;
+  gURLBar.handleCommand();
+
+  // If this ever starts going intermittent, we've broken this.
+  yield new Promise(resolve => setTimeout(resolve, 200));
+  expectedURLBarChange = SLOW_PAGE2;
+  let pageLoadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  gURLBar.value = expectedURLBarChange;
+  gURLBar.handleCommand();
+  is(gURLBar.value, expectedURLBarChange, "Should not have changed URL bar value synchronously.");
+  yield pageLoadPromise;
+  obs.disconnect();
+  obs = null;
+  gBrowser.removeCurrentTab();
+});
+
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/urlbar/slow-page.sjs
@@ -0,0 +1,22 @@
+"use strict";
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+let timer;
+
+const DELAY_MS = 5000;
+function handleRequest(request, response) {
+  if (request.queryString.endsWith("faster")) {
+    response.setHeader("Content-Type", "text/html", false);
+    response.write("<body>Not so slow!</body>");
+    return;
+  }
+  response.processAsync();
+  timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+  timer.init(() => {
+    response.setHeader("Content-Type", "text/html", false);
+    response.write("<body>This was the slow load. You should never see this.</body>");
+    response.finish();
+  }, DELAY_MS, Ci.nsITimer.TYPE_ONE_SHOT);
+}
--- a/browser/base/moz.build
+++ b/browser/base/moz.build
@@ -18,16 +18,17 @@ BROWSER_CHROME_MANIFESTS += [
     'content/test/alerts/browser.ini',
     'content/test/chat/browser.ini',
     'content/test/general/browser.ini',
     'content/test/newtab/browser.ini',
     'content/test/plugins/browser.ini',
     'content/test/popupNotifications/browser.ini',
     'content/test/referrer/browser.ini',
     'content/test/social/browser.ini',
+    'content/test/urlbar/browser.ini',
 ]
 
 DEFINES['MOZ_APP_VERSION'] = CONFIG['MOZ_APP_VERSION']
 DEFINES['MOZ_APP_VERSION_DISPLAY'] = CONFIG['MOZ_APP_VERSION_DISPLAY']
 
 DEFINES['APP_LICENSE_BLOCK'] = '%s/content/overrides/app-license.html' % SRCDIR
 
 if CONFIG['MOZ_WIDGET_TOOLKIT'] in ('windows', 'gtk2', 'gtk3', 'cocoa'):
--- a/toolkit/content/browser-child.js
+++ b/toolkit/content/browser-child.js
@@ -125,16 +125,17 @@ var WebProgressListener = {
     // It's possible that this state change was triggered by
     // loading an internal error page, for which the parent
     // will want to know some details, so we'll update it with
     // the documentURI.
     if (aWebProgress && aWebProgress.isTopLevel) {
       json.documentURI = content.document.documentURIObject.spec;
     }
 
+    json.inLoadURI = WebNavigation.inLoadURI;
     this._send("Content:StateChange", json, objects);
   },
 
   onProgressChange: function onProgressChange(aWebProgress, aRequest, aCurSelf, aMaxSelf, aCurTotal, aMaxTotal) {
     let json = this._setupJSON(aWebProgress, aRequest);
     let objects = this._setupObjects(aWebProgress, aRequest);
 
     json.curSelf = aCurSelf;
@@ -232,16 +233,22 @@ var WebNavigation =  {
     addMessageListener("WebNavigation:Reload", this);
     addMessageListener("WebNavigation:Stop", this);
   },
 
   get webNavigation() {
     return docShell.QueryInterface(Ci.nsIWebNavigation);
   },
 
+  _inLoadURI: false,
+
+  get inLoadURI() {
+    return this._inLoadURI;
+  },
+
   receiveMessage: function(message) {
     switch (message.name) {
       case "WebNavigation:GoBack":
         this.goBack();
         break;
       case "WebNavigation:GoForward":
         this.goForward();
         break;
@@ -293,18 +300,23 @@ var WebNavigation =  {
     if (referrer)
       referrer = Services.io.newURI(referrer, null, null);
     if (postData)
       postData = makeInputStream(postData);
     if (headers)
       headers = makeInputStream(headers);
     if (baseURI)
       baseURI = Services.io.newURI(baseURI, null, null);
-    this.webNavigation.loadURIWithOptions(uri, flags, referrer, referrerPolicy,
-                                          postData, headers, baseURI);
+    this._inLoadURI = true;
+    try {
+      this.webNavigation.loadURIWithOptions(uri, flags, referrer, referrerPolicy,
+                                            postData, headers, baseURI);
+    } finally {
+      this._inLoadURI = false;
+    }
   },
 
   reload: function(flags) {
     this.webNavigation.reload(flags);
   },
 
   stop: function(flags) {
     this.webNavigation.stop(flags);
--- a/toolkit/modules/RemoteWebProgress.jsm
+++ b/toolkit/modules/RemoteWebProgress.jsm
@@ -58,16 +58,17 @@ RemoteWebProgress.prototype = {
   NOTIFY_REFRESH:        0x00000100,
   NOTIFY_ALL:            0x000001ff,
 
   get isLoadingDocument() { return this._isLoadingDocument },
   get DOMWindow() { return this._DOMWindow; },
   get DOMWindowID() { return this._DOMWindowID; },
   get isTopLevel() { return this._isTopLevel },
   get loadType() { return this._loadType; },
+  get inLoadURI() { return this._inLoadURI; },
 
   addProgressListener: function (aListener) {
     this._manager.addProgressListener(aListener);
   },
 
   removeProgressListener: function (aListener) {
     this._manager.removeProgressListener(aListener);
   }
@@ -192,16 +193,17 @@ RemoteWebProgressManager.prototype = {
       webProgress = isTopLevel ? this._topLevelWebProgress
                                : new RemoteWebProgress(this, false);
 
       // Update the actual WebProgress fields.
       webProgress._isLoadingDocument = json.webProgress.isLoadingDocument;
       webProgress._DOMWindow = objects.DOMWindow;
       webProgress._DOMWindowID = json.webProgress.DOMWindowID;
       webProgress._loadType = json.webProgress.loadType;
+      webProgress._inLoadURI = json.inLoadURI;
       webProgress._webProgressCPOW = objects.webProgress;
     }
 
     // The WebProgressRequest object however is always dynamic.
     let request = null;
     if (json.requestURI) {
       request = new RemoteWebProgressRequest(json.requestURI,
                                              json.originalRequestURI,