Skip to content

Commit

Permalink
Bug 1654922 - Part 1: Remove DocumentChannel pref usage from tests an…
Browse files Browse the repository at this point in the history
…d document navigation code, r=mattwoodrow

Differential Revision: https://phabricator.services.mozilla.com/D85483
  • Loading branch information
annygakh committed Aug 1, 2020
1 parent 126f58b commit f2c5b14
Show file tree
Hide file tree
Showing 19 changed files with 59 additions and 204 deletions.
5 changes: 0 additions & 5 deletions browser/base/content/tabbrowser.js
Original file line number Diff line number Diff line change
Expand Up @@ -6007,11 +6007,6 @@
}
}
} 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@
"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);
});

await Services.search.init();

// Create an engine to use for the test.
Expand All @@ -37,31 +34,28 @@ add_task(async function setup() {
* 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.browserStarted(
browser,
Services.uriFixup.keywordToURI(kNonExistingHost).preferredURI.spec
);
await BrowserTestUtils.withNewTab("about:blank", async browser => {
const kNonExistingHost = "idontreallyexistonthisnetwork";
let searchPromise = BrowserTestUtils.browserStarted(
browser,
Services.uriFixup.keywordToURI(kNonExistingHost).preferredURI.spec
);

gURLBar.value = kNonExistingHost;
gURLBar.select();
EventUtils.synthesizeKey("KEY_Enter");
gURLBar.value = kNonExistingHost;
gURLBar.select();
EventUtils.synthesizeKey("KEY_Enter");

await searchPromise;
ok(kButton.hasAttribute("displaystop"), "Should be showing stop");
await searchPromise;
ok(kButton.hasAttribute("displaystop"), "Should be showing stop");

await BrowserTestUtils.waitForCondition(
() => !kButton.hasAttribute("displaystop")
);
ok(
!kButton.hasAttribute("displaystop"),
"Should no longer be showing stop after search"
);
});
}
await BrowserTestUtils.waitForCondition(
() => !kButton.hasAttribute("displaystop")
);
ok(
!kButton.hasAttribute("displaystop"),
"Should no longer be showing stop after search"
);
});
});

