From f02d1fa467c613aa2579de99a4fb894fb378a302 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Wed, 30 Oct 2019 19:10:21 +0000 Subject: [PATCH] Bug 1591183 - fix tabbrowser's expectations about state_start with documentchannel and add a test, r=dao Differential Revision: https://phabricator.services.mozilla.com/D51081 --HG-- extra : moz-landing-system : lando --- browser/base/content/tabbrowser.js | 47 +++++------ browser/base/content/test/tabs/browser.ini | 1 + ...rowser_progress_keyword_search_handling.js | 80 +++++++++++++++++++ .../BrowserTestUtils/BrowserTestUtils.jsm | 7 +- 4 files changed, 107 insertions(+), 28 deletions(-) create mode 100644 browser/base/content/test/tabs/browser_progress_keyword_search_handling.js diff --git a/browser/base/content/tabbrowser.js b/browser/base/content/tabbrowser.js index 0c8c4356d49d..dbebe576ad28 100644 --- a/browser/base/content/tabbrowser.js +++ b/browser/base/content/tabbrowser.js @@ -5625,37 +5625,28 @@ location ); + const { + STATE_START, + STATE_STOP, + STATE_IS_NETWORK, + } = Ci.nsIWebProgressListener; + // If we were ignoring some messages about the initial about:blank, and we // got the STATE_STOP for it, we'll want to pay attention to those messages // from here forward. Similarly, if we conclude that this state change // is one that we shouldn't be ignoring, then stop ignoring. if ( (ignoreBlank && - aStateFlags & Ci.nsIWebProgressListener.STATE_STOP && - aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK) || + aStateFlags & STATE_STOP && + aStateFlags & STATE_IS_NETWORK) || (!ignoreBlank && this.mBlank) ) { this.mBlank = false; } - if (aStateFlags & Ci.nsIWebProgressListener.STATE_START) { + if (aStateFlags & STATE_START && aStateFlags & STATE_IS_NETWORK) { this.mRequestCount++; - } else if (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) { - const NS_ERROR_UNKNOWN_HOST = 2152398878; - if (--this.mRequestCount > 0 && aStatus == NS_ERROR_UNKNOWN_HOST) { - // to prevent bug 235825: wait for the request handled - // by the automatic keyword resolver - return; - } - // since we (try to) only handle STATE_STOP of the last request, - // the count of open requests should now be 0 - this.mRequestCount = 0; - } - if ( - aStateFlags & Ci.nsIWebProgressListener.STATE_START && - aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK - ) { if (aWebProgress.isTopLevel) { // Need to use originalLocation rather than location because things // like about:home and about:privatebrowsing arrive with nsIRequest @@ -5705,10 +5696,16 @@ gBrowser._isBusy = true; } } - } else if ( - aStateFlags & Ci.nsIWebProgressListener.STATE_STOP && - aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK - ) { + } else if (aStateFlags & STATE_STOP && aStateFlags & STATE_IS_NETWORK) { + if (--this.mRequestCount > 0 && aStatus == Cr.NS_ERROR_UNKNOWN_HOST) { + // to prevent bug 235825: wait for the request handled + // by the automatic keyword resolver + return; + } + // since we (try to) only handle STATE_STOP of the last request, + // the count of open requests should now be 0 + this.mRequestCount = 0; + let modifiedAttrs = []; if (this.mTab.hasAttribute("busy")) { this.mTab.removeAttribute("busy"); @@ -5805,11 +5802,7 @@ false ); - if ( - aStateFlags & - (Ci.nsIWebProgressListener.STATE_START | - Ci.nsIWebProgressListener.STATE_STOP) - ) { + if (aStateFlags & (STATE_START | STATE_STOP)) { // reset cached temporary values at beginning and end this.mMessage = ""; this.mTotalProgress = 0; diff --git a/browser/base/content/test/tabs/browser.ini b/browser/base/content/test/tabs/browser.ini index 91708bd45f43..6d980502557f 100644 --- a/browser/base/content/test/tabs/browser.ini +++ b/browser/base/content/test/tabs/browser.ini @@ -83,6 +83,7 @@ support-files = file_anchor_elements.html [browser_positional_attributes.js] skip-if = (verify && (os == 'win' || os == 'mac')) [browser_preloadedBrowser_zoom.js] +[browser_progress_keyword_search_handling.js] [browser_reload_deleted_file.js] skip-if = (debug && os == 'mac') || (debug && os == 'linux' && bits == 64) #Bug 1421183, disabled on Linux/OSX for leaked windows [browser_tabCloseSpacer.js] diff --git a/browser/base/content/test/tabs/browser_progress_keyword_search_handling.js b/browser/base/content/test/tabs/browser_progress_keyword_search_handling.js new file mode 100644 index 000000000000..48d3d7117ffc --- /dev/null +++ b/browser/base/content/test/tabs/browser_progress_keyword_search_handling.js @@ -0,0 +1,80 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const kButton = document.getElementById("reload-button"); +const kDocChanPref = "browser.tabs.documentchannel"; + +add_task(async function setup() { + await SpecialPowers.pushPrefEnv({ + set: [["browser.fixup.dns_first_for_single_words", true]], + }); + registerCleanupFunction(() => { + Services.prefs.clearUserPref(kDocChanPref); + }); +}); + +/* + * When loading a keyword search as a result of an unknown host error, + * check that we can stop the load. + * See https://bugzilla.mozilla.org/show_bug.cgi?id=235825 + */ +add_task(async function test_unknown_host() { + for (let docChan of [true, false]) { + Services.prefs.setBoolPref(kDocChanPref, docChan); + await BrowserTestUtils.withNewTab("about:blank", async browser => { + const kNonExistingHost = "idontreallyexistonthisnetwork"; + let searchPromise = BrowserTestUtils.waitForDocLoadAndStopIt( + Services.uriFixup.keywordToURI(kNonExistingHost).preferredURI.spec, + browser, + () => { + ok(kButton.hasAttribute("displaystop"), "Should be showing stop"); + return true; + } + ); + gURLBar.value = kNonExistingHost; + gURLBar.select(); + EventUtils.synthesizeKey("KEY_Enter"); + await searchPromise; + await BrowserTestUtils.waitForCondition( + () => !kButton.hasAttribute("displaystop") + ); + ok( + !kButton.hasAttribute("displaystop"), + "Should no longer be showing stop after search" + ); + }); + } +}); + +/* + * When NOT loading a keyword search as a result of an unknown host error, + * check that the stop button goes back to being a reload button. + * See https://bugzilla.mozilla.org/show_bug.cgi?id=1591183 + */ +add_task(async function test_unknown_host_without_search() { + for (let docChan of [true, false]) { + Services.prefs.setBoolPref(kDocChanPref, docChan); + await BrowserTestUtils.withNewTab("about:blank", async browser => { + const kNonExistingHost = "idontreallyexistonthisnetwork.example.com"; + let searchPromise = BrowserTestUtils.browserLoaded( + browser, + false, + "http://" + kNonExistingHost + "/", + true /* want an error page */ + ); + gURLBar.value = kNonExistingHost; + gURLBar.select(); + EventUtils.synthesizeKey("KEY_Enter"); + await searchPromise; + await BrowserTestUtils.waitForCondition( + () => !kButton.hasAttribute("displaystop") + ); + ok( + !kButton.hasAttribute("displaystop"), + "Should not be showing stop on error page" + ); + }); + } +}); diff --git a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm index fbacc89ceddd..9bfefd5cd482 100644 --- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm +++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm @@ -1422,9 +1422,11 @@ var BrowserTestUtils = { * The URL of the document that is expected to load. * @param {object} browser * The browser to wait for. + * @param {function} checkFn (optional) + * Function to run on the channel before stopping it. * @returns {Promise} */ - waitForDocLoadAndStopIt(expectedURL, browser) { + waitForDocLoadAndStopIt(expectedURL, browser, checkFn) { let isHttp = url => /^https?:/.test(url); let stoppedDocLoadPromise = () => { @@ -1464,6 +1466,9 @@ var BrowserTestUtils = { if (!chan.originalURI || chan.originalURI.spec !== expectedURL) { return; } + if (checkFn && !checkFn(chan)) { + return; + } // TODO: We should check that the channel's BrowsingContext matches // the browser's. See bug 1587114.