Bug 1186139 - Don't use a sync message where we don't have to. r=felipe
authorBlake Kaplan <mrbkap@gmail.com>
Fri, 11 Mar 2016 12:06:05 -0800
changeset 290514 0dca990f8325a584c30332ab2b3053451e00a59a
parent 290513 81fe08eb86f500d7d79bbdb4690c009b93baeadc
child 290515 9ebccbbb04a65870d49627fa8ca701c494974cef
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfelipe
bugs1186139
milestone48.0a1
Bug 1186139 - Don't use a sync message where we don't have to. r=felipe
browser/base/content/browser.js
browser/base/content/test/general/browser_bug408415.js
browser/base/content/test/general/browser_bug550565.js
browser/modules/ContentLinkHandler.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -3348,17 +3348,17 @@ const DOMLinkHandler = {
   receiveMessage: function (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":
-        return this.setIcon(aMsg.target, aMsg.data.url, aMsg.data.loadingPrincipal);
+        this.setIcon(aMsg.target, aMsg.data.url, aMsg.data.loadingPrincipal);
         break;
 
       case "Link:AddSearch":
         this.addSearch(aMsg.target, aMsg.data.engine, aMsg.data.url);
         break;
     }
   },
 
--- a/browser/base/content/test/general/browser_bug408415.js
+++ b/browser/base/content/test/general/browser_bug408415.js
@@ -1,21 +1,45 @@
-function test() {
-  waitForExplicitFinish();
-
+add_task(function* test() {
   let testPath = getRootDirectory(gTestPath);
 
-  let tab = gBrowser.addTab(testPath + "file_with_favicon.html");
+  yield BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" },
+    function* (tabBrowser) {
+      const URI = testPath + "file_with_favicon.html";
+      const expectedIcon = testPath + "file_generic_favicon.ico";
 
-  tab.linkedBrowser.addEventListener("DOMContentLoaded", function() {
-    tab.linkedBrowser.removeEventListener("DOMContentLoaded", arguments.callee, true);
+      let got_favicon = Promise.defer();
+      let listener = {
+        onLinkIconAvailable(browser, iconURI) {
+          if (got_favicon && iconURI && browser === tabBrowser) {
+            got_favicon.resolve(iconURI);
+            got_favicon = null;
+          }
+        }
+      };
+      gBrowser.addTabsProgressListener(listener);
 
-    let expectedIcon = testPath + "file_generic_favicon.ico";
+      BrowserTestUtils.loadURI(tabBrowser, URI);
+
+      let iconURI = yield got_favicon.promise;
+      is(iconURI, expectedIcon, "Correct icon before pushState.");
 
-    is(gBrowser.getIcon(tab), expectedIcon, "Correct icon before hash change.");
-    tab.linkedBrowser.contentWindow.location.href += "#foo";
-    is(gBrowser.getIcon(tab), expectedIcon, "Correct icon after hash change.");
+      got_favicon = Promise.defer();
+      got_favicon.promise.then(() => { ok(false, "shouldn't be called"); }, (e) => e);
+      yield ContentTask.spawn(tabBrowser, null, function() {
+        content.location.href += "#foo";
+      });
 
-    gBrowser.removeTab(tab);
+      // We've navigated and shouldn't get a call to onLinkIconAvailable.
+      TestUtils.executeSoon(() => {
+        got_favicon.reject(gBrowser.getIcon(gBrowser.getTabForBrowser(tabBrowser)));
+      });
+      try {
+        yield got_favicon.promise;
+      } catch (e) {
+        iconURI = e;
+      }
+      is(iconURI, expectedIcon, "Correct icon after pushState.");
 
-    finish();
-  }, true);
-}
+      gBrowser.removeTabsProgressListener(listener);
+    });
+});
+
--- a/browser/base/content/test/general/browser_bug550565.js
+++ b/browser/base/content/test/general/browser_bug550565.js
@@ -1,21 +1,44 @@
-function test() {
-  waitForExplicitFinish();
-
+add_task(function* test() {
   let testPath = getRootDirectory(gTestPath);
 
-  let tab = gBrowser.addTab(testPath + "file_with_favicon.html");
+  yield BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" },
+    function* (tabBrowser) {
+      const URI = testPath + "file_with_favicon.html";
+      const expectedIcon = testPath + "file_generic_favicon.ico";
 
-  tab.linkedBrowser.addEventListener("DOMContentLoaded", function() {
-    tab.linkedBrowser.removeEventListener("DOMContentLoaded", arguments.callee, true);
+      let got_favicon = Promise.defer();
+      let listener = {
+        onLinkIconAvailable(browser, iconURI) {
+          if (got_favicon && iconURI && browser === tabBrowser) {
+            got_favicon.resolve(iconURI);
+            got_favicon = null;
+          }
+        }
+      };
+      gBrowser.addTabsProgressListener(listener);
 
-    let expectedIcon = testPath + "file_generic_favicon.ico";
+      BrowserTestUtils.loadURI(tabBrowser, URI);
+
+      let iconURI = yield got_favicon.promise;
+      is(iconURI, expectedIcon, "Correct icon before pushState.");
 
-    is(gBrowser.getIcon(tab), expectedIcon, "Correct icon before pushState.");
-    tab.linkedBrowser.contentWindow.history.pushState("page2", "page2", "page2");
-    is(gBrowser.getIcon(tab), expectedIcon, "Correct icon after pushState.");
+      got_favicon = Promise.defer();
+      got_favicon.promise.then(() => { ok(false, "shouldn't be called"); }, (e) => e);
+      yield ContentTask.spawn(tabBrowser, null, function() {
+        content.history.pushState("page2", "page2", "page2");
+      });
 
-    gBrowser.removeTab(tab);
+      // We've navigated and shouldn't get a call to onLinkIconAvailable.
+      TestUtils.executeSoon(() => {
+        got_favicon.reject(gBrowser.getIcon(gBrowser.getTabForBrowser(tabBrowser)));
+      });
+      try {
+        yield got_favicon.promise;
+      } catch (e) {
+        iconURI = e;
+      }
+      is(iconURI, expectedIcon, "Correct icon after pushState.");
 
-    finish();
-  }, true);
-}
+      gBrowser.removeTabsProgressListener(listener);
+    });
+});
--- a/browser/modules/ContentLinkHandler.jsm
+++ b/browser/modules/ContentLinkHandler.jsm
@@ -66,57 +66,56 @@ this.ContentLinkHandler = {
                                             {type: link.type,
                                              href: link.href,
                                              title: link.title});
               feedAdded = true;
             }
           }
           break;
         case "icon":
