Skip to content

Commit

Permalink
Bug 1671983 - Part 1: Remove now-unused process switching shouldLoadU…
Browse files Browse the repository at this point in the history 
…RI methods, r=annyG,geckoview-reviewers,snorp

These methods are no longer necessary, as all loads which can trigger process
switches now go through DocumentChannel.

The shouldLoadURI methods on nsIWebBrowserChrome3 are unfortunately still
necessary as they're used by the disabled-by-default "Single-Site Browser"
feature. In the future this may be possible to clean-up.

Differential Revision: https://phabricator.services.mozilla.com/D94638
  • Loading branch information
@mystor
mystor committed Nov 12, 2020
1 parent 67a1f9e commit 9ff27ad
Show file tree
Hide file tree
Showing 9 changed files with 6 additions and 291 deletions.
9 changes: 0 additions & 9 deletions browser/actors/BrowserTabParent.jsm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,6 @@ class BrowserTabParent extends JSWindowActorParent {
browser.ownerGlobal.gBrowserInit._firstContentWindowPaintDeferred.resolve();
break;
}

case "Browser:LoadURI": {
if (gBrowser.sessionHistoryInParent) {
message.data.historyIndex =
browsingContext.sessionHistory.requestedIndex;
}
gBrowser.ownerGlobal.RedirectLoad(browser, message.data);
break;
}
}
}
}
76 changes: 3 additions & 73 deletions browser/base/content/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1639,32 +1639,6 @@ function LoadInOtherProcess(browser, loadOptions, historyIndex = -1) {
SessionStore.navigateAndRestore(tab, loadOptions, historyIndex);
}

// Called when a docshell has attempted to load a page in an incorrect process.
// This function is responsible for loading the page in the correct process.
function RedirectLoad(browser, data) {
if (browser.getAttribute("preloadedState") === "consumed") {
browser.removeAttribute("preloadedState");
data.loadOptions.newFrameloader = true;
}

// We should only start the redirection if the browser window has finished
// starting up. Otherwise, we should wait until the startup is done.
if (gBrowserInit.delayedStartupFinished) {
LoadInOtherProcess(browser, data.loadOptions, data.historyIndex);
} else {
let delayedStartupFinished = (subject, topic) => {
if (topic == "browser-delayed-startup-finished" && subject == window) {
Services.obs.removeObserver(delayedStartupFinished, topic);
LoadInOtherProcess(browser, data.loadOptions, data.historyIndex);
}
};
Services.obs.addObserver(
delayedStartupFinished,
"browser-delayed-startup-finished"
);
}
}

let _resolveDelayedStartup;
var delayedStartupPromise = new Promise(resolve => {
_resolveDelayedStartup = resolve;
Expand Down Expand Up @@ -5193,50 +5167,6 @@ var XULBrowserWindow = {
);
},

// Check whether this URI should load in the current process
shouldLoadURI(
aDocShell,
aURI,
aReferrerInfo,
aHasPostData,
aTriggeringPrincipal,
aCsp
) {
if (!gMultiProcessBrowser) {
return true;
}

let browser = aDocShell
.QueryInterface(Ci.nsIDocShellTreeItem)
.sameTypeRootTreeItem.QueryInterface(Ci.nsIDocShell).chromeEventHandler;

// Ignore loads that aren't in the main tabbrowser
if (
browser.localName != "browser" ||
!browser.getTabBrowser ||
browser.getTabBrowser() != gBrowser
) {
return true;
}

if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aHasPostData)) {
// XXX: Do we want to complain if we have post data but are still
// redirecting the load? Perhaps a telemetry probe? Theoretically we
// shouldn't do this, as it throws out data. See bug 1348018.
E10SUtils.redirectLoad(
aDocShell,
aURI,
aReferrerInfo,
aTriggeringPrincipal,
null,
aCsp
);
return false;
}

return true;
},

