Bug 1364127 - Set the initial tab label to the URL for new tabs and to the saved title for restored tabs, and make sure that label doesn't subsequently get clobbered with a placeholder r=past
authorDão Gottwald <dao@mozilla.com>
Fri, 12 May 2017 13:35:44 +0200
changeset 358256 9f3f63f8acb7f71ecda9d6d44cdad3af54fb9944
parent 358255 5872d9137b5189b020a736dd5c8fb6c9aaec2d66
child 358257 4ca6d53664ac9e63830d35d8edf87af8a72ab35b
push id90303
push userarchaeopteryx@coole-files.de
push dateSun, 14 May 2017 16:08:51 +0000
treeherdermozilla-inbound@626efff0df63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast
bugs1364127
milestone55.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 1364127 - Set the initial tab label to the URL for new tabs and to the saved title for restored tabs, and make sure that label doesn't subsequently get clobbered with a placeholder r=past MozReview-Commit-ID: 9t07uAVE13H
browser/base/content/tabbrowser.xml
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/test/browser.ini
browser/components/sessionstore/test/browser_tab_label_during_restore.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1404,23 +1404,44 @@
 
 
       <!-- TODO: remove after 57, once we know add-ons can no longer use it. -->
       <method name="setTabTitleLoading">
         <parameter name="aTab"/>
         <body/>
       </method>
 
