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

@@ -3701,8 +3701,7 @@ const DOMEventHandler = {
         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":
@@ -3721,7 +3720,7 @@ const DOMEventHandler = {
     return true;
   },
 
-  setIcon(aBrowser, aURL, aLoadingPrincipal, aRequestContextID = 0, aCanUseForTab = true) {
+  setIcon(aBrowser, aURL, aLoadingPrincipal, aRequestContextID) {
     if (gBrowser.isFailedIcon(aURL))
       return false;
 
@@ -3729,22 +3728,7 @@ const DOMEventHandler = {
     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;
   },
 

browser/base/content/tabbrowser.xml

@@ -980,9 +980,18 @@
           <![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) {

browser/base/content/test/general/browser_discovery.js

@@ -1,15 +1,28 @@
-/* eslint-disable mozilla/no-arbitrary-setTimeout */
+var browser;
 
-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 doc() {
+  return browser.contentDocument;
+}
 
-let iconDiscoveryTests = [
+function setHandlerFunc(aResultFunc) {
+  gBrowser.addEventListener("DOMLinkAdded", function(event) {
+    executeSoon(aResultFunc);
+  }, {once: true});
+}
+
+function test() {
+  waitForExplicitFinish();
+
+  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" },
@@ -17,77 +30,116 @@ let iconDiscoveryTests = [
   { 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" },
@@ -103,69 +155,71 @@ let searchDiscoveryTests = [
   { 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);
 
-  for (let testCase of searchDiscoveryTests) {
+  searchDiscoveryTests.shift();
+  searchDiscovery(); // Run the next test.
+}
+
+// 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;
+  }
 }

browser/base/content/test/general/browser_favicon_change.js

@@ -17,8 +17,7 @@ add_task(async function() {
         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;
       }

browser/components/uitour/test/browser_UITour_availableTargets.js

@@ -116,11 +116,14 @@ add_UITour_task(async function test_availableTargets_addUrlbarPageActionsAll() {
 
 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(),

browser/modules/ContentLinkHandler.jsm

@@ -114,9 +114,7 @@ function setIconForLink(aIconInfo, aChromeGlobal) {
     "Link:SetIcon",
     { url: aIconInfo.iconUri.spec,
       loadingPrincipal: aIconInfo.loadingPrincipal,
-      requestContextID: aIconInfo.requestContextID,
-      canUseForTab: !aIconInfo.isRichIcon,
-    });
+      requestContextID: aIconInfo.requestContextID });
 }
 
 /*
@@ -151,9 +149,7 @@ function faviconTimeoutCallback(aFaviconLoads, aPageUrl, aChromeGlobal) {
       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;
       }
@@ -163,19 +159,17 @@ function faviconTimeoutCallback(aFaviconLoads, aPageUrl, aChromeGlobal) {
   }
 
   // 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);
 }
@@ -210,8 +204,12 @@ function handleFaviconLink(aLink, aIsRichIcon, aChromeGlobal, aFaviconLoads) {
   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,