Bug 1420285 - Change <browser> attribute isPreloadBrowser to preloadedState draft
authorUrsula Sarracini <usarracini@mozilla.com>
Thu, 04 Jan 2018 15:54:37 -0500
changeset 715822 71a865006ca99ce0e891283bceb68a9c7f3b8b47
parent 715271 ac93fdadf1022211eec62258ad22b42cb37a6d14
child 744904 a86ee5acd584adcdaff3fcf65345baff62395b4b
push id94280
push userusarracini@mozilla.com
push dateThu, 04 Jan 2018 20:58:59 +0000
bugs1420285
milestone59.0a1
Bug 1420285 - Change <browser> attribute isPreloadBrowser to preloadedState MozReview-Commit-ID: 3ooQldAnPZl
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/extensions/activity-stream/lib/TelemetryFeed.jsm
dom/base/nsGkAtomList.h
dom/base/test/browser_aboutnewtab_process_selection.js
dom/ipc/ContentParent.cpp
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1033,22 +1033,22 @@ function _loadURIWithFlags(browser, uri,
     // 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") {
+  if (browser.getAttribute("preloadedState") === "consumed" && 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");
+    browser.removeAttribute("preloadedState");
   }
 
   // !requiredRemoteType means we're loading in the parent/this process.
   if (!requiredRemoteType) {
     browser.inLoadURI = true;
   }
   try {
     if (!mustChangeProcess) {
@@ -1120,18 +1120,18 @@ 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");
+  if (browser.getAttribute("preloadedState") === "consumed") {
+    browser.removeAttribute("preloadedState");
     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;
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2131,18 +2131,21 @@
             this._preloadedBrowser = null;
 
             // Attach the nsIFormFillController now that we know the browser
             // will be used. If we do that before and the preloaded browser
             // won't be consumed until shutdown then we leak a docShell.
             // Also, we do not need to take care of attaching nsIFormFillControllers
             // in the case that the browser is remote, as remote browsers take
             // care of that themselves.
-            if (browser && this.hasAttribute("autocompletepopup")) {
-              browser.setAttribute("autocompletepopup", this.getAttribute("autocompletepopup"));
+            if (browser) {
+              browser.setAttribute("preloadedState", "consumed");
+              if (this.hasAttribute("autocompletepopup")) {
+                browser.setAttribute("autocompletepopup", this.getAttribute("autocompletepopup"));
+              }
             }
 
             return browser;
           ]]>
         </body>
       </method>
 
       <method name="_isPreloadingEnabled">
@@ -2234,18 +2237,33 @@
               }
               b.presetOpenerWindow(aParams.openerWindow);
             }
 
             if (!aParams.isPreloadBrowser && this.hasAttribute("autocompletepopup")) {
               b.setAttribute("autocompletepopup", this.getAttribute("autocompletepopup"));
             }
 
+            /*
+             * This attribute is meant to describe if the browser is the
+             * preloaded browser. There are 2 defined states: "preloaded" or
+             * "consumed". The order of events goes as follows:
+             *   1. The preloaded browser is created and the 'preloadedState'
+             *      attribute for that browser is set to "preloaded".
+             *   2. When a new tab is opened and it is time to show that
+             *      preloaded browser, the 'preloadedState' attribute for that
+             *      browser is set to "consumed"
+             *   3. When we then navigate away from about:newtab, the "consumed"
+             *      browsers will attempt to switch to a new content process,
+             *      therefore the 'preloadedState' attribute is removed from
+             *      that browser altogether
+             * See more details on Bug 1420285.
+             */
             if (aParams.isPreloadBrowser) {
-              b.setAttribute("isPreloadBrowser", "true");
+              b.setAttribute("preloadedState", "preloaded");
             }
 
             if (this.hasAttribute("selectmenulist"))
               b.setAttribute("selectmenulist", this.getAttribute("selectmenulist"));
 
             if (this.hasAttribute("datetimepicker")) {
               b.setAttribute("datetimepicker", this.getAttribute("datetimepicker"));
             }
--- a/browser/extensions/activity-stream/lib/TelemetryFeed.jsm
+++ b/browser/extensions/activity-stream/lib/TelemetryFeed.jsm
@@ -241,17 +241,17 @@ this.TelemetryFeed = class TelemetryFeed
   /**
    * handleNewTabInit - Handle NEW_TAB_INIT, which creates a new session and sets the a flag
    *                    for session.perf based on whether or not this new tab is preloaded
    *
    * @param  {obj} action the Action object
    */
   handleNewTabInit(action) {
     const session = this.addSession(au.getPortIdOfSender(action), action.data.url);
-    session.perf.is_preloaded = action.data.browser.getAttribute("isPreloadBrowser") === "true";
+    session.perf.is_preloaded = action.data.browser.getAttribute("preloadedState") === "preloaded";
   }
 
   /**
    * createPing - Create a ping with common properties
    *
    * @param  {string} id The portID of the session, if a session is relevant (optional)
    * @return {obj}    A telemetry ping
    */
--- a/dom/base/nsGkAtomList.h
+++ b/dom/base/nsGkAtomList.h
@@ -2250,17 +2250,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")
+GK_ATOM(preloadedState, "preloadedState")
 
 // Names for system metrics
 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")
 GK_ATOM(overlay_scrollbars, "overlay-scrollbars")
--- a/dom/base/test/browser_aboutnewtab_process_selection.js
+++ b/dom/base/test/browser_aboutnewtab_process_selection.js
@@ -1,9 +1,11 @@
 const TEST_URL = "http://www.example.com/browser/dom/base/test/dummy.html";
+const PRELOADED_STATE = "preloaded";
+const CONSUMED_STATE = "consumed";
 
 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],
