Bug 1472212 - Ensure that tab does not show busy or burst status whenever we navigate to about:home, about:newtab, or about:welcome in a new window. draft
authorJay Lim <jlim@mozilla.com>
Fri, 20 Jul 2018 16:43:12 -0400
changeset 827729 de039a5a0740
parent 827728 e813860ce681
child 827730 93f3d65ae2ae
push id118580
push userbmo:jay@imjching.com
push dateThu, 09 Aug 2018 03:02:02 +0000
bugs1472212
milestone63.0a1
Bug 1472212 - Ensure that tab does not show busy or burst status whenever we navigate to about:home, about:newtab, or about:welcome in a new window. Now that we have moved some about: pages to the privileged content process, opening these URLs from a non-privileged content process will trigger SessionStore to restore the tab state due to a process flip. We will set favicons for these URLs earlier to avoid flickering and improve perceived performance. This patch also prevents the spinner whenever a page with a local about: URI (about:blank and about: pages that resolve to jar:// or file:// URIs) is loaded from a process that the URI cannot load in (e.g. loading about:newtab in the web content process), as well as during tab duplication or session restoration for such local about: URIs. Before this patch, there were additional frames when opening a new window, causing browser/base/content/test/performance/browser_windowopen.js to fail. This patch will reduce the number of frames when opening a new window. MozReview-Commit-ID: yjj2964KSz
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser_isLocalAboutURI.js
browser/components/sessionstore/SessionStore.jsm
browser/modules/AsyncTabSwitcher.jsm
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -746,17 +746,17 @@ window._gBrowser = {
     }
 
     return rv;
   },
 
   /**
    * Determine if a URI is an about: page pointing to a local resource.
    */
-  _isLocalAboutURI(aURI, aResolvedURI) {
+  isLocalAboutURI(aURI, aResolvedURI) {
     if (!aURI.schemeIs("about")) {
       return false;
     }
 
     // Specially handle about:blank as local
     if (aURI.pathQueryRef === "blank") {
       return true;
     }
@@ -773,16 +773,25 @@ window._gBrowser = {
       ).URI;
       return resolvedURI.schemeIs("jar") || resolvedURI.schemeIs("file");
     } catch (ex) {
       // aURI might be invalid.
       return false;
     }
   },
 