-          if (!iconAdded) {
-            if (!Services.prefs.getBoolPref("browser.chrome.site_icons"))
-              break;
+          if (iconAdded || !Services.prefs.getBoolPref("browser.chrome.site_icons"))
+            break;
 
-            var uri = this.getLinkIconURI(link);
-            if (!uri)
-              break;
+          var uri = this.getLinkIconURI(link);
+          if (!uri)
+            break;
 
-            // Telemetry probes for measuring the sizes attribute
-            // usage and available dimensions.
-            let sizeHistogramTypes = Services.telemetry.
-                                     getHistogramById("LINK_ICON_SIZES_ATTR_USAGE");
-            let sizeHistogramDimension = Services.telemetry.
-                                         getHistogramById("LINK_ICON_SIZES_ATTR_DIMENSION");
-            let sizesType;
-            if (link.sizes.length) {
-              for (let size of link.sizes) {
-                if (size.toLowerCase() == "any") {
-                  sizesType = SIZES_TELEMETRY_ENUM.ANY;
+          // Telemetry probes for measuring the sizes attribute
+          // usage and available dimensions.
+          let sizeHistogramTypes = Services.telemetry.
+                                   getHistogramById("LINK_ICON_SIZES_ATTR_USAGE");
+          let sizeHistogramDimension = Services.telemetry.
+                                       getHistogramById("LINK_ICON_SIZES_ATTR_DIMENSION");
+          let sizesType;
+          if (link.sizes.length) {
+            for (let size of link.sizes) {
+              if (size.toLowerCase() == "any") {
+                sizesType = SIZES_TELEMETRY_ENUM.ANY;
+                break;
+              } else {
+                let re = /^([1-9][0-9]*)x[1-9][0-9]*$/i;
+                let values = re.exec(size);
+                if (values && values.length > 1) {
+                  sizesType = SIZES_TELEMETRY_ENUM.DIMENSION;
+                  sizeHistogramDimension.add(parseInt(values[1]));
+                } else {
+                  sizesType = SIZES_TELEMETRY_ENUM.INVALID;
                   break;
-                } else {
-                  let re = /^([1-9][0-9]*)x[1-9][0-9]*$/i;
-                  let values = re.exec(size);
-                  if (values && values.length > 1) {
-                    sizesType = SIZES_TELEMETRY_ENUM.DIMENSION;
-                    sizeHistogramDimension.add(parseInt(values[1]));
-                  } else {
-                    sizesType = SIZES_TELEMETRY_ENUM.INVALID;
-                    break;
-                  }
                 }
               }
-            } else {
-              sizesType = SIZES_TELEMETRY_ENUM.NO_SIZES;
             }
-            sizeHistogramTypes.add(sizesType);
+          } else {
+            sizesType = SIZES_TELEMETRY_ENUM.NO_SIZES;
+          }
+          sizeHistogramTypes.add(sizesType);
 
-            [iconAdded] = chromeGlobal.sendSyncMessage(
-                            "Link:SetIcon",
-                            {url: uri.spec, loadingPrincipal: link.ownerDocument.nodePrincipal});
-          }
+          chromeGlobal.sendAsyncMessage(
+            "Link:SetIcon",
+            {url: uri.spec, loadingPrincipal: link.ownerDocument.nodePrincipal});
+          iconAdded = true;
           break;
         case "search":
           if (!searchAdded && event.type == "DOMLinkAdded") {
             var type = link.type && link.type.toLowerCase();
             type = type.replace(/^\s+|\s*(?:;.*)?$/g, "");
 
             let re = /^(?:https?|ftp):/i;
             if (type == "application/opensearchdescription+xml" && link.title &&