onProgressChange(
aWebProgress,
aRequest,
Expand Down Expand Up @@ -6253,9 +6183,9 @@ nsBrowserAccess.prototype = {
// If we have an opener, that means that the caller is expecting access
// to the nsIDOMWindow of the opened tab right away. For e10s windows,
// this means forcing the newly opened browser to be non-remote so that
// we can hand back the nsIDOMWindow. The XULBrowserWindow.shouldLoadURI
// will do the job of shuttling off the newly opened browser to run in
// the right process once it starts loading a URI.
// we can hand back the nsIDOMWindow. DocumentLoadListener will do the
// job of shuttling off the newly opened browser to run in the right
// process once it starts loading a URI.
let forceNotRemote = aOpenWindowInfo && !aOpenWindowInfo.isRemote;
let userContextId = aOpenWindowInfo
? aOpenWindowInfo.originAttributes.userContextId
Expand Down
21 changes: 1 addition & 20 deletions browser/base/content/tab-content.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@

var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");

ChromeUtils.defineModuleGetter(
this,
"E10SUtils",
"resource://gre/modules/E10SUtils.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"BrowserUtils",
Expand Down Expand Up @@ -42,25 +37,11 @@ var WebBrowserChrome = {
aTriggeringPrincipal,
aCsp
) {
if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aHasPostData)) {
E10SUtils.redirectLoad(
aDocShell,
aURI,
aReferrerInfo,
aTriggeringPrincipal,
null,
aCsp
);
return false;
}

return true;
},

shouldLoadURIInThisProcess(aURI) {
let remoteSubframes = docShell.QueryInterface(Ci.nsILoadContext)
.useRemoteSubframes;
return E10SUtils.shouldLoadURIInThisProcess(aURI, remoteSubframes);
return true;
},
};

Expand Down
5 changes: 0 additions & 5 deletions dom/security/test/general/browser_test_referrer_loadInOtherProcess.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,6 @@ var waitForLoad = async function(uri) {

// Tests referrerInfo when navigating from a page in the remote process to main
// process and vice versa.
// The changing process code flow is (cpp) docshell.shouldLoadURI
// -> (JS) browser.shouldLoadURI -> E10sUtils.redirectLoad
// -> ContentRestore.restoreTabContent.
// Finally, docshell will do the load in correct process with the input
// referrerInfo and store an entry to SessionHistory
add_task(async function test_navigation() {
// Navigate from non remote to remote
gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser, "about:blank");
Expand Down
19 changes: 1 addition & 18 deletions mobile/android/actors/WebBrowserChromeChild.jsm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const { GeckoViewActorChild } = ChromeUtils.import(

XPCOMUtils.defineLazyModuleGetters(this, {
BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
E10SUtils: "resource://gre/modules/E10SUtils.jsm",
GeckoViewSettings: "resource://gre/modules/GeckoViewSettings.jsm",
});

Expand Down Expand Up @@ -40,28 +39,12 @@ class WebBrowserChromeChild extends GeckoViewActorChild {
aTriggeringPrincipal,
aCsp
) {
debug`shouldLoadURI ${aURI.displaySpec}`;

if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aHasPostData)) {
E10SUtils.redirectLoad(
aDocShell,
aURI,
aReferrerInfo,
aTriggeringPrincipal,
null,
aCsp
);
return false;
}

return true;
}

// nsIWebBrowserChrome
shouldLoadURIInThisProcess(aURI) {
debug`shouldLoadURIInThisProcess ${aURI.displaySpec}`;
const remoteSubframes = this.docShell.nsILoadContext.useRemoteSubframes;
return E10SUtils.shouldLoadURIInThisProcess(aURI, remoteSubframes);
return true;
}
}

Expand Down
5 changes: 1 addition & 4 deletions toolkit/components/extensions/ext-browser-content.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ var { XPCOMUtils } = ChromeUtils.import(
XPCOMUtils.defineLazyModuleGetters(this, {
BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
clearTimeout: "resource://gre/modules/Timer.jsm",
E10SUtils: "resource://gre/modules/E10SUtils.jsm",
ExtensionCommon: "resource://gre/modules/ExtensionCommon.jsm",
setTimeout: "resource://gre/modules/Timer.jsm",
});
Expand Down Expand Up @@ -336,9 +335,7 @@ var WebBrowserChrome = {
},

shouldLoadURIInThisProcess(URI) {
let remoteSubframes = docShell.QueryInterface(Ci.nsILoadContext)
.useRemoteSubframes;
return E10SUtils.shouldLoadURIInThisProcess(URI, remoteSubframes);
return true;
},
};

Expand Down
127 changes: 0 additions & 127 deletions toolkit/modules/E10SUtils.jsm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -972,133 +972,6 @@ var E10SUtils = {
};
},

shouldLoadURIInThisProcess(aURI, aRemoteSubframes) {
let remoteType = Services.appinfo.remoteType;
let wantRemoteType = this.getRemoteTypeForURIObject(
aURI,
/* remote */ true,
aRemoteSubframes,
remoteType
);
this.log().info(
`shouldLoadURIInThisProcess: have ${remoteType} want ${wantRemoteType}`
);

if (documentChannelPermittedForURI(aURI)) {
// We can switch later with documentchannel.
return true;
}

return remoteType == wantRemoteType;
},