+  /**
+   * Sets an icon for the tab if the URI is defined in FAVICON_DEFAULTS.
+   */
+  setDefaultIcon(aTab, aURI) {
+    if (aURI && aURI.spec in FAVICON_DEFAULTS) {
+      this.setIcon(aTab, FAVICON_DEFAULTS[aURI.spec]);
+    }
+  },
+
   setIcon(aTab, aIconURL = "", aOriginalURL = aIconURL) {
     let makeString = (url) => url instanceof Ci.nsIURI ? url.spec : url;
 
     aIconURL = makeString(aIconURL);
     aOriginalURL = makeString(aOriginalURL);
 
     let LOCAL_PROTOCOLS = [
       "chrome:",
@@ -2412,19 +2421,17 @@ window._gBrowser = {
         let notificationbox = this.getNotificationBox(t.linkedBrowser);
         notificationbox.remove();
       }
       throw e;
     }
 
     // Hack to ensure that the about:newtab, and about:welcome favicon is loaded
     // instantaneously, to avoid flickering and improve perceived performance.
-    if (aURI in FAVICON_DEFAULTS) {
-      this.setIcon(t, FAVICON_DEFAULTS[aURI]);
-    }
+    this.setDefaultIcon(t, aURIObject);
 
     // Dispatch a new tab notification.  We do this once we're
     // entirely done, so that things are in a consistent state
     // even if the event listener opens or closes tabs.
     let evt = new CustomEvent("TabOpen", { bubbles: true, detail: eventDetail || {} });
     t.dispatchEvent(evt);
     Services.telemetry.recordEvent("savant", "tab", "open", null, { subcategory: "frame" });
 
@@ -4583,17 +4590,17 @@ class TabProgressListener {
 
   _shouldShowProgress(aRequest) {
     if (this.mBlank)
       return false;
 
     // Don't show progress indicators in tabs for about: URIs
     // pointing to local resources.
     if ((aRequest instanceof Ci.nsIChannel) &&
-        gBrowser._isLocalAboutURI(aRequest.originalURI, aRequest.URI)) {
+        gBrowser.isLocalAboutURI(aRequest.originalURI, aRequest.URI)) {
       return false;
     }
 
     return true;
   }
 
   _isForInitialAboutBlank(aWebProgress, aStateFlags, aLocation) {
     if (!this.mBlank || !aWebProgress.isTopLevel) {
--- a/browser/base/content/test/tabs/browser_isLocalAboutURI.js
+++ b/browser/base/content/test/tabs/browser_isLocalAboutURI.js
@@ -1,41 +1,41 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 /**
- * Unit tests for tabbrowser._isLocalAboutURI to make sure it returns the
+ * Unit tests for tabbrowser.isLocalAboutURI to make sure it returns the
  * appropriate values for various URIs as well as optional resolved URI.
  */
 
 add_task(function test_URI() {
   const check = (spec, expect, description) => {
     const URI = Services.io.newURI(spec);
     try {
-      is(gBrowser._isLocalAboutURI(URI), expect, description);
+      is(gBrowser.isLocalAboutURI(URI), expect, description);
     } catch (ex) {
-      ok(false, "_isLocalAboutURI should not throw");
+      ok(false, "isLocalAboutURI should not throw");
     }
   };
   check("https://www.mozilla.org/", false, "https is not about");
   check("http://www.mozilla.org/", false, "http is not about");
   check("about:blank", true, "about:blank is local");
   check("about:about", true, "about:about is local");
   check("about:newtab", true, "about:newtab is local");
   check("about:random-invalid-uri", false,
         "about:random-invalid-uri is invalid but should not throw");
 });
 
 add_task(function test_URI_with_resolved() {
   const check = (spec, resolvedSpec, expect, description) => {
     const URI = Services.io.newURI(spec);
     const resolvedURI = Services.io.newURI(resolvedSpec);
-    is(gBrowser._isLocalAboutURI(URI, resolvedURI), expect, description);
+    is(gBrowser.isLocalAboutURI(URI, resolvedURI), expect, description);
   };
   check("about:newtab",
     "jar:file:///Applications/Firefox.app/Contents/Resources/browser/omni.ja!/chrome/browser/res/activity-stream/prerendered/en-US/activity-stream.html",
     true,
     "about:newtab with jar is local");
   check("about:newtab",
     "file:///mozilla-central/browser/base/content/newtab/newTab.xhtml",
     true,
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -2431,18 +2431,27 @@ var SessionStoreInternal = {
 
     // Create a new tab.
     let userContextId = aTab.getAttribute("usercontextid");
     let newTab = aTab == aWindow.gBrowser.selectedTab ?
       aWindow.gBrowser.addTab(null, {relatedToCurrent: true, ownerTab: aTab, userContextId}) :
       aWindow.gBrowser.addTab(null, {userContextId});
 
     // Start the throbber to pretend we're doing something while actually
-    // waiting for data from the frame script.
-    newTab.setAttribute("busy", "true");
+    // waiting for data from the frame script. This throbber is disabled
+    // if the URI is a local about: URI.
+    let uriObj = aTab.linkedBrowser.currentURI;
+    if (!uriObj || (uriObj && !aWindow.gBrowser.isLocalAboutURI(uriObj))) {
+      newTab.setAttribute("busy", "true");
+    }
+
+    // Hack to ensure that the about:home, about:newtab, and about:welcome
+    // favicon is loaded instantaneously, to avoid flickering and improve
+    // perceived performance.
+    aWindow.gBrowser.setDefaultIcon(newTab, uriObj);
 
     // Collect state before flushing.
     let tabState = TabState.collect(aTab, TAB_CUSTOM_VALUES.get(aTab));
 
     // Flush to get the latest tab state to duplicate.
     let browser = aTab.linkedBrowser;
     TabStateFlusher.flush(browser).then(() => {
       // The new tab might have been closed in the meantime.
@@ -2771,19 +2780,26 @@ var SessionStoreInternal = {
         win.gBrowser.setInitialTabTitle(tab, activePageData.title, { isContentTitle: true });
       } else if (activePageData.url != "about:blank") {
         win.gBrowser.setInitialTabTitle(tab, activePageData.url);
       }
     }
 
     // Restore the tab icon.
     if ("image" in tabData) {
-      // Use the serialized contentPrincipal with the new icon load.
-      let loadingPrincipal = Utils.deserializePrincipal(tabData.iconLoadingPrincipal);
-      win.gBrowser.setIcon(tab, tabData.image, loadingPrincipal);
+      // We know that about:blank is safe to load in any remote type. Since
+      // SessionStore is triggered with about:blank, there must be a process
+      // flip. We will ignore the first about:blank load to prevent resetting the
+      // favicon that we have set earlier to avoid flickering and improve
+      // perceived performance.
+      if (!activePageData || (activePageData && activePageData.url != "about:blank")) {
+        // Use the serialized contentPrincipal with the new icon load.
+        let loadingPrincipal = Utils.deserializePrincipal(tabData.iconLoadingPrincipal);
+        win.gBrowser.setIcon(tab, tabData.image, loadingPrincipal);
+      }
       TabStateCache.update(browser, { image: null, iconLoadingPrincipal: null });
     }
   },
 
   // This method deletes all the closedTabs matching userContextId.
   _forgetTabsWithUserContextId(userContextId) {
     let windowsEnum = Services.wm.getEnumerator("navigator:browser");
     while (windowsEnum.hasMoreElements()) {
@@ -3013,19 +3029,32 @@ var SessionStoreInternal = {
     this._remotenessChangingBrowsers.set(browser.permanentKey, loadArguments);
 
     if (alreadyRestoring) {
       // This tab was already being restored to run in the
       // correct process. We're done here.
       return;
     }
 
+    let uriObj;
+    try {
+      uriObj = Services.io.newURI(loadArguments.uri);
+    } catch (e) {}
+
     // Start the throbber to pretend we're doing something while actually
-    // waiting for data from the frame script.
-    tab.setAttribute("busy", "true");
+    // waiting for data from the frame script. This throbber is disabled
+    // if the URI is a local about: URI.
+    if (!uriObj || (uriObj && !window.gBrowser.isLocalAboutURI(uriObj))) {
+      tab.setAttribute("busy", "true");
+    }
+
+    // Hack to ensure that the about:home, about:newtab, and about:welcome
+    // favicon is loaded instantaneously, to avoid flickering and improve
+    // perceived performance.
+    window.gBrowser.setDefaultIcon(tab, uriObj);
 
     // Flush to get the latest tab state.
     TabStateFlusher.flush(browser).then(() => {
       // loadArguments might have been overwritten by multiple calls
       // to navigateAndRestore while we waited for the tab to flush,
       // so we use the most recently stored one.
       let recentLoadArguments =
         this._remotenessChangingBrowsers.get(browser.permanentKey);
--- a/browser/modules/AsyncTabSwitcher.jsm
+++ b/browser/modules/AsyncTabSwitcher.jsm
@@ -342,17 +342,17 @@ class AsyncTabSwitcher {
       //    tab crashed page yet (in this case, the TabParent is null)
       // 2. The tab has never presented, and has not finished loading
       //    a non-local-about: page.
       //
       // For (2), "finished loading a non-local-about: page" is
       // determined by the busy state on the tab element and checking
       // if the loaded URI is local.
       let hasSufficientlyLoaded = !this.requestedTab.hasAttribute("busy") &&
-        !this.tabbrowser._isLocalAboutURI(requestedBrowser.currentURI);
+        !this.tabbrowser.isLocalAboutURI(requestedBrowser.currentURI);
 
       let fl = requestedBrowser.frameLoader;
       shouldBeBlank = !this.minimizedOrFullyOccluded &&
         (!fl.tabParent ||
           (!hasSufficientlyLoaded && !fl.tabParent.hasPresented));
     }
 
     this.log("Tab should be blank: " + shouldBeBlank);