Bug 1473514: Display an empty space for the tab icon while waiting for the real favicon to appear. r=dao

This adds a simple empty box that is displayed when we're still loading an icon
but are no longer showing the throbber. Ideally I'd like to keep showing the
throbber and maintain the busy state but that seems more risky for now.

Differential Revision: https://phabricator.services.mozilla.com/D2364

--HG--
extra : moz-landing-system : lando

browser/base/content/browser.css

@@ -185,6 +185,7 @@ panelview[mainview] > .panel-header {
   transition: visibility 0ms 25ms;
 }
 
+.tab-icon-pending:not([fadein]),
 .tab-icon-image:not([fadein]),
 .tab-close-button:not([fadein]),
 .tabbrowser-tab:not([fadein])::after,

browser/base/content/browser.js

@@ -3623,7 +3623,9 @@ const DOMEventHandler = {
   init() {
     let mm = window.messageManager;
     mm.addMessageListener("Link:AddFeed", this);
+    mm.addMessageListener("Link:LoadingIcon", this);
     mm.addMessageListener("Link:SetIcon", this);
+    mm.addMessageListener("Link:SetFailedIcon", this);
     mm.addMessageListener("Link:AddSearch", this);
     mm.addMessageListener("Meta:SetPageInfo", this);
   },
@@ -3635,11 +3637,23 @@ const DOMEventHandler = {
         FeedHandler.addFeed(link, aMsg.target);
         break;
 
+      case "Link:LoadingIcon":
+        if (aMsg.data.canUseForTab) {
+          this.setPendingIcon(aMsg.target);
+        }
+        break;
+
       case "Link:SetIcon":
         this.setIconFromLink(aMsg.target, aMsg.data.originalURL, aMsg.data.canUseForTab,
                              aMsg.data.expiration, aMsg.data.iconURL);
         break;
 
+      case "Link:SetFailedIcon":
+        if (aMsg.data.canUseForTab) {
+          this.clearPendingIcon(aMsg.target);
+        }
+        break;
+
       case "Link:AddSearch":
         this.addSearch(aMsg.target, aMsg.data.engine, aMsg.data.url);
         break;
@@ -3656,6 +3670,16 @@ const DOMEventHandler = {
     return true;
   },
 
+  setPendingIcon(aBrowser) {
+    let tab = gBrowser.getTabForBrowser(aBrowser);
+    tab.setAttribute("pendingicon", "true");
+  },
+
+  clearPendingIcon(aBrowser) {
+    let tab = gBrowser.getTabForBrowser(aBrowser);
+    tab.removeAttribute("pendingicon");
+  },
+
   setIconFromLink(aBrowser, aOriginalURL, aCanUseForTab, aExpiration, aIconURL) {
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
@@ -3669,6 +3693,7 @@ const DOMEventHandler = {
     }
 
     if (aCanUseForTab) {
+      this.clearPendingIcon(aBrowser);
       gBrowser.setIcon(tab, aIconURL, aOriginalURL);
     }
     return true;

browser/base/content/tabbrowser.css

@@ -8,6 +8,8 @@
 
 .tab-close-button[pinned],
 #tabbrowser-tabs[closebuttons="activetab"] > .tabbrowser-tab > .tab-stack > .tab-content > .tab-close-button:not([selected="true"]),
+.tab-icon-pending:not([pendingicon]),
+.tab-icon-pending[busy],
 .tab-icon-image:not([src]):not([pinned]):not([crashed])[selected],
 .tab-icon-image:not([src]):not([pinned]):not([crashed]):not([sharing]),
 .tab-icon-image[busy],

browser/base/content/tabbrowser.xml

@@ -1675,6 +1675,9 @@
                      class="tab-throbber-fallback"
                      role="presentation"
                      layer="true"/>
+          <xul:hbox xbl:inherits="fadein,pinned,busy,progress,selected=visuallyselected,pendingicon"
+                    anonid="tab-icon-pending"
+                    class="tab-icon-pending"/>
           <xul:image xbl:inherits="src=image,triggeringprincipal=iconloadingprincipal,requestcontextid,fadein,pinned,selected=visuallyselected,busy,crashed,sharing"
                      anonid="tab-icon-image"
                      class="tab-icon-image"

browser/base/content/test/favicons/browser.ini

@@ -55,3 +55,6 @@ support-files =
 support-files =
   file_insecure_favicon.html
   file_favicon.png
+[browser_title_flicker.js]
+support-files =
+  file_with_slow_favicon.html

browser/base/content/test/favicons/browser_title_flicker.js

@@ -0,0 +1,43 @@
+// Verify that the title doesn't flicker if the icon takes too long to load.
+// We expect to see events in the following order:
+// "label" added to tab
+// "busy" removed from tab
+// icon available
+// In all those cases the title should be in the same position.
+
+function waitForAttributeChange(tab, attr) {
+  info(`Waiting for attribute ${attr}`);
+  return new Promise(resolve => {
+    let listener = (event) => {
+      if (event.detail.changed.includes(attr)) {
+        tab.removeEventListener("TabAttrModified", listener);
+        resolve();
+      }
+    };
+
+    tab.addEventListener("TabAttrModified", listener);
+  });
+}
+
+add_task(async () => {
+  const testPath = "http://example.com/browser/browser/base/content/test/favicons/";
+
+  await BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" }, async (browser) => {
+    let tab = gBrowser.getTabForBrowser(browser);
+    BrowserTestUtils.loadURI(browser, testPath + "file_with_slow_favicon.html");
+
+    await waitForAttributeChange(tab, "label");
+    ok(tab.hasAttribute("busy"), "Should have seen the busy attribute");
+    let label = document.getAnonymousElementByAttribute(tab, "anonid", "tab-label");
+    let bounds = label.getBoundingClientRect();
+
+    await waitForAttributeChange(tab, "busy");
+    ok(!tab.hasAttribute("busy"), "Should have seen the busy attribute removed");
+    let newBounds = label.getBoundingClientRect();
+    is(bounds.x, newBounds.left, "Should have seen the title in the same place.");
+
+    await waitForFaviconMessage(true);
+    newBounds = label.getBoundingClientRect();
+    is(bounds.x, newBounds.left, "Should have seen the title in the same place.");
+  });
+});

browser/base/content/test/favicons/file_with_slow_favicon.html

@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for title flicker</title>
+</head>
+<body>
+  <!-- Putting the icon down here means we won't start loading it until the doc is fully parsed -->
+  <link rel="icon" href="file_generic_favicon.ico">
+</body>
+</html>

browser/modules/FaviconLoader.jsm

@@ -385,6 +385,12 @@ class IconLoader {
       return;
     }
 
+    // Let the main process that a tab icon is possibly coming.
+    this.mm.sendAsyncMessage("Link:LoadingIcon", {
+      originalURL: iconInfo.iconUri.spec,
+      canUseForTab: !iconInfo.isRichIcon,
+    });
+
     try {
       this._loader = new FaviconLoad(iconInfo);
       let { dataURL, expiration } = await this._loader.load();

browser/themes/shared/tabs.inc.css

@@ -153,6 +153,7 @@
 
 .tab-throbber,
 .tab-throbber-fallback,
+.tab-icon-pending,
 .tab-icon-image,
 .tab-sharing-icon-overlay,
 .tab-icon-sound,
@@ -162,6 +163,7 @@
 
 .tab-throbber,
 .tab-throbber-fallback,
+.tab-icon-pending,
 .tab-icon-image,
 .tab-sharing-icon-overlay {
   height: 16px;
@@ -171,6 +173,7 @@
 .tab-throbber:not([pinned]),
 .tab-throbber-fallback:not([pinned]),
 .tab-sharing-icon-overlay:not([pinned]),
+.tab-icon-pending:not([pinned]),
 .tab-icon-image:not([pinned]) {
   margin-inline-end: 6px;
 }