From 722233fed1ba7cf7c1871ffc4db1df2a33df4b68 Mon Sep 17 00:00:00 2001 From: Gabor Krizsanits Date: Wed, 16 Aug 2017 13:00:22 +0200 Subject: [PATCH] Bug 1376895 - Make preloaded browser use pre-existing content process. r=mconley We want to avoid to have several cached content processes, one for each preloaded browser (one per window) and one for the preallocated process. For that we force the preloaded browser to choose an existing process and during the first navigation in that tab, that leaves about:newtab, we re-run the process selecting algorithm --- browser/base/content/browser.js | 16 +++- browser/base/content/tabbrowser.xml | 4 + ...rowser_urlbarKeepStateAcrossTabSwitches.js | 4 +- browser/modules/E10SUtils.jsm | 8 ++ dom/base/nsGkAtomList.h | 1 + dom/base/test/browser.ini | 3 + .../browser_aboutnewtab_process_selection.js | 76 +++++++++++++++++++ dom/base/test/dummy.html | 9 +++ dom/ipc/ContentParent.cpp | 23 +++++- dom/ipc/ContentParent.h | 3 +- .../file_frameNavigation_blankTarget.html | 6 +- 11 files changed, 141 insertions(+), 12 deletions(-) create mode 100644 dom/base/test/browser_aboutnewtab_process_selection.js create mode 100644 dom/base/test/dummy.html diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 9a519db5a770..2215cd7cca71 100755 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -1088,6 +1088,14 @@ function _loadURIWithFlags(browser, uri, params) { } let mustChangeProcess = requiredRemoteType != currentRemoteType; + let newFrameloader = false; + if (browser.getAttribute("isPreloadBrowser") == "true" && uri != "about:newtab") { + // Leaving about:newtab from a used to be preloaded browser should run the process + // selecting algorithm again. + mustChangeProcess = true; + newFrameloader = true; + browser.removeAttribute("isPreloadBrowser"); + } // !requiredRemoteType means we're loading in the parent/this process. if (!requiredRemoteType) { @@ -1122,7 +1130,8 @@ function _loadURIWithFlags(browser, uri, params) { referrer: referrer ? referrer.spec : null, referrerPolicy, remoteType: requiredRemoteType, - postData + postData, + newFrameloader, } if (params.userContextId) { @@ -1167,6 +1176,11 @@ function LoadInOtherProcess(browser, loadOptions, historyIndex = -1) { // 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({ target: browser, data }) { + if (browser.getAttribute("isPreloadBrowser") == "true") { + browser.removeAttribute("isPreloadBrowser"); + data.loadOptions.newFrameloader = true; + } + if (data.loadOptions.reloadInFreshProcess) { // Convert the fresh process load option into a large allocation remote type // to use common processing from this point. diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml index 03878dd555b8..f62881801ba7 100644 --- a/browser/base/content/tabbrowser.xml +++ b/browser/base/content/tabbrowser.xml @@ -2118,6 +2118,10 @@ b.setAttribute("autocompletepopup", this.getAttribute("autocompletepopup")); } + if (aParams.isPreloadBrowser) { + b.setAttribute("isPreloadBrowser", "true"); + } + if (this.hasAttribute("selectmenulist")) b.setAttribute("selectmenulist", this.getAttribute("selectmenulist")); diff --git a/browser/base/content/test/urlbar/browser_urlbarKeepStateAcrossTabSwitches.js b/browser/base/content/test/urlbar/browser_urlbarKeepStateAcrossTabSwitches.js index ac0e4d4af768..216637858b1d 100644 --- a/browser/base/content/test/urlbar/browser_urlbarKeepStateAcrossTabSwitches.js +++ b/browser/base/content/test/urlbar/browser_urlbarKeepStateAcrossTabSwitches.js @@ -6,7 +6,7 @@ */ add_task(async function() { let input = "i-definitely-dont-exist.example.com"; - let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:newtab", false); + let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank", false); // NB: CPOW usage because new tab pages can be preloaded, in which case no // load events fire. await BrowserTestUtils.waitForCondition(() => !tab.linkedBrowser.contentDocument.hidden) @@ -29,7 +29,7 @@ add_task(async function() { add_task(async function() { let input = "To be or not to be-that is the question"; await SpecialPowers.pushPrefEnv({set: [["keyword.enabled", false]]}); - let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:newtab", false); + let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank", false); // NB: CPOW usage because new tab pages can be preloaded, in which case no // load events fire. await BrowserTestUtils.waitForCondition(() => !tab.linkedBrowser.contentDocument.hidden) diff --git a/browser/modules/E10SUtils.jsm b/browser/modules/E10SUtils.jsm index c003f341472b..11778c1a4b41 100644 --- a/browser/modules/E10SUtils.jsm +++ b/browser/modules/E10SUtils.jsm @@ -243,6 +243,14 @@ this.E10SUtils = { this.getRemoteTypeForURIObject(aURI, true, remoteType, webNav.currentURI); } + if (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. + return false; + } + // If the URI can be loaded in the current process then continue return this.shouldLoadURIInThisProcess(aURI); }, diff --git a/dom/base/nsGkAtomList.h b/dom/base/nsGkAtomList.h index d96c54c1f18c..b837fafefc9a 100644 --- a/dom/base/nsGkAtomList.h +++ b/dom/base/nsGkAtomList.h @@ -2231,6 +2231,7 @@ GK_ATOM(DisplayPortMargins, "_displayportmargins") GK_ATOM(DisplayPortBase, "_displayportbase") GK_ATOM(AsyncScrollLayerCreationFailed, "_asyncscrolllayercreationfailed") GK_ATOM(forcemessagemanager, "forcemessagemanager") +GK_ATOM(isPreloadBrowser, "isPreloadBrowser") // Names for system metrics GK_ATOM(color_picker_available, "color-picker-available") diff --git a/dom/base/test/browser.ini b/dom/base/test/browser.ini index ce3901c96914..cd66358b7c00 100644 --- a/dom/base/test/browser.ini +++ b/dom/base/test/browser.ini @@ -1,6 +1,7 @@ [DEFAULT] support-files = audio.ogg + dummy.html empty.html file_audioLoop.html file_audioLoopInIframe.html @@ -35,6 +36,8 @@ tags = mcb [browser_force_process_selector.js] skip-if = !e10s # this only makes sense with e10s-multi [browser_messagemanager_loadprocessscript.js] +[browser_aboutnewtab_process_selection.js] +skip-if = os == 'linux' # Linux x64 JSDCov failure [browser_messagemanager_targetframeloader.js] [browser_messagemanager_unload.js] [browser_pagehide_on_tab_close.js] diff --git a/dom/base/test/browser_aboutnewtab_process_selection.js b/dom/base/test/browser_aboutnewtab_process_selection.js new file mode 100644 index 000000000000..fc4180f249ec --- /dev/null +++ b/dom/base/test/browser_aboutnewtab_process_selection.js @@ -0,0 +1,76 @@ +const TEST_URL = "http://www.example.com/browser/dom/base/test/dummy.html"; + +var ppmm = Services.ppmm; + +add_task(async function(){ + // We want to count processes in this test, so let's disable the pre-allocated process manager. + await SpecialPowers.pushPrefEnv({"set": [ + ["dom.ipc.processPrelaunch.enabled", false], + ["dom.ipc.processCount", 10], + ["dom.ipc.keepProcessesAlive.web", 10], + ]}); +}); + +// Ensure that the preloaded browser exists, and it's finished loading. +async function ensurePreloaded(gBrowser) { + gBrowser._createPreloadBrowser(); + // We cannot use the regular BrowserTestUtils helper for waiting here, since that + // would try to insert the preloaded browser, which would only break things. + await BrowserTestUtils.waitForCondition( () => { + return gBrowser._preloadedBrowser.contentDocument.readyState == "complete"; + }); +} + +add_task(async function(){ + // This test is only relevant in e10s. + if (!gMultiProcessBrowser) + return; + + ppmm.releaseCachedProcesses(); + + // Wait for the preloaded browser to load. + await ensurePreloaded(gBrowser); + + // Store the number of processes (note: +1 for the parent process). + const { childCount: originalChildCount } = ppmm; + + // Use the preloaded browser and create another one. + BrowserOpenTab(); + let tab1 = gBrowser.selectedTab; + await ensurePreloaded(gBrowser); + + // Check that the process count did not change. + is(ppmm.childCount, originalChildCount, "Preloaded browser should not create a new content process.") + + // Let's do another round. + BrowserOpenTab(); + let tab2 = gBrowser.selectedTab; + await ensurePreloaded(gBrowser); + + // Check that the process count did not change. + is(ppmm.childCount, originalChildCount, "Preloaded browser should (still) not create a new content process.") + + // Navigate to a content page from the parent side. + tab2.linkedBrowser.loadURI(TEST_URL); + await BrowserTestUtils.browserLoaded(tab2.linkedBrowser, false, TEST_URL); + is(ppmm.childCount, originalChildCount + 1, + "Navigating away from the preloaded browser (parent side) should create a new content process.") + + // Navigate to a content page from the child side. + await BrowserTestUtils.switchTab(gBrowser, tab1); + await ContentTask.spawn(tab1.linkedBrowser, null, async function() { + const TEST_URL = "http://www.example.com/browser/dom/base/test/dummy.html"; + content.location.href = TEST_URL; + }); + await BrowserTestUtils.browserLoaded(tab1.linkedBrowser, false, TEST_URL); + is(ppmm.childCount, originalChildCount + 2, + "Navigating away from the preloaded browser (child side) should create a new content process.") + + await BrowserTestUtils.removeTab(tab1); + await BrowserTestUtils.removeTab(tab2); + + // Since we kept alive all the processes, we can shut down the ones that do + // not host any tabs reliably. + ppmm.releaseCachedProcesses(); + is(ppmm.childCount, originalChildCount, "We're back to the original process count."); +}); diff --git a/dom/base/test/dummy.html b/dom/base/test/dummy.html new file mode 100644 index 000000000000..1a87e28408d0 --- /dev/null +++ b/dom/base/test/dummy.html @@ -0,0 +1,9 @@ + + +Dummy test page + + + +

Dummy test page

+ + diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 4a323f14fe73..02b28275992b 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -762,11 +762,11 @@ ContentParent::MinTabSelect(const nsTArray& aContentParents, /*static*/ already_AddRefed ContentParent::GetNewOrUsedBrowserProcess(const nsAString& aRemoteType, ProcessPriority aPriority, - ContentParent* aOpener) + ContentParent* aOpener, + bool aPreferUsed) { nsTArray& contentParents = GetOrCreatePool(aRemoteType); uint32_t maxContentParents = GetMaxProcessCount(aRemoteType); - if (aRemoteType.EqualsLiteral(LARGE_ALLOCATION_REMOTE_TYPE)) { // We never want to re-use Large-Allocation processes. if (contentParents.Length() >= maxContentParents) { @@ -775,11 +775,18 @@ ContentParent::GetNewOrUsedBrowserProcess(const nsAString& aRemoteType, aOpener); } } else { - nsTArray infos(contentParents.Length()); + uint32_t numberOfParents = contentParents.Length(); + nsTArray infos(numberOfParents); for (auto* cp : contentParents) { infos.AppendElement(cp->mScriptableHelper); } + if (aPreferUsed && numberOfParents) { + // For the preloaded browser we don't want to create a new process but reuse an + // existing one. + maxContentParents = numberOfParents; + } + nsCOMPtr cpp = do_GetService("@mozilla.org/ipc/processselector;1"); nsIContentProcessInfo* openerInfo = aOpener ? aOpener->mScriptableHelper.get() : nullptr; @@ -1131,6 +1138,13 @@ ContentParent::CreateBrowser(const TabContext& aContext, remoteType.AssignLiteral(DEFAULT_REMOTE_TYPE); } + bool isPreloadBrowser = false; + nsAutoString isPreloadBrowserStr; + if (aFrameElement->GetAttr(kNameSpaceID_None, nsGkAtoms::isPreloadBrowser, + isPreloadBrowserStr)) { + isPreloadBrowser = isPreloadBrowserStr.EqualsLiteral("true"); + } + RefPtr constructorSender; if (isInContentProcess) { MOZ_ASSERT(aContext.IsMozBrowserElement() || aContext.IsJSPlugin()); @@ -1146,7 +1160,8 @@ ContentParent::CreateBrowser(const TabContext& aContext, initialPriority); } else { constructorSender = - GetNewOrUsedBrowserProcess(remoteType, initialPriority, nullptr); + GetNewOrUsedBrowserProcess(remoteType, initialPriority, + nullptr, isPreloadBrowser); } if (!constructorSender) { return nullptr; diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 8d2ed9b34343..7caf469cfc79 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -172,7 +172,8 @@ public: GetNewOrUsedBrowserProcess(const nsAString& aRemoteType = NS_LITERAL_STRING(NO_REMOTE_TYPE), hal::ProcessPriority aPriority = hal::ProcessPriority::PROCESS_PRIORITY_FOREGROUND, - ContentParent* aOpener = nullptr); + ContentParent* aOpener = nullptr, + bool aPreferUsed = false); /** * Get or create a content process for a JS plugin. aPluginID is the id of the JS plugin diff --git a/dom/security/test/mixedcontentblocker/file_frameNavigation_blankTarget.html b/dom/security/test/mixedcontentblocker/file_frameNavigation_blankTarget.html index 6b4434ed317c..9d234b42ffcf 100644 --- a/dom/security/test/mixedcontentblocker/file_frameNavigation_blankTarget.html +++ b/dom/security/test/mixedcontentblocker/file_frameNavigation_blankTarget.html @@ -15,17 +15,15 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=841850 var blankTarget = document.getElementById("blankTarget"); blankTarget.click(); - var os = SpecialPowers.Cc["@mozilla.org/observer-service;1"]. - getService(SpecialPowers.Components.interfaces.nsIObserverService); var observer = { observe: function(subject, topic, data) { if(topic == "content-document-global-created" && data =="http://example.com") { parent.parent.postMessage({"test": "blankTarget", "msg": "opened an http link with target=_blank from a secure page"}, "http://mochi.test:8888"); - os.removeObserver(observer, "content-document-global-created"); + SpecialPowers.removeAsyncObserver(observer, "content-document-global-created"); } } } - os.addObserver(observer, "content-document-global-created"); + SpecialPowers.addAsyncObserver(observer, "content-document-global-created");