+      <method name="setInitialTabTitle">
+        <parameter name="aTab"/>
+        <parameter name="aTitle"/>
+        <body><![CDATA[
+          if (aTitle) {
+            aTab.setAttribute("label", aTitle);
+
+            // Don't replace the set label with the empty tab label or the URL
+            // while the tab is loading.
+            aTab._suppressTransientPlaceholderLabel = true;
+          }
+        ]]></body>
+      </method>
+
       <method name="setTabTitle">
         <parameter name="aTab"/>
         <body>
           <![CDATA[
             var browser = this.getBrowserForTab(aTab);
             var title = browser.contentTitle;
 
+            if (aTab._suppressTransientPlaceholderLabel) {
+              if (!title) {
+                return false;
+              }
+              delete aTab._suppressTransientPlaceholderLabel;
+            }
+
             if (!title) {
               if (browser.currentURI.spec) {
                 try {
                   title = this.mURIFixup.createExposableURI(browser.currentURI).spec;
                 } catch (ex) {
                   title = browser.currentURI.spec;
                 }
               }
@@ -1460,32 +1481,32 @@
 
       <method name="loadOneTab">
         <parameter name="aURI"/>
         <parameter name="aReferrerURI"/>
         <parameter name="aCharset"/>
         <parameter name="aPostData"/>
         <parameter name="aLoadInBackground"/>
         <parameter name="aAllowThirdPartyFixup"/>
-        <parameter name="aIsPrerendered"/>
         <body>
           <![CDATA[
             var aTriggeringPrincipal;
             var aReferrerPolicy;
             var aFromExternal;
             var aRelatedToCurrent;
             var aAllowMixedContent;
             var aSkipAnimation;
             var aForceNotRemote;
             var aPreferredRemoteType;
             var aNoReferrer;
             var aUserContextId;
             var aSameProcessAsFrameLoader;
             var aOriginPrincipal;
             var aOpener;
+            var aIsPrerendered;
             var aCreateLazyBrowser;
             var aNextTabParentId;
             if (arguments.length == 2 &&
                 typeof arguments[1] == "object" &&
                 !(arguments[1] instanceof Ci.nsIURI)) {
               let params = arguments[1];
               aTriggeringPrincipal      = params.triggeringPrincipal
               aReferrerURI              = params.referrerURI;
@@ -2214,17 +2235,16 @@
 
       <method name="addTab">
         <parameter name="aURI"/>
         <parameter name="aReferrerURI"/>
         <parameter name="aCharset"/>
         <parameter name="aPostData"/>
         <parameter name="aOwner"/>
         <parameter name="aAllowThirdPartyFixup"/>
-        <parameter name="aIsPrerendered"/>
         <body>
           <![CDATA[
             "use strict";
 
             const NS_XUL = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
             var aTriggeringPrincipal;
             var aReferrerPolicy;
             var aFromExternal;
@@ -2235,19 +2255,21 @@
             var aPreferredRemoteType;
             var aNoReferrer;
             var aUserContextId;
             var aEventDetail;
             var aSameProcessAsFrameLoader;
             var aOriginPrincipal;
             var aDisallowInheritPrincipal;
             var aOpener;
+            var aIsPrerendered;
             var aCreateLazyBrowser;
             var aSkipBackgroundNotify;
             var aNextTabParentId;
+            var aNoInitialLabel;
             if (arguments.length == 2 &&
                 typeof arguments[1] == "object" &&
                 !(arguments[1] instanceof Ci.nsIURI)) {
               let params = arguments[1];
               aTriggeringPrincipal      = params.triggeringPrincipal;
               aReferrerURI              = params.referrerURI;
               aReferrerPolicy           = params.referrerPolicy;
               aCharset                  = params.charset;
@@ -2266,16 +2288,17 @@
               aSameProcessAsFrameLoader = params.sameProcessAsFrameLoader;
               aOriginPrincipal          = params.originPrincipal;
               aDisallowInheritPrincipal = params.disallowInheritPrincipal;
               aOpener                   = params.opener;
               aIsPrerendered            = params.isPrerendered;
               aCreateLazyBrowser        = params.createLazyBrowser;
               aSkipBackgroundNotify     = params.skipBackgroundNotify;
               aNextTabParentId          = params.nextTabParentId;
+              aNoInitialLabel           = params.noInitialLabel;
             }
 
             // if we're adding tabs, we're past interrupt mode, ditch the owner
             if (this.mCurrentTab.owner)
               this.mCurrentTab.owner = null;
 
             var t = document.createElementNS(NS_XUL, "tab");
 
@@ -2284,21 +2307,23 @@
             let lazyBrowserURI;
             if (aCreateLazyBrowser && aURI != "about:blank") {
               lazyBrowserURI = Services.io.newURI(aURI);
               aURI = "about:blank";
             }
 
             var uriIsAboutBlank = aURI == "about:blank";
 
-            if (isBlankPageURL(aURI)) {
-              t.setAttribute("label", this.mStringBundle.getString("tabs.emptyTabTitle"));
-            } else if (aURI.toLowerCase().startsWith("javascript:")) {
-              // This can go away when bug 672618 or bug 55696 are fixed.
-              t.setAttribute("label", aURI);
+            if (!aNoInitialLabel) {
+              if (isBlankPageURL(aURI)) {
+                t.setAttribute("label", this.mStringBundle.getString("tabs.emptyTabTitle"));
+              } else {
+                // Set URL as label so that the tab isn't empty initially.
+                this.setInitialTabTitle(t, aURI);
+              }
             }
 
             if (aIsPrerendered) {
               t.setAttribute("hidden", "true");
             }
 
             // Related tab inherits current tab's user context unless a different
             // usercontextid is specified
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -2639,19 +2639,19 @@ var SessionStoreInternal = {
       }
     }
 
     let activePageData = tabData.entries[tabData.index - 1] || null;
 
     // If the page has a title, set it.
     if (activePageData) {
       if (activePageData.title) {
-        tab.label = activePageData.title;
+        win.gBrowser.setInitialTabTitle(tab, activePageData.title);
       } else if (activePageData.url != "about:blank") {
-        tab.label = activePageData.url;
+        win.gBrowser.setInitialTabTitle(tab, activePageData.url);
       }
     } else if (tab.hasAttribute("customizemode")) {
       win.gCustomizeMode.setTab(tab);
     }
 
     // Restore the tab icon.
     if ("image" in tabData) {
       // Use the serialized contentPrincipal with the new icon load.
@@ -3286,19 +3286,23 @@ var SessionStoreInternal = {
       let url = "about:blank";
       if (createLazyBrowser) {
         // Let tabbrowser know the future URI because progress listeners won't
         // get onLocationChange notification before the browser is inserted.
         let activeIndex = (tabData.index || tabData.entries.length) - 1;
         url = tabData.entries[activeIndex].url;
       }
 
+      // Setting noInitialLabel is a perf optimization. Rendering tab labels
+      // would make resizing the tabs more expensive as we're adding them.
+      // Each tab will get its initial label set in restoreTab.
       let tab = tabbrowser.addTab(url,
                                   { createLazyBrowser,
                                     skipAnimation: true,
+                                    noInitialLabel: true,
                                     userContextId,
                                     skipBackgroundNotify: true });
 
       if (select) {
         // Select a new tab first to prevent the removeTab loop from changing
         // the selected tab over and over again.
         tabbrowser.selectedTab = tab;
 
--- a/browser/components/sessionstore/test/browser.ini
+++ b/browser/components/sessionstore/test/browser.ini
@@ -107,21 +107,23 @@ skip-if = e10s # Bug 1271024
 [browser_replace_load.js]
 [browser_restore_redirect.js]
 [browser_restore_cookies_noOriginAttributes.js]
 [browser_scrollPositions.js]
 [browser_scrollPositionsReaderMode.js]
 [browser_sessionHistory.js]
 [browser_sessionStorage.js]
 [browser_sessionStorage_size.js]
+[browser_tab_label_during_restore.js]
 [browser_swapDocShells.js]
 [browser_switch_remoteness.js]
 run-if = e10s
 [browser_upgrade_backup.js]
 [browser_windowRestore_perwindowpb.js]
+
 [browser_248970_b_perwindowpb.js]
 # Disabled because of leaks.
 # Re-enabling and rewriting this test is tracked in bug 936919.
 skip-if = true
 [browser_339445.js]
 [browser_345898.js]
 [browser_350525.js]
 [browser_354894_perwindowpb.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/sessionstore/test/browser_tab_label_during_restore.js
@@ -0,0 +1,85 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Test that we don't do unnecessary tab label changes while restoring a tab.
+ */
+
+add_task(async function() {
+  await SpecialPowers.pushPrefEnv({
+    "set": [
+      ["browser.sessionstore.restore_on_demand", true],
+      ["browser.sessionstore.restore_tabs_lazily", true],
+    ]
+  });
+  const BACKUP_STATE = SessionStore.getBrowserState();
+  const TEST_URL = "http://example.com/";
+
+  function observeLabelChanges(tab) {
+    info("observing tab label changes. initial label: " + tab.label);
+    let labelChangeCount = 0;
+    function TabAttrModifiedListener(event) {
+      if (event.detail.changed.some(attr => { return attr == "label" })) {
+        info("tab label change: " + tab.label);
+        labelChangeCount++;
+      }
+    }
+    tab.addEventListener("TabAttrModified", TabAttrModifiedListener);
+    return (expectedCount) => {
+      tab.removeEventListener("TabAttrModified", TabAttrModifiedListener);
+      is(labelChangeCount, expectedCount, "observed tab label changes");
+    }
+  }
+
+  info("setting test browser state");
+  let browserLoadedPromise = BrowserTestUtils.firstBrowserLoaded(window, false);
+  await promiseBrowserState({
+    windows: [{
+      tabs: [
+        { entries: [{ url: TEST_URL }] },
+        { entries: [{ url: TEST_URL }] },
+      ]
+    }]
+  });
+  let [firstTab, secondTab] = gBrowser.tabs;
+  is(gBrowser.selectedTab, firstTab, "first tab is selected");
+
+  await browserLoadedPromise;
+  const CONTENT_TITLE = firstTab.linkedBrowser.contentTitle;
+  is(firstTab.linkedBrowser.currentURI.spec, TEST_URL, "correct URL loaded in first tab");
+  is(typeof CONTENT_TITLE, "string", "content title is a string");
+  isnot(CONTENT_TITLE.length, 0, "content title isn't empty");
+  isnot(CONTENT_TITLE, TEST_URL, "content title is different from the URL");
+  is(firstTab.label, CONTENT_TITLE, "first tab displays content title");
+  ok(secondTab.hasAttribute("pending"), "second tab is pending");
+  is(secondTab.label, TEST_URL, "second tab displays URL as its title");
+
+  info("selecting the second tab");
+  let checkLabelChangeCount = observeLabelChanges(secondTab);
+  browserLoadedPromise = BrowserTestUtils.browserLoaded(secondTab.linkedBrowser, false, TEST_URL);
+  gBrowser.selectedTab = secondTab;
+  await browserLoadedPromise;
+  ok(!secondTab.hasAttribute("pending"), "second tab isn't pending anymore");
+  is(gBrowser.selectedTab.label, CONTENT_TITLE, "second tab displays content title");
+  checkLabelChangeCount(1);
+
+  info("restoring the modified browser state");
+  await TabStateFlusher.flushWindow(window);
+  await promiseBrowserState(SessionStore.getBrowserState());
+  [firstTab, secondTab] = gBrowser.tabs;
+  is(secondTab, gBrowser.selectedTab, "second tab is selected after restoring");
+  ok(firstTab.hasAttribute("pending"), "first tab is pending after restoring");
+  is(firstTab.label, CONTENT_TITLE, "first tab displays content title in pending state");
+
+  info("selecting the first tab");
+  checkLabelChangeCount = observeLabelChanges(firstTab);
+  let tabContentRestored = TestUtils.topicObserved("sessionstore-debug-tab-restored");
+  gBrowser.selectedTab = firstTab;
+  await tabContentRestored;
+  ok(!firstTab.hasAttribute("pending"), "first tab isn't pending anymore");
+  checkLabelChangeCount(0);
+  is(firstTab.label, CONTENT_TITLE, "first tab displays content title after restoring content");
+
+  await promiseBrowserState(BACKUP_STATE);
+});
+