shouldLoadURI(aDocShell, aURI, aHasPostData) {
let { useRemoteSubframes } = aDocShell;
this.log().debug(`shouldLoadURI(${this._uriStr(aURI)})`);

let remoteType = Services.appinfo.remoteType;

if (aDocShell.browsingContext.parent) {
return true;
}

let webNav = aDocShell.QueryInterface(Ci.nsIWebNavigation);
let sessionHistory = webNav.sessionHistory;
let wantRemoteType = this.getRemoteTypeForURIObject(
aURI,
true,
useRemoteSubframes,
remoteType,
webNav.currentURI
);

// If we are using DocumentChannel or remote subframes (fission), we
// can start the load in the current process, and then perform the
// switch later-on using the DocumentLoadListener mechanism.
if (documentChannelPermittedForURI(aURI)) {
return true;
}

if (
!aHasPostData &&
remoteType == WEB_REMOTE_TYPE &&
sessionHistory.count == 1 &&
webNav.currentURI.spec == "about:newtab"
) {
// This is possibly a preloaded browser and we're about to navigate away for
// the first time. On the child side there is no way to tell for sure if that
// is the case, so let's redirect this request to the parent to decide if a new
// process is needed. But we don't currently properly handle POST data in
// redirects (bug 1457520), so if there is POST data, don't return false here.
return false;
}

// Allow history load if loaded in this process before.
if (!Services.appinfo.sessionHistoryInParent) {
let requestedIndex = sessionHistory.legacySHistory.requestedIndex;
if (requestedIndex >= 0) {
this.log().debug("Checking history case\n");
if (
sessionHistory.legacySHistory.getEntryAtIndex(requestedIndex)
.loadedInThisProcess
) {
this.log().info("History entry loaded in this process");
return true;
}

// If not originally loaded in this process allow it if the URI would
// normally be allowed to load in this process by default.
this.log().debug(
`Checking remote type, got: ${remoteType} want: ${wantRemoteType}\n`
);
return remoteType == wantRemoteType;
}
}

// If the URI can be loaded in the current process then continue
return remoteType == wantRemoteType;
},

redirectLoad(
aDocShell,
aURI,
aReferrerInfo,
aTriggeringPrincipal,
aFlags,
aCsp
) {
const actor = aDocShell.domWindow.windowGlobalChild.getActor("BrowserTab");

let loadOptions = {
uri: aURI.spec,
flags: aFlags || Ci.nsIWebNavigation.LOAD_FLAGS_NONE,
referrerInfo: this.serializeReferrerInfo(aReferrerInfo),
triggeringPrincipal: this.serializePrincipal(
aTriggeringPrincipal ||
Services.scriptSecurityManager.createNullPrincipal({})
),
csp: aCsp ? this.serializeCSP(aCsp) : null,
};
// Retarget the load to the correct process
if (!Services.appinfo.sessionHistoryInParent) {
let sessionHistory = aDocShell.QueryInterface(Ci.nsIWebNavigation)
.sessionHistory;
actor.sendAsyncMessage("Browser:LoadURI", {
loadOptions,
historyIndex: sessionHistory.legacySHistory.requestedIndex,
});
} else {
// If we can't access legacySHistory, session history in the
// parent is enabled. Browser:LoadURI knows about this and will
// act accordingly.
actor.sendAsyncMessage("Browser:LoadURI", {
loadOptions,
});
}

return false;
},

wrapHandlingUserInput(aWindow, aIsHandling, aCallback) {
var handlingUserInput;
try {
Expand Down
10 changes: 0 additions & 10 deletions xpfe/appshell/nsContentTreeOwner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,16 +386,6 @@ NS_IMETHODIMP nsContentTreeOwner::ShouldLoadURI(
nsIDocShell* aDocShell, nsIURI* aURI, nsIReferrerInfo* aReferrerInfo,
bool aHasPostData, nsIPrincipal* aTriggeringPrincipal,
nsIContentSecurityPolicy* aCsp, bool* _retval) {
NS_ENSURE_STATE(mAppWindow);

nsCOMPtr<nsIXULBrowserWindow> xulBrowserWindow;
mAppWindow->GetXULBrowserWindow(getter_AddRefs(xulBrowserWindow));

if (xulBrowserWindow)
return xulBrowserWindow->ShouldLoadURI(aDocShell, aURI, aReferrerInfo,
aHasPostData, aTriggeringPrincipal,
aCsp, _retval);

*_retval = true;
return NS_OK;
}
Expand Down
Loading

0 comments on commit 9ff27ad

Please sign in to comment.