/*
Expand All @@ -70,27 +64,24 @@ add_task(async function test_unknown_host() {
* 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"
);
});
}
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"
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ let setupTest = async function(
],
["browser.tabs.remote.useCrossOriginOpenerPolicy", crossOriginIsolated],
["browser.tabs.remote.useCrossOriginEmbedderPolicy", crossOriginIsolated],
["browser.tabs.documentchannel", crossOriginIsolated],
],
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ let setupAndRunCrossOriginIsolatedTest = async function(
],
["browser.tabs.remote.useCrossOriginOpenerPolicy", crossOriginIsolated],
["browser.tabs.remote.useCrossOriginEmbedderPolicy", crossOriginIsolated],
["browser.tabs.documentchannel", crossOriginIsolated],
],
});

Expand Down
3 changes: 0 additions & 3 deletions docshell/base/CanonicalBrowsingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,6 @@ bool CanonicalBrowsingContext::LoadInParent(nsDocShellLoadState* aLoadState,
// We currently only support starting loads directly from the
// CanonicalBrowsingContext for top-level BCs.
if (!IsTopContent() || !GetContentParent() ||
!StaticPrefs::browser_tabs_documentchannel() ||
!StaticPrefs::browser_tabs_documentchannel_parent_controlled()) {
return false;
}
Expand All @@ -1110,8 +1109,6 @@ bool CanonicalBrowsingContext::AttemptSpeculativeLoadInParent(
// We currently only support starting loads directly from the
// CanonicalBrowsingContext for top-level BCs.
if (!IsTopContent() || !GetContentParent() ||
!StaticPrefs::browser_tabs_documentchannel() ||
!StaticPrefs::browser_tabs_documentchannel_parent_initiated() ||
StaticPrefs::browser_tabs_documentchannel_parent_controlled()) {
return false;
}
Expand Down
68 changes: 2 additions & 66 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6238,73 +6238,9 @@ nsresult nsDocShell::EndPageLoad(nsIWebProgress* aProgress,
// Test whether this is the top frame or a subframe
bool isTopFrame = mBrowsingContext->IsTop();

//
// If the page load failed, then deal with the error condition...
// Errors are handled as follows:
// 1. Check to see if it's a file not found error or bad content
// encoding error.
// 2. Send the URI to a keyword server (if enabled)
// 3. If the error was DNS failure, then add www and .com to the URI
// (if appropriate).
// 4. If the www .com additions don't work, try those with an HTTPS scheme
// (if appropriate).
// 5. Throw an error dialog box...
//
// If status code indicates an error it means that DocumentChannel already
// tried to fixup the uri and failed. Throw an error dialog box here.
if (NS_FAILED(aStatus)) {
nsCOMPtr<nsIInputStream> newPostData;
nsCOMPtr<nsIURI> newURI =
AttemptURIFixup(aChannel, aStatus, Some(mOriginalUriString), mLoadType,
isTopFrame, mAllowKeywordFixup, UsePrivateBrowsing(),
true, getter_AddRefs(newPostData));
if (newURI) {
nsAutoCString newSpec;
newURI->GetSpec(newSpec);
NS_ConvertUTF8toUTF16 newSpecW(newSpec);

nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
MOZ_ASSERT(loadInfo, "loadInfo is required on all channels");
nsCOMPtr<nsIPrincipal> triggeringPrincipal =
loadInfo->TriggeringPrincipal();

// If the new URI is HTTP, it may not work and we may want to fall
// back to HTTPS, so kick off a speculative connect to get that
// started. Even if we don't adjust to HTTPS here, there could be a
// redirect to HTTPS coming so this could speed things up.
if (SchemeIsHTTP(url)) {
int32_t port = 0;
rv = url->GetPort(&port);

// only do this if the port is default.
if (NS_SUCCEEDED(rv) && port == -1) {
nsCOMPtr<nsIURI> httpsURI;
rv = NS_MutateURI(url)
.SetScheme("https"_ns)
.Finalize(getter_AddRefs(httpsURI));

if (NS_SUCCEEDED(rv)) {
nsCOMPtr<nsIIOService> ios = do_GetIOService();
if (ios) {
nsCOMPtr<nsISpeculativeConnect> speculativeService =
do_QueryInterface(ios);
if (speculativeService) {
speculativeService->SpeculativeConnect(
httpsURI, triggeringPrincipal, nullptr);
}
}
}
}
}

LoadURIOptions loadURIOptions;
loadURIOptions.mTriggeringPrincipal = triggeringPrincipal;
loadURIOptions.mCsp = loadInfo->GetCspToInherit();
loadURIOptions.mPostData = newPostData;
return LoadURI(newSpecW, loadURIOptions);
}

// Well, fixup didn't work :-(
// It is time to throw an error dialog box, and be done with it...

// If we got CONTENT_BLOCKED from EndPageLoad, then we need to fire
// the error event to our embedder, since tests are relying on this.
// The error event is usually fired by the caller of InternalLoad, but
Expand Down
9 changes: 9 additions & 0 deletions docshell/base/nsDocShell.h
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,15 @@ class nsDocShell final : public nsDocLoader,
nsLoadFlags aLoadFlags, uint32_t aCacheKey, nsresult& rv,
nsIChannel** aChannel);

// This is used to deal with errors resulting from a failed page load.
// Errors are handled as follows:
// 1. Check to see if it's a file not found error or bad content
// encoding error.
// 2. Send the URI to a keyword server (if enabled)
// 3. If the error was DNS failure, then add www and .com to the URI
// (if appropriate).
// 4. If the www .com additions don't work, try those with an HTTPS scheme
// (if appropriate).
static already_AddRefed<nsIURI> AttemptURIFixup(
nsIChannel* aChannel, nsresult aStatus,
const mozilla::Maybe<nsCString>& aOriginalURIString, uint32_t aLoadType,
Expand Down
6 changes: 1 addition & 5 deletions dom/base/test/chrome/test_bug1339722.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@
// This topic used to be http-on-useragent-request, but that got removed in
// bug 1513574. on-modify-request is called around the same time, and should
// behave similarly.
const TOPIC =
SpecialPowers.getBoolPref("browser.tabs.documentchannel") &&
SpecialPowers.getBoolPref("browser.tabs.documentchannel.ppdc")
? "document-on-modify-request"
: "http-on-modify-request";
const TOPIC = "document-on-modify-request";
let win;
const observe = (subject, topic, data) => {
info("Got " + topic);
Expand Down
7 changes: 0 additions & 7 deletions dom/html/test/browser_form_post_from_file_to_http.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,6 @@ async function runTest(doNewTab) {
}

add_task(async function runWithDocumentChannel() {
// Set prefs to use documentchannel.
await SpecialPowers.pushPrefEnv({
set: [["browser.tabs.documentchannel", true]],
});

await runTest(false);
await runTest(true);

await SpecialPowers.popPrefEnv();
});
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ async function runTestcase(aTab, aTestcase) {
add_task(async function setupPrefs() {
await SpecialPowers.pushPrefEnv({
set: [
["browser.tabs.documentchannel", true],
["dom.serviceWorkers.enabled", true],
["dom.serviceWorkers.testing.enabled", true],
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ async function runTest() {
// requests to `ORIGIN` will be marked as controlled.
await SpecialPowers.pushPrefEnv({
set: [
["browser.tabs.documentchannel", true],
["dom.serviceWorkers.enabled", true],
["dom.serviceWorkers.exemptFromPerDomainMax", true],
["dom.serviceWorkers.testing.enabled", true],
Expand Down
5 changes: 1 addition & 4 deletions dom/tests/browser/browser_windowProxy_transplant.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ add_task(async function() {
// Turn on BC preservation and frameloader rebuilding to ensure that the
// BrowsingContext is preserved.
await SpecialPowers.pushPrefEnv({
set: [
["fission.preserve_browsing_contexts", true],
["browser.tabs.documentchannel", true],
],
set: [["fission.preserve_browsing_contexts", true]],
});

// Open a window with fission force-enabled in it.
Expand Down
8 changes: 1 addition & 7 deletions netwerk/ipc/DocumentChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,18 +161,12 @@ static bool URIUsesDocChannel(nsIURI* aURI) {
}

bool DocumentChannel::CanUseDocumentChannel(nsIURI* aURI, uint32_t aLoadFlags) {
if (XRE_IsParentProcess() &&
!StaticPrefs::browser_tabs_documentchannel_ppdc()) {
return false;
}

// We want to use DocumentChannel if we're using a supported scheme. Sandboxed
// srcdoc loads break due to failing assertions after changing processes, and
// non-sandboxed srcdoc loads need to share the same principal object as their
// outer document (and must load in the same process), which breaks if we
// serialize to the parent process.
return StaticPrefs::browser_tabs_documentchannel() &&
!(aLoadFlags & nsDocShell::INTERNAL_LOAD_FLAGS_IS_SRCDOC) &&
return !(aLoadFlags & nsDocShell::INTERNAL_LOAD_FLAGS_IS_SRCDOC) &&
URIUsesDocChannel(aURI);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
prefs: [browser.tabs.remote.useCrossOriginOpenerPolicy:true, browser.tabs.remote.useCrossOriginEmbedderPolicy:true, browser.tabs.documentchannel:true]
prefs: [browser.tabs.remote.useCrossOriginOpenerPolicy:true, browser.tabs.remote.useCrossOriginEmbedderPolicy:true]
disabled:
if os == "android": no content process switch in android
leak-threshold: [default:51200, tab:102400]
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ const FILE_DUMMY = fileURL("dummy_page.html");
const DATA_URL = "data:text/html,Hello%2C World!";
const DATA_STRING = "Hello, World!";

const DOCUMENT_CHANNEL_PREF = "browser.tabs.documentchannel";

async function setPref() {
await SpecialPowers.pushPrefEnv({
set: [[DOCUMENT_CHANNEL_PREF, true]],
});
}

async function performLoad(browser, opts, action) {
let loadedPromise = BrowserTestUtils.browserLoaded(
browser,
Expand Down Expand Up @@ -222,9 +214,7 @@ async function testLoadAndRedirect(
}

add_task(async function test_enabled() {
await setPref();

// With the pref enabled, URIs should correctly switch processes & the POST
// URIs should correctly switch processes & the POST
// should succeed.
info("ENABLED -- FILE -- raw URI load");
let resp = await postFrom(FILE_DUMMY, PRINT_POSTDATA);
Expand Down Expand Up @@ -289,8 +279,6 @@ async function sendMessage(ext, method, url) {

// TODO: Currently no test framework for ftp://.
add_task(async function test_protocol() {
await setPref();

// TODO: Processes should be switched due to navigation of different origins.
await testLoadAndRedirect("data:,foo", false, true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,10 @@ const { E10SUtils } = ChromeUtils.import(
);

const COOP_PREF = "browser.tabs.remote.useCrossOriginOpenerPolicy";
const DOCUMENT_CHANNEL_PREF = "browser.tabs.documentchannel";

async function setPref() {
await SpecialPowers.pushPrefEnv({
set: [
[COOP_PREF, true],
[DOCUMENT_CHANNEL_PREF, true],
],
set: [[COOP_PREF, true]],
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const { E10SUtils } = ChromeUtils.import(
"resource://gre/modules/E10SUtils.jsm"
);

const DOCUMENT_CHANNEL_PREF = "browser.tabs.documentchannel";
const HISTORY = [
{ url: httpURL("dummy_page.html") },
{ url: fileURL("dummy_page.html") },
Expand Down
Loading

0 comments on commit f2c5b14

Please sign in to comment.