Bug 1376895 - Make preloaded browser use pre-existing content process. r=mconley
authorGabor Krizsanits <gkrizsanits@mozilla.com>
Tue, 15 Aug 2017 14:05:17 +0200
changeset 374781 adf5ed713e0df1a523b8b7634dd7b943a1daa5b2
parent 374780 6949fdc7f97c9888c2e739c59a9150301a40b685
child 374782 92c5fef6d702c5690057b36a68e76354c164d8ca
push id93759
push usergkrizsanits@mozilla.com
push dateTue, 15 Aug 2017 12:06:12 +0000
treeherdermozilla-inbound@adf5ed713e0d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1376895
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
browser/base/content/tabbrowser.xml
browser/base/content/test/urlbar/browser_urlbarKeepStateAcrossTabSwitches.js
browser/modules/E10SUtils.jsm
dom/base/nsGkAtomList.h
dom/base/test/browser.ini
dom/base/test/browser_aboutnewtab_process_selection.js
dom/base/test/dummy.html
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/security/test/mixedcontentblocker/file_frameNavigation_blankTarget.html
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1083,16 +1083,24 @@ function _loadURIWithFlags(browser, uri,
   } catch (e) {
     // createFixupURI throws if it can't create a URI. If that's the case then
     // we still need to pass down the uri because docshell handles this case.
     requiredRemoteType = gMultiProcessBrowser ? E10SUtils.DEFAULT_REMOTE_TYPE
                                               : E10SUtils.NOT_REMOTE;
   }
 
   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) {
     browser.inLoadURI = true;
   }
   try {
     if (!mustChangeProcess) {
       if (params.userContextId) {
@@ -1117,17 +1125,18 @@ function _loadURIWithFlags(browser, uri,
         uri,
         triggeringPrincipal: triggeringPrincipal
           ? gSerializationHelper.serializeToString(triggeringPrincipal)
           : null,
         flags,
         referrer: referrer ? referrer.spec : null,
         referrerPolicy,
         remoteType: requiredRemoteType,
-        postData
+        postData,
+        newFrameloader,
       }
 
       if (params.userContextId) {
         loadParams.userContextId = params.userContextId;
       }
 
       LoadInOtherProcess(browser, loadParams);
     }
@@ -1162,16 +1171,21 @@ function _loadURIWithFlags(browser, uri,
 function LoadInOtherProcess(browser, loadOptions, historyIndex = -1) {
   let tab = gBrowser.getTabForBrowser(browser);
   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({ 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.
     data.loadOptions.remoteType = E10SUtils.LARGE_ALLOCATION_REMOTE_TYPE;
     data.loadOptions.newFrameloader = true;
   } else if (browser.remoteType == E10SUtils.LARGE_ALLOCATION_REMOTE_TYPE) {
     // If we're in a Large-Allocation process, we prefer switching back into a
     // normal content process, as that way we can clean up the L-A process.
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2113,16 +2113,20 @@
               }
               b.QueryInterface(Ci.nsIFrameLoaderOwner).presetOpenerWindow(aParams.opener);
             }
 
             if (!aParams.isPreloadBrowser && this.hasAttribute("autocompletepopup")) {
               b.setAttribute("autocompletepopup", this.getAttribute("autocompletepopup"));
             }
 
+            if (aParams.isPreloadBrowser) {
+              b.setAttribute("isPreloadBrowser", "true");
+            }
+
             if (this.hasAttribute("selectmenulist"))
               b.setAttribute("selectmenulist", this.getAttribute("selectmenulist"));
 
             if (this.hasAttribute("datetimepicker")) {
               b.setAttribute("datetimepicker", this.getAttribute("datetimepicker"));
             }
 
             b.setAttribute("autoscrollpopup", this._autoScrollPopup.id);
--- a/browser/base/content/test/urlbar/browser_urlbarKeepStateAcrossTabSwitches.js
+++ b/browser/base/content/test/urlbar/browser_urlbarKeepStateAcrossTabSwitches.js
@@ -1,17 +1,17 @@
 "use strict";
 
 /**
  * Verify user typed text remains in the URL bar when tab switching, even when
  * loads fail.
  */
 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)
   let errorPageLoaded = BrowserTestUtils.waitForErrorPage(tab.linkedBrowser);
   gURLBar.value = input;
   gURLBar.select();
   EventUtils.sendKey("return");
   await errorPageLoaded;
@@ -24,17 +24,17 @@ add_task(async function() {
 
 /**
  * Invalid URIs fail differently (that is, immediately, in the loadURI call)
  * if keyword searches are turned off. Test that this works, too.
  */
 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)
   let errorPageLoaded = BrowserTestUtils.waitForErrorPage(tab.linkedBrowser);
   gURLBar.value = input;
   gURLBar.select();
   EventUtils.sendKey("return");
   await errorPageLoaded;
--- a/browser/modules/E10SUtils.jsm
+++ b/browser/modules/E10SUtils.jsm
@@ -238,16 +238,24 @@ this.E10SUtils = {
 
       // If not originally loaded in this process allow it if the URI would
       // normally be allowed to load in this process by default.
       let remoteType = Services.appinfo.remoteType;
       return remoteType ==
         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);
   },
 
   redirectLoad(aDocShell, aURI, aReferrer, aTriggeringPrincipal, aFreshProcess, aFlags) {
     // Retarget the load to the correct process
     let messageManager = aDocShell.QueryInterface(Ci.nsIInterfaceRequestor)
                                   .getInterface(Ci.nsIContentFrameMessageManager);
--- a/dom/base/nsGkAtomList.h
+++ b/dom/base/nsGkAtomList.h
@@ -2226,16 +2226,17 @@ GK_ATOM(mozfixed, "-moz-fixed")
 GK_ATOM(Remote, "remote")
 GK_ATOM(RemoteId, "_remote_id")
 GK_ATOM(RemoteType, "remoteType")
 GK_ATOM(DisplayPort, "_displayport")
 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")
 GK_ATOM(scrollbar_start_backward, "scrollbar-start-backward")
 GK_ATOM(scrollbar_start_forward, "scrollbar-start-forward")
 GK_ATOM(scrollbar_end_backward, "scrollbar-end-backward")
 GK_ATOM(scrollbar_end_forward, "scrollbar-end-forward")
 GK_ATOM(scrollbar_thumb_proportional, "scrollbar-thumb-proportional")
--- a/dom/base/test/browser.ini
+++ b/dom/base/test/browser.ini
@@ -1,11 +1,12 @@
 [DEFAULT]
 support-files =
   audio.ogg
+  dummy.html
   empty.html
   file_audioLoop.html
   file_audioLoopInIframe.html
   file_blocking_image.html
   file_bug902350.html
   file_bug902350_frame.html
   file_bug1011748_redirect.sjs
   file_bug1011748_OK.sjs
@@ -30,16 +31,17 @@ support-files =
 [browser_bug593387.js]
 [browser_bug902350.js]
 tags = mcb
 [browser_bug1011748.js]
 [browser_bug1058164.js]
 [browser_force_process_selector.js]
 skip-if = !e10s # this only makes sense with e10s-multi
 [browser_messagemanager_loadprocessscript.js]
+[browser_aboutnewtab_process_selection.js]
 [browser_messagemanager_targetframeloader.js]
 [browser_messagemanager_unload.js]
 [browser_pagehide_on_tab_close.js]
 skip-if = e10s # this tests non-e10s behavior. it's not expected to work in e10s.
 [browser_state_notifications.js]
 skip-if = true # Bug 1271028
 [browser_use_counters.js]
 [browser_timeout_throttling_with_audio_playback.js]
new file mode 100644
--- /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.");
+});
new file mode 100644
--- /dev/null
+++ b/dom/base/test/dummy.html
@@ -0,0 +1,9 @@
+<html>
+<head>
+<title>Dummy test page</title>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8"></meta>
+</head>
+<body>
+<p>Dummy test page</p>
+</body>
+</html>
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -757,34 +757,41 @@ ContentParent::MinTabSelect(const nsTArr
   }
 
   return candidate.forget();
 }
 
 /*static*/ already_AddRefed<ContentParent>
 ContentParent::GetNewOrUsedBrowserProcess(const nsAString& aRemoteType,
                                           ProcessPriority aPriority,
-                                          ContentParent* aOpener)
+                                          ContentParent* aOpener,
+                                          bool aPreferUsed)
 {
   nsTArray<ContentParent*>& 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) {
       return GetNewOrUsedBrowserProcess(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE),
                                         aPriority,
                                         aOpener);
     }
   } else {
-    nsTArray<nsIContentProcessInfo*> infos(contentParents.Length());
+    uint32_t numberOfParents = contentParents.Length();
+    nsTArray<nsIContentProcessInfo*> 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<nsIContentProcessProvider> cpp =
       do_GetService("@mozilla.org/ipc/processselector;1");
     nsIContentProcessInfo* openerInfo = aOpener ? aOpener->mScriptableHelper.get() : nullptr;
     int32_t index;
     if (cpp &&
         NS_SUCCEEDED(cpp->ProvideProcess(aRemoteType, openerInfo,
                                          infos.Elements(), infos.Length(),
                                          maxContentParents, &index))) {
@@ -1126,32 +1133,40 @@ ContentParent::CreateBrowser(const TabCo
   }
 
   nsAutoString remoteType;
   if (!aFrameElement->GetAttr(kNameSpaceID_None, nsGkAtoms::RemoteType,
                               remoteType)) {
     remoteType.AssignLiteral(DEFAULT_REMOTE_TYPE);
   }
 
+  bool isPreloadBrowser = false;
+  nsAutoString isPreloadBrowserStr;
+  if (aFrameElement->GetAttr(kNameSpaceID_None, nsGkAtoms::isPreloadBrowser,
+                             isPreloadBrowserStr)) {
+    isPreloadBrowser = isPreloadBrowserStr.EqualsLiteral("true");
+  }
+
   RefPtr<nsIContentParent> constructorSender;
   if (isInContentProcess) {
     MOZ_ASSERT(aContext.IsMozBrowserElement() || aContext.IsJSPlugin());
     constructorSender = CreateContentBridgeParent(aContext, initialPriority,
                                                   openerTabId, tabId);
   } else {
     if (aOpenerContentParent) {
       constructorSender = aOpenerContentParent;
     } else {
       if (aContext.IsJSPlugin()) {
         constructorSender =
           GetNewOrUsedJSPluginProcess(aContext.JSPluginId(),
                                       initialPriority);
       } else {
         constructorSender =
-          GetNewOrUsedBrowserProcess(remoteType, initialPriority, nullptr);
+          GetNewOrUsedBrowserProcess(remoteType, initialPriority,
+                                     nullptr, isPreloadBrowser);
       }
       if (!constructorSender) {
         return nullptr;
       }
     }
     ContentProcessManager* cpm = ContentProcessManager::GetSingleton();
     cpm->RegisterRemoteFrame(tabId,
                              ContentParentId(0),
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -167,17 +167,18 @@ public:
    * 1. browser iframe
    * 2. remote xul <browser>
    * 3. normal iframe
    */
   static already_AddRefed<ContentParent>
   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
    * (@see nsFakePlugin::mId). There is a maximum of one process per JS plugin.
    */
   static already_AddRefed<ContentParent>
   GetNewOrUsedJSPluginProcess(uint32_t aPluginID,
                               const hal::ProcessPriority& aPriority);
--- a/dom/security/test/mixedcontentblocker/file_frameNavigation_blankTarget.html
+++ b/dom/security/test/mixedcontentblocker/file_frameNavigation_blankTarget.html
@@ -10,23 +10,21 @@ https://bugzilla.mozilla.org/show_bug.cg
 </head>
 <body>
 <a href="http://example.com/tests/dom/security/test/mixedcontentblocker/file_frameNavigation_innermost.html?blankTarget" id="blankTarget" target="_blank">Go to http site</a>
 
 <script>
   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");
 
 </script>
 </body>
 </html>