Backed out changeset 698252497ed8 (bug 1401777) for failing browser-chrome's browser/base/content/test/general/browser_discovery.js. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sun, 24 Sep 2017 21:43:23 +0200
changeset 382672 076a7c57b868064dc778ae77481d8245ed5b4491
parent 382671 f44a80769c80ef7aec9d175f88dc68897928275d
child 382673 94c2ea91c36fd03edb1fc925571a94587084e485
push id95394
push userarchaeopteryx@coole-files.de
push dateMon, 25 Sep 2017 10:03:57 +0000
treeherdermozilla-inbound@ff48fab67d3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1401777
milestone58.0a1
backs out698252497ed8387d10de995262938316faaf41de
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
Backed out changeset 698252497ed8 (bug 1401777) for failing browser-chrome's browser/base/content/test/general/browser_discovery.js. r=backout
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/general/browser_discovery.js
browser/base/content/test/general/browser_favicon_change.js
browser/components/uitour/test/browser_UITour_availableTargets.js
browser/modules/ContentLinkHandler.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -3696,18 +3696,17 @@ const DOMEventHandler = {
   receiveMessage(aMsg) {
     switch (aMsg.name) {
       case "Link:AddFeed":
         let link = {type: aMsg.data.type, href: aMsg.data.href, title: aMsg.data.title};
         FeedHandler.addFeed(link, aMsg.target);
         break;
 
       case "Link:SetIcon":
-        this.setIcon(aMsg.target, aMsg.data.url, aMsg.data.loadingPrincipal,
-                     aMsg.data.requestContextID, aMsg.data.canUseForTab);
+        this.setIcon(aMsg.target, aMsg.data.url, aMsg.data.loadingPrincipal, aMsg.data.requestContextID);
         break;
 
       case "Link:AddSearch":
         this.addSearch(aMsg.target, aMsg.data.engine, aMsg.data.url);
         break;
 
       case "Meta:SetPageInfo":
         this.setPageInfo(aMsg.data);
@@ -3716,40 +3715,25 @@ const DOMEventHandler = {
   },
 
   setPageInfo(aData) {
     const {url, description, previewImageURL} = aData;
     gBrowser.setPageInfo(url, description, previewImageURL);
     return true;
   },
 
-  setIcon(aBrowser, aURL, aLoadingPrincipal, aRequestContextID = 0, aCanUseForTab = true) {
+  setIcon(aBrowser, aURL, aLoadingPrincipal, aRequestContextID) {
     if (gBrowser.isFailedIcon(aURL))
       return false;
 
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
       return false;
 
-    let loadingPrincipal = aLoadingPrincipal ||
-                           Services.scriptSecurityManager.getSystemPrincipal();
-    if (aURL) {
-      try {
-        if (!(aURL instanceof Ci.nsIURI)) {
-          aURL = makeURI(aURL);
-        }
-        PlacesUIUtils.loadFavicon(aBrowser, loadingPrincipal, aURL, aRequestContextID);
-      } catch (ex) {
-        Components.utils.reportError(ex);
-      }
-    }
-
-    if (aCanUseForTab) {
-      gBrowser.setIcon(tab, aURL, loadingPrincipal, aRequestContextID);
-    }
+    gBrowser.setIcon(tab, aURL, aLoadingPrincipal, aRequestContextID);
     return true;
   },
 
   addSearch(aBrowser, aEngine, aURL) {
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
       return;
 
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -975,19 +975,28 @@
         <parameter name="aTab"/>
         <parameter name="aURI"/>
         <parameter name="aLoadingPrincipal"/>
         <parameter name="aRequestContextID"/>
         <body>
           <![CDATA[
             let browser = this.getBrowserForTab(aTab);
             browser.mIconURL = aURI instanceof Ci.nsIURI ? aURI.spec : aURI;
-            let loadingPrincipal = aLoadingPrincipal ||
-                                   Services.scriptSecurityManager.getSystemPrincipal();
+            let loadingPrincipal = aLoadingPrincipal
+              ? aLoadingPrincipal
+              : Services.scriptSecurityManager.getSystemPrincipal();
             let requestContextID = aRequestContextID || 0;
+
+            if (aURI) {
+              if (!(aURI instanceof Ci.nsIURI)) {
+                aURI = makeURI(aURI);
+              }
+              PlacesUIUtils.loadFavicon(browser, loadingPrincipal, aURI, requestContextID);
+            }
+
             let sizedIconUrl = browser.mIconURL || "";
             if (sizedIconUrl != aTab.getAttribute("image")) {
               if (sizedIconUrl) {
                 if (!browser.mIconLoadingPrincipal ||
                     !browser.mIconLoadingPrincipal.equals(loadingPrincipal)) {
                   aTab.setAttribute("iconLoadingPrincipal",
                     this.serializationHelper.serializeToString(loadingPrincipal));
                   aTab.setAttribute("requestcontextid", requestContextID);
--- a/browser/base/content/test/general/browser_discovery.js
+++ b/browser/base/content/test/general/browser_discovery.js
@@ -1,171 +1,225 @@
-/* eslint-disable mozilla/no-arbitrary-setTimeout */
+var browser;
+
+function doc() {
+  return browser.contentDocument;
+}
+
+function setHandlerFunc(aResultFunc) {
+  gBrowser.addEventListener("DOMLinkAdded", function(event) {
+    executeSoon(aResultFunc);
+  }, {once: true});
+}
 
-add_task(async function() {
-  let rootDir = getRootDirectory(gTestPath);
-  let url = rootDir + "discovery.html";
-  info("Test icons discovery");
-  await BrowserTestUtils.withNewTab(url, iconDiscovery);
-  info("Test search discovery");
-  await BrowserTestUtils.withNewTab(url, searchDiscovery);
-});
+function test() {
+  waitForExplicitFinish();
 
-let iconDiscoveryTests = [
+  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
+  browser = gBrowser.selectedBrowser;
+  browser.addEventListener("load", function(event) {
+    iconDiscovery();
+  }, {capture: true, once: true});
+  var rootDir = getRootDirectory(gTestPath);
+  content.location = rootDir + "discovery.html";
+}
+
+var iconDiscoveryTests = [
   { text: "rel icon discovered" },
   { rel: "abcdefg icon qwerty", text: "rel may contain additional rels separated by spaces" },
   { rel: "ICON", text: "rel is case insensitive" },
   { rel: "shortcut-icon", pass: false, text: "rel shortcut-icon not discovered" },
   { href: "moz.png", text: "relative href works" },
   { href: "notthere.png", text: "404'd icon is removed properly" },
   { href: "data:image/x-icon,%00", type: "image/x-icon", text: "data: URIs work" },
-  { type: "image/png; charset=utf-8", text: "type may have optional parameters (RFC2046)" },
-  { richIcon: true, rel: "apple-touch-icon", text: "apple-touch-icon discovered" },
-  { richIcon: true, rel: "apple-touch-icon-precomposed", text: "apple-touch-icon-precomposed discovered" },
-  { richIcon: true, rel: "fluid-icon", text: "fluid-icon discovered" },
-  { richIcon: true, rel: "unknown-icon", pass: false, text: "unknown icon not discovered" }
+  { type: "image/png; charset=utf-8", text: "type may have optional parameters (RFC2046)" }
 ];
 
-async function iconDiscovery() {
-  let mm = window.messageManager;
-  for (let testCase of iconDiscoveryTests) {
+function runIconDiscoveryTest() {
+  let testCase = iconDiscoveryTests[0];
+  let head = doc().getElementById("linkparent");
+
+  // Because there is debounce logic in ContentLinkHandler.jsm to reduce the
+  // favicon loads, we have to wait some time before checking that icon was
+  // stored properly.
+  BrowserTestUtils.waitForCondition(() => {
+    return gBrowser.getIcon() != null;
+  }, "wait for icon load to finish", 100, 5)
+  .then(() => {
+    ok(testCase.pass, testCase.text);
+  })
+  .catch(() => {
+    ok(!testCase.pass, testCase.text);
+  })
+  .then(() => {
+    head.removeChild(head.getElementsByTagName("link")[0]);
+    iconDiscoveryTests.shift();
+    iconDiscovery(); // Run the next test.
+  });
+}
+
+function iconDiscovery() {
+  if (iconDiscoveryTests.length) {
+    setHandlerFunc(runIconDiscoveryTest);
+    gBrowser.setIcon(gBrowser.selectedTab, null,
+                     Services.scriptSecurityManager.getSystemPrincipal());
+
+    var testCase = iconDiscoveryTests[0];
+    var head = doc().getElementById("linkparent");
+    var link = doc().createElement("link");
+
+    var rootDir = getRootDirectory(gTestPath);
+    var rel = testCase.rel || "icon";
+    var href = testCase.href || rootDir + "moz.png";
+    var type = testCase.type || "image/png";
     if (testCase.pass == undefined)
       testCase.pass = true;
-    testCase.rootDir = getRootDirectory(gTestPath);
 
-    // Clear the current icon.
-    gBrowser.setIcon(gBrowser.selectedTab, null);
-
-    let promiseLinkAdded =
-      BrowserTestUtils.waitForEvent(gBrowser.selectedBrowser, "DOMLinkAdded",
-                                    false, null, true);
-    let promiseMessage = new Promise(resolve => {
-      testCase.listener = resolve;
-      mm.addMessageListener("Link:SetIcon", testCase.listener);
-    });
-
-    await ContentTask.spawn(gBrowser.selectedBrowser, testCase, test => {
-      let doc = content.document;
-      let head = doc.getElementById("linkparent");
-      let link = doc.createElement("link");
-      link.rel = test.rel || "icon";
-      link.href = test.href || test.rootDir + "moz.png";
-      link.type = test.type || "image/png";
-      head.appendChild(link);
-    });
-
-    await promiseLinkAdded;
-
-    if (!testCase.richIcon) {
-      // Because there is debounce logic in ContentLinkHandler.jsm to reduce the
-      // favicon loads, we have to wait some time before checking that icon was
-      // stored properly.
-      try {
-        await BrowserTestUtils.waitForCondition(() => {
-          return gBrowser.getIcon() != null;
-        }, "wait for icon load to finish", 100, 5);
-        ok(testCase.pass, testCase.text);
-      } catch (ex) {
-        ok(!testCase.pass, testCase.text);
-      }
-    } else {
-      // Rich icons are not set as tab icons, so just check for the SetIcon message.
-      try {
-        let msg = await Promise.race([promiseMessage,
-                                      new Promise((resolve, reject) => setTimeout(reject(), 2000))]);
-        is(msg.data.canUseForTab, false, "Rich icons cannot be used for tabs");
-        ok(testCase.pass, testCase.text);
-      } catch (ex) {
-        ok(!testCase.pass, testCase.text);
-      } finally {
-        mm.removeMessageListener("Link:SetIcon", testCase.listener);
-      }
-    }
-
-    await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
-      let head = content.document.getElementById("linkparent");
-      head.removeChild(head.getElementsByTagName("link")[0]);
-    });
+    link.rel = rel;
+    link.href = href;
+    link.type = type;
+    head.appendChild(link);
+  } else {
+    richIconDiscovery();
   }
 }
 
-let searchDiscoveryTests = [
+let richIconDiscoveryTests = [
+  { rel: "apple-touch-icon", text: "apple-touch-icon discovered" },
+  { rel: "apple-touch-icon-precomposed", text: "apple-touch-icon-precomposed discovered" },
+  { rel: "fluid-icon", text: "fluid-icon discovered" },
+  { rel: "unknown-icon", pass: false, text: "unknown icon not discovered" }
+];
+
+function runRichIconDiscoveryTest() {
+  let testCase = richIconDiscoveryTests[0];
+  let head = doc().getElementById("linkparent");
+
+  // Because there is debounce logic in ContentLinkHandler.jsm to reduce the
+  // favicon loads, we have to wait some time before checking that icon was
+  // stored properly.
+  BrowserTestUtils.waitForCondition(() => {
+    return gBrowser.getIcon() != null;
+  }, "wait for icon load to finish", 100, 5)
+  .then(() => {
+    ok(testCase.pass, testCase.text);
+  })
+  .catch(() => {
+    ok(!testCase.pass, testCase.text);
+  })
+  .then(() => {
+    head.removeChild(head.getElementsByTagName("link")[0]);
+    richIconDiscoveryTests.shift();
+    richIconDiscovery(); // Run the next test.
+  });
+}
+
+function richIconDiscovery() {
+  if (richIconDiscoveryTests.length) {
+    setHandlerFunc(runRichIconDiscoveryTest);
+    gBrowser.setIcon(gBrowser.selectedTab, null,
+                     Services.scriptSecurityManager.getSystemPrincipal()
+    );
+
+    let testCase = richIconDiscoveryTests[0];
+    let head = doc().getElementById("linkparent");
+    let link = doc().createElement("link");
+
+    let rel = testCase.rel;
+    let rootDir = getRootDirectory(gTestPath);
+    let href = testCase.href || rootDir + "moz.png";
+    let type = testCase.type || "image/png";
+    if (testCase.pass === undefined)
+      testCase.pass = true;
+
+    link.rel = rel;
+    link.href = href;
+    link.type = type;
+    head.appendChild(link);
+  } else {
+    searchDiscovery();
+  }
+}
+
+var searchDiscoveryTests = [
   { text: "rel search discovered" },
   { rel: "SEARCH", text: "rel is case insensitive" },
   { rel: "-search-", pass: false, text: "rel -search- not discovered" },
   { rel: "foo bar baz search quux", text: "rel may contain additional rels separated by spaces" },
   { href: "https://not.mozilla.com", text: "HTTPS ok" },
   { href: "ftp://not.mozilla.com", text: "FTP ok" },
   { href: "data:text/foo,foo", pass: false, text: "data URI not permitted" },
   { href: "javascript:alert(0)", pass: false, text: "JS URI not permitted" },
   { type: "APPLICATION/OPENSEARCHDESCRIPTION+XML", text: "type is case insensitve" },
   { type: " application/opensearchdescription+xml ", text: "type may contain extra whitespace" },
   { type: "application/opensearchdescription+xml; charset=utf-8", text: "type may have optional parameters (RFC2046)" },
   { type: "aapplication/opensearchdescription+xml", pass: false, text: "type should not be loosely matched" },
   { rel: "search search search", count: 1, text: "only one engine should be added" }
 ];
 
-async function searchDiscovery() {
-  let browser = gBrowser.selectedBrowser;
+function runSearchDiscoveryTest() {
+  var testCase = searchDiscoveryTests[0];
+  var title = testCase.title || searchDiscoveryTests.length;
+  if (browser.engines) {
+    var hasEngine = (testCase.count) ? (browser.engines[0].title == title &&
+                                        browser.engines.length == testCase.count) :
+                                       (browser.engines[0].title == title);
+    ok(hasEngine, testCase.text);
+    browser.engines = null;
+  } else
+    ok(!testCase.pass, testCase.text);
+
+  searchDiscoveryTests.shift();
+  searchDiscovery(); // Run the next test.
+}
 
-  for (let testCase of searchDiscoveryTests) {
+// This handler is called twice, once for each added link element.
+// Only want to check once the second link element has been added.
+var ranOnce = false;
+function runMultipleEnginesTestAndFinalize() {
+  if (!ranOnce) {
+    ranOnce = true;
+    return;
+  }
+  ok(browser.engines, "has engines");
+  is(browser.engines.length, 1, "only one engine");
+  is(browser.engines[0].uri, "http://first.mozilla.com/search.xml", "first engine wins");
+
+  gBrowser.removeCurrentTab();
+  finish();
+}
+
+function searchDiscovery() {
+  let head = doc().getElementById("linkparent");
+
+  if (searchDiscoveryTests.length) {
+    setHandlerFunc(runSearchDiscoveryTest);
+    let testCase = searchDiscoveryTests[0];
+    let link = doc().createElement("link");
+
+    let rel = testCase.rel || "search";
+    let href = testCase.href || "http://so.not.here.mozilla.com/search.xml";
+    let type = testCase.type || "application/opensearchdescription+xml";
+    let title = testCase.title || searchDiscoveryTests.length;
     if (testCase.pass == undefined)
       testCase.pass = true;
-    testCase.title = testCase.title || searchDiscoveryTests.indexOf(testCase);
 
-    let promiseLinkAdded =
-      BrowserTestUtils.waitForEvent(gBrowser.selectedBrowser, "DOMLinkAdded",
-                                    false, null, true);
-
-    await ContentTask.spawn(gBrowser.selectedBrowser, testCase, test => {
-      let doc = content.document;
-      let head = doc.getElementById("linkparent");
-      let link = doc.createElement("link");
-      link.rel = test.rel || "search";
-      link.href = test.href || "http://so.not.here.mozilla.com/search.xml";
-      link.type = test.type || "application/opensearchdescription+xml";
-      link.title = test.title;
-      head.appendChild(link);
-    });
-
-    await promiseLinkAdded;
-    await new Promise(resolve => executeSoon(resolve));
-
-    if (browser.engines) {
-      info(`Found ${browser.engines.length} engines`);
-      info(`First engine title: ${browser.engines[0].title}`);
-      let hasEngine = testCase.count ?
-        (browser.engines[0].title == testCase.title && browser.engines.length == testCase.count) :
-        (browser.engines[0].title == testCase.title);
-      ok(hasEngine, testCase.text);
-      browser.engines = null;
-    } else {
-      ok(!testCase.pass, testCase.text);
-    }
-  }
-
-  info("Test multiple engines with the same title");
-  let count = 0;
-  let promiseLinkAdded =
-    BrowserTestUtils.waitForEvent(gBrowser.selectedBrowser, "DOMLinkAdded",
-                                  false, () => ++count == 2, true);
-  await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
-    let doc = content.document;
-    let head = doc.getElementById("linkparent");
-    let link = doc.createElement("link");
+    link.rel = rel;
+    link.href = href;
+    link.type = type;
+    link.title = title;
+    head.appendChild(link);
+  } else {
+    setHandlerFunc(runMultipleEnginesTestAndFinalize);
+    setHandlerFunc(runMultipleEnginesTestAndFinalize);
+    // Test multiple engines with the same title
+    let link = doc().createElement("link");
     link.rel = "search";
     link.href = "http://first.mozilla.com/search.xml";
     link.type = "application/opensearchdescription+xml";
     link.title = "Test Engine";
     let link2 = link.cloneNode(false);
     link2.href = "http://second.mozilla.com/search.xml";
+
     head.appendChild(link);
     head.appendChild(link2);
-  });
-
-  await promiseLinkAdded;
-  await new Promise(resolve => executeSoon(resolve));
-
-  ok(browser.engines, "has engines");
-  is(browser.engines.length, 1, "only one engine");
-  is(browser.engines[0].uri, "http://first.mozilla.com/search.xml", "first engine wins");
-  browser.engines = null;
+  }
 }
--- a/browser/base/content/test/general/browser_favicon_change.js
+++ b/browser/base/content/test/general/browser_favicon_change.js
@@ -12,18 +12,17 @@ add_task(async function() {
   let expectedFavicon = "http://example.org/one-icon";
   let haveChanged = PromiseUtils.defer();
   let observer = new MutationObserver(function(mutations) {
     for (let mut of mutations) {
       if (mut.attributeName != "image") {
         continue;
       }
       let imageVal = extraTab.getAttribute("image").replace(/#.*$/, "");
-      // Ignore chrome favicons set on the tab before the actual page load.
-      if (!imageVal || !imageVal.startsWith("http://example.org/")) {
+      if (!imageVal) {
         // The value gets removed because it doesn't load.
         continue;
       }
       is(imageVal, expectedFavicon, "Favicon image should correspond to expected image.");
       haveChanged.resolve();
     }
   });
   observer.observe(extraTab, {attributes: true});
--- a/browser/components/uitour/test/browser_UITour_availableTargets.js
+++ b/browser/components/uitour/test/browser_UITour_availableTargets.js
@@ -111,21 +111,24 @@ add_UITour_task(async function test_avai
   for (let [ targetName, expectedNodeId ] of expectedActions) {
     await assertTargetNode(targetName, expectedNodeId);
   }
   pageActionsHelper.restoreActionsUrlbarState();
 });
 
 function ok_targets(actualData, expectedTargets) {
   // Depending on how soon after page load this is called, the selected tab icon
-  // may or may not be showing the loading throbber.  We can't be sure whether
-  // it appears in the list of targets, so remove it.
-  let index = actualData.targets.indexOf("selectedTabIcon");
-  if (index != -1)
-    actualData.targets.splice(index, 1);
+  // may or may not be showing the loading throbber.  Check for its presence and
+  // insert it into expectedTargets if it's visible.
+  let selectedTabIcon =
+    document.getAnonymousElementByAttribute(gBrowser.selectedTab,
+                                            "anonid",
+                                            "tab-icon-image");
+  if (selectedTabIcon && UITour.isElementVisible(selectedTabIcon))
+    expectedTargets.push("selectedTabIcon");
 
   ok(Array.isArray(actualData.targets), "data.targets should be an array");
   is(actualData.targets.sort().toString(), expectedTargets.sort().toString(),
      "Targets should be as expected");
 }
 
 async function assertTargetNode(targetName, expectedNodeId) {
   let target = await UITour.getTarget(window, targetName);
--- a/browser/modules/ContentLinkHandler.jsm
+++ b/browser/modules/ContentLinkHandler.jsm
@@ -109,19 +109,17 @@ function getLinkIconURI(aLink) {
  * }.
  * @param {Object} aChromeGlobal A global chrome object.
  */
 function setIconForLink(aIconInfo, aChromeGlobal) {
   aChromeGlobal.sendAsyncMessage(
     "Link:SetIcon",
     { url: aIconInfo.iconUri.spec,
       loadingPrincipal: aIconInfo.loadingPrincipal,
-      requestContextID: aIconInfo.requestContextID,
-      canUseForTab: !aIconInfo.isRichIcon,
-    });
+      requestContextID: aIconInfo.requestContextID });
 }
 
 /*
  * Timeout callback function for loading favicon.
  *
  * @param {Map} aFaviconLoads A map of page URL and FaviconLoad object pairs,
  *   where the FaviconLoad object looks like {
  *     timer: a nsITimer object,
@@ -146,41 +144,37 @@ function faviconTimeoutCallback(aFavicon
   for (let icon of load.iconInfos) {
     if (icon.type === "image/svg+xml" ||
       icon.type === "image/x-icon" ||
       icon.type === "image/vnd.microsoft.icon") {
       preferredIcon = icon;
       continue;
     }
 
-    // Note that some sites use hi-res icons without specifying them as
-    // apple-touch or fluid icons.
-    if (icon.isRichIcon || icon.width >= FAVICON_RICH_ICON_MIN_WIDTH) {
+    if (icon.isRichIcon) {
       if (!largestRichIcon || largestRichIcon.width < icon.width) {
         largestRichIcon = icon;
       }
     } else if (!defaultIcon) {
       defaultIcon = icon;
     }
   }
 
   // Now set the favicons for the page in the following order:
-  // 1. Set the best rich icon if any.
-  // 2. Set the preferred one if any, otherwise use the default one.
-  // This order allows smaller icon frames to eventually override rich icon
-  // frames.
-  if (largestRichIcon) {
-    setIconForLink(largestRichIcon, aChromeGlobal);
-  }
+  // 1. Set the preferred one if any, otherwise use the default one.
+  // 2. Set the best rich icon if any.
   if (preferredIcon) {
     setIconForLink(preferredIcon, aChromeGlobal);
   } else if (defaultIcon) {
     setIconForLink(defaultIcon, aChromeGlobal);
   }
 
+  if (largestRichIcon) {
+    setIconForLink(largestRichIcon, aChromeGlobal);
+  }
   load.timer = null;
   aFaviconLoads.delete(aPageUrl);
 }
 
 /*
  * Get request context ID of the link dom node's document.
  *
  * @param {DOMNode} aLink A link dom node.
@@ -205,18 +199,22 @@ function getLinkRequestContextID(aLink) 
  * @return {bool} Returns true if the link is successfully handled.
  */
 function handleFaviconLink(aLink, aIsRichIcon, aChromeGlobal, aFaviconLoads) {
   let pageUrl = aLink.ownerDocument.documentURI;
   let iconUri = getLinkIconURI(aLink);
   if (!iconUri)
     return false;
 
-  // Extract the size type and width.
+  // Extract the size type and width. Note that some sites use hi-res icons
+  // without specifying them as apple-touch or fluid icons.
   let width = extractIconSize(aLink.sizes);
+  if (width >= FAVICON_RICH_ICON_MIN_WIDTH)
+    aIsRichIcon = true;
+
   let iconInfo = {
     iconUri,
     width,
     isRichIcon: aIsRichIcon,
     type: aLink.type,
     loadingPrincipal: aLink.ownerDocument.nodePrincipal,
     requestContextID: getLinkRequestContextID(aLink)
   };