@@ -72,8 +74,35 @@ add_task(async function(){
 
   // Make sure the preload browser does not keep any of the new processes alive.
   gBrowser.removePreloadedBrowser();
 
   // Since we kept alive all the processes, we can shut down the ones that do
   // not host any tabs reliably.
   ppmm.releaseCachedProcesses();
 });
+
+add_task(async function preloaded_state_attribute() {
+  // Wait for a preloaded browser to exist, use it, and then create another one
+  await ensurePreloaded(gBrowser);
+  let preloadedTabState = gBrowser._preloadedBrowser.getAttribute("preloadedState");
+  is(preloadedTabState, PRELOADED_STATE, "Sanity check that the first preloaded browser has the correct attribute");
+
+  BrowserOpenTab();
+  await ensurePreloaded(gBrowser);
+
+  // Now check that the tabs have the correct browser attributes set
+  let consumedTabState = gBrowser.selectedBrowser.getAttribute("preloadedState");
+  is(consumedTabState, CONSUMED_STATE, "The opened tab consumed the preloaded browser and updated the attribute");
+
+  preloadedTabState = gBrowser._preloadedBrowser.getAttribute("preloadedState");
+  is(preloadedTabState, PRELOADED_STATE, "The preloaded browser has the correct attribute");
+
+  // Navigate away and check that the attribute has been removed altogether
+  gBrowser.selectedBrowser.loadURI(TEST_URL);
+  let navigatedTabHasState = gBrowser.selectedBrowser.hasAttribute("preloadedState");
+  ok(!navigatedTabHasState, "Correctly removed the preloadState attribute when navigating away");
+
+  // Remove tabs and preloaded browsers
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+  gBrowser.removePreloadedBrowser();
+});
+
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -1131,19 +1131,19 @@ ContentParent::CreateBrowser(const TabCo
   nsIDocShell* docShell = GetOpenerDocShellHelper(aFrameElement);
   TabId openerTabId;
   if (docShell) {
     openerTabId = TabParent::GetTabIdFrom(docShell);
   }
 
   bool isPreloadBrowser = false;
   nsAutoString isPreloadBrowserStr;
-  if (aFrameElement->GetAttr(kNameSpaceID_None, nsGkAtoms::isPreloadBrowser,
+  if (aFrameElement->GetAttr(kNameSpaceID_None, nsGkAtoms::preloadedState,
                              isPreloadBrowserStr)) {
-    isPreloadBrowser = isPreloadBrowserStr.EqualsLiteral("true");
+    isPreloadBrowser = isPreloadBrowserStr.EqualsLiteral("preloaded");
   }
 
   RefPtr<nsIContentParent> constructorSender;
   if (isInContentProcess) {
     MOZ_ASSERT(aContext.IsMozBrowserElement() || aContext.IsJSPlugin());
     constructorSender = CreateContentBridgeParent(aContext, initialPriority,
                                                   openerTabId, tabId);
   } else {