Bug 798249 - track when we're 'inside' a loadURI call when dealing with Stop() calls resulting from the same, r=mconley

MozReview-Commit-ID: 48xQn03HtjZ --HG-- extra : rebase_source : 29669594a65d2f8710445bb8ae9b4b92c5834c91
2016-02-16 14:08:14 +00:00 · 2016-02-16 14:08:14 +00:00 · e4f6985644
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@ -830,10 +830,16 @@ function _loadURIWithFlags(browser, uri, params) {
    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,
@ -867,6 +873,10 @@ function _loadURIWithFlags(browser, uri, params) {
      throw e;
    }
  } finally {
+    if ((!wasRemote && !mustChangeProcess) ||
+        (wasRemote && mustChangeProcess)) {
+      browser.inLoadURI = false;
+    }
    if (browser.userTypedClear) {
      browser.userTypedClear--;
    }
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@ -684,8 +684,10 @@

                    this.mBrowser.userTypedValue = null;

-                    if (this.mTab.selected && gURLBar)
+                    let inLoadURI = this.mBrowser.inLoadURI;
+                    if (this.mTab.selected && gURLBar && !inLoadURI) {
                      URLBarSetURI();
+                    }
                  } else {
                    // The document is done loading, we no longer want the
                    // value cleared.
@ -1320,7 +1322,7 @@
        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;
--- a/browser/base/content/test/general/browser_urlbarStop.js
+++ b/browser/base/content/test/general/browser_urlbarStop.js
@ -25,7 +25,7 @@ function typeAndSubmitAndStop(url) {
  URLBarSetURI();
  is(gURLBar.textValue, gURLBar.trimValue(url), "location bar reflects loading page");

-  let promise = waitForDocLoadAndStopIt(url);
+  let promise = waitForDocLoadAndStopIt(url, gBrowser.selectedBrowser, false);
  gURLBar.handleCommand();
  return promise;
 }
--- a/browser/base/content/test/general/head.js
+++ b/browser/base/content/test/general/head.js
@ -382,14 +382,33 @@ function promiseHistoryClearedState(aURIs, aShouldBeCleared) {
 *
 * @param aExpectedURL
 *        The URL of the document that is expected to load.
+ * @param aStopFromProgressListener
+ *        Whether to cancel the load directly from the progress listener. Defaults to true.
+ *        If you're using this method to avoid hitting the network, you want the default (true).
+ *        However, the browser UI will behave differently for loads stopped directly from
+ *        the progress listener (effectively in the middle of a call to loadURI) and so there
+ *        are cases where you may want to avoid stopping the load directly from within the
+ *        progress listener callback.
 * @return promise
 */
-function waitForDocLoadAndStopIt(aExpectedURL, aBrowser=gBrowser.selectedBrowser) {
-  function content_script() {
+function waitForDocLoadAndStopIt(aExpectedURL, aBrowser=gBrowser.selectedBrowser, aStopFromProgressListener=true) {
+  function content_script(aStopFromProgressListener) {
    let { interfaces: Ci, utils: Cu } = Components;
    Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
    let wp = docShell.QueryInterface(Ci.nsIWebProgress);

+    function stopContent(now, uri) {
+      if (now) {
+        /* Hammer time. */
+        content.stop();
+
+        /* Let the parent know we're done. */
+        sendAsyncMessage("Test:WaitForDocLoadAndStopIt", { uri });
+      } else {
+        setTimeout(stopContent.bind(null, true, uri), 0);
+      }
+    }
+
    let progressListener = {
      onStateChange: function (webProgress, req, flags, status) {
        dump("waitForDocLoadAndStopIt: onStateChange " + flags.toString(16) + ": " + req.name + "\n");
@ -401,11 +420,7 @@ function waitForDocLoadAndStopIt(aExpectedURL, aBrowser=gBrowser.selectedBrowser
          let chan = req.QueryInterface(Ci.nsIChannel);
          dump(`waitForDocLoadAndStopIt: Document start: ${chan.URI.spec}\n`);

-          /* Hammer time. */
-          content.stop();
-
-          /* Let the parent know we're done. */
-          sendAsyncMessage("Test:WaitForDocLoadAndStopIt", { uri: chan.originalURI.spec });
+          stopContent(aStopFromProgressListener, chan.originalURI.spec);
        }
      },
      QueryInterface: XPCOMUtils.generateQI(["nsISupportsWeakReference"])
@ -432,7 +447,7 @@ function waitForDocLoadAndStopIt(aExpectedURL, aBrowser=gBrowser.selectedBrowser
    }

    let mm = aBrowser.messageManager;
-    mm.loadFrameScript("data:,(" + content_script.toString() + ")();", true);
+    mm.loadFrameScript("data:,(" + content_script.toString() + ")(" + aStopFromProgressListener + ");", true);
    mm.addMessageListener("Test:WaitForDocLoadAndStopIt", complete);
    info("waitForDocLoadAndStopIt: Waiting for URL: " + aExpectedURL);
  });
--- a/browser/base/content/test/urlbar/browser.ini
+++ b/browser/base/content/test/urlbar/browser.ini
@ -2,3 +2,6 @@
 [browser_urlbar_blanking.js]
 support-files =
  file_blank_but_not_blank.html
+[browser_urlbar_stop_pending.js]
+support-files =
+  slow-page.sjs
--- a/browser/base/content/test/urlbar/browser_urlbar_stop_pending.js
+++ b/browser/base/content/test/urlbar/browser_urlbar_stop_pending.js
@ -0,0 +1,42 @@
+"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";
+
+/**
+ * Check that if we:
+ * 1) have a loaded page
+ * 2) load a separate URL
+ * 3) before the URL for step 2 has finished loading, load a third URL
+ * we don't revert to the URL from (1).
+ */
+add_task(function*() {
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com", true, true);
+
+  let expectedURLBarChange = SLOW_PAGE;
+  let sawChange = false;
+  let handler = e => {
+    sawChange = true;
+    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;
+  ok(sawChange, "The URL bar change handler should have been called by the time the page was loaded");
+  obs.disconnect();
+  obs = null;
+  yield BrowserTestUtils.removeTab(tab);
+});
+
--- a/browser/base/content/test/urlbar/slow-page.sjs
+++ 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/toolkit/content/browser-child.js
+++ b/toolkit/content/browser-child.js
@ -132,6 +132,7 @@ var WebProgressListener = {
      json.mayEnableCharacterEncodingMenu = docShell.mayEnableCharacterEncodingMenu;
    }

+    json.inLoadURI = WebNavigation.inLoadURI;
    this._send("Content:StateChange", json, objects);
  },

@ -208,6 +209,10 @@ var WebProgressListener = {
    return true;
  },

+  sendLoadCallResult() {
+    this._send("Content:LoadURIResult");
+  },
+
  QueryInterface: function QueryInterface(aIID) {
    if (aIID.equals(Ci.nsIWebProgressListener) ||
        aIID.equals(Ci.nsIWebProgressListener2) ||
@ -239,6 +244,12 @@ var WebNavigation =  {
    return docShell.QueryInterface(Ci.nsIWebNavigation);
  },

+  _inLoadURI: false,
+
+  get inLoadURI() {
+    return this._inLoadURI;
+  },
+
  receiveMessage: function(message) {
    switch (message.name) {
      case "WebNavigation:GoBack":
@ -300,8 +311,14 @@ var WebNavigation =  {
      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;
+      WebProgressListener.sendLoadCallResult();
+    }
  },

  reload: function(flags) {
--- a/toolkit/modules/RemoteWebProgress.jsm
+++ b/toolkit/modules/RemoteWebProgress.jsm
@ -117,6 +117,7 @@ RemoteWebProgressManager.prototype = {
      this._messageManager.removeMessageListener("Content:SecurityChange", this);
      this._messageManager.removeMessageListener("Content:StatusChange", this);
      this._messageManager.removeMessageListener("Content:ProgressChange", this);
+      this._messageManager.removeMessageListener("Content:LoadURIResult", this);
    }

    this._browser = aBrowser;
@ -126,6 +127,7 @@ RemoteWebProgressManager.prototype = {
    this._messageManager.addMessageListener("Content:SecurityChange", this);
    this._messageManager.addMessageListener("Content:StatusChange", this);
    this._messageManager.addMessageListener("Content:ProgressChange", this);
+    this._messageManager.addMessageListener("Content:LoadURIResult", this);
  },

  get topLevelWebProgress() {
@ -211,6 +213,7 @@ RemoteWebProgressManager.prototype = {
    if (isTopLevel) {
      this._browser._contentWindow = objects.contentWindow;
      this._browser._documentContentType = json.documentContentType;
+      this._browser.inLoadURI = json.inLoadURI;
      if (json.charset) {
        this._browser._characterSet = json.charset;
        this._browser._mayEnableCharacterEncodingMenu = json.mayEnableCharacterEncodingMenu;
@ -268,6 +271,9 @@ RemoteWebProgressManager.prototype = {
    case "Content:ProgressChange":
      this._callProgressListeners("onProgressChange", webProgress, request, json.curSelf, json.maxSelf, json.curTotal, json.maxTotal);
      break;
+    case "Content:LoadURIResult":
+      this._browser.inLoadURI = false;
+      break;
    }
  },
 };