Bug 1453580 - Remove promiseFaviconLinkUrl and fix its consumers. r=Gijs
☠☠ backed out by 09c4cd5c8946 ☠ ☠
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 16 Apr 2018 18:24:06 +0200
changeset 414180 897bbf35e781fc7067b7e358379fad54d5342a4f
parent 414179 f3691336c4e412e7873bf0fb6d02d9fdd874ab8c
child 414181 c513037dd5511c2d34e73aa22d9e70d6c18bd15c
push id62815
push usermak77@bonardo.net
push dateWed, 18 Apr 2018 10:11:42 +0000
treeherderautoland@897bbf35e781 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1453580
milestone61.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 1453580 - Remove promiseFaviconLinkUrl and fix its consumers. r=Gijs MozReview-Commit-ID: GrY8s3l71Mp
browser/modules/ReaderParent.jsm
services/sync/modules/SyncedTabs.jsm
toolkit/components/passwordmgr/content/passwordManager.js
toolkit/components/places/PlacesUtils.jsm
toolkit/components/reader/AboutReader.jsm
--- a/browser/modules/ReaderParent.jsm
+++ b/browser/modules/ReaderParent.jsm
@@ -17,26 +17,31 @@ ChromeUtils.defineModuleGetter(this, "UI
 const gStringBundle = Services.strings.createBundle("chrome://global/locale/aboutReader.properties");
 
 var ReaderParent = {
   // Listeners are added in nsBrowserGlue.js
   receiveMessage(message) {
     switch (message.name) {
       case "Reader:FaviconRequest": {
         if (message.target.messageManager) {
-          let faviconUrl = PlacesUtils.promiseFaviconLinkUrl(message.data.url);
-          faviconUrl.then(function onResolution(favicon) {
-            message.target.messageManager.sendAsyncMessage("Reader:FaviconReturn", {
-              url: message.data.url,
-              faviconUrl: favicon.pathQueryRef.replace(/^favicon:/, "")
-            });
-          },
-          function onRejection(reason) {
-            Cu.reportError("Error requesting favicon URL for about:reader content: " + reason);
-          }).catch(Cu.reportError);
+          try {
+            let preferredWidth = message.data.preferredWidth || 0;
+            let uri = Services.io.newURI(message.data.url);
+            PlacesUtils.favicons.getFaviconURLForPage(uri, iconUri => {
+              if (iconUri) {
+                iconUri = PlacesUtils.favicons.getFaviconLinkForIcon(iconUri);
+                message.target.messageManager.sendAsyncMessage("Reader:FaviconReturn", {
+                  url: message.data.url,
+                  faviconUrl: iconUri.pathQueryRef.replace(/^favicon:/, "")
+                });
+              }
+            }, preferredWidth);
+          } catch (ex) {
+            Cu.reportError("Error requesting favicon URL for about:reader content: " + ex);
+          }
         }
         break;
       }
 
       case "Reader:UpdateReaderButton": {
         let browser = message.target;
         if (message.data && message.data.isArticle !== undefined) {
           browser.isArticle = message.data.isArticle;
--- a/services/sync/modules/SyncedTabs.jsm
+++ b/services/sync/modules/SyncedTabs.jsm
@@ -48,22 +48,20 @@ XPCOMUtils.defineLazyGetter(this, "log",
 let SyncedTabsInternal = {
   /* Make a "tab" record. Returns a promise */
   async _makeTab(client, tab, url, showRemoteIcons) {
     let icon;
     if (showRemoteIcons) {
       icon = tab.icon;
     }
     if (!icon) {
-      try {
-        icon = (await PlacesUtils.promiseFaviconLinkUrl(url)).spec;
-      } catch (ex) { /* no favicon avaiable */ }
-    }
-    if (!icon) {
-      icon = "";
+      // By not specifying a size the favicon service will pick the default,
+      // that is usually set through setDefaultIconURIPreferredSize by the
+      // first browser window. Commonly it's 16px at current dpi.
+      icon = "page-icon:" + url;
     }
     return {
       type:  "tab",
       title: tab.title || url,
       url,
       icon,
       client: client.id,
       lastUsed: tab.lastUsed,
--- a/toolkit/components/passwordmgr/content/passwordManager.js
+++ b/toolkit/components/passwordmgr/content/passwordManager.js
@@ -113,52 +113,30 @@ function Shutdown() {
 }
 
 function setFilter(aFilterString) {
   filterField.value = aFilterString;
   FilterPasswords();
 }
 
 let signonsTreeView = {
-  // Keep track of which favicons we've fetched or started fetching.
-  // Maps a login origin to a favicon URL.
-  _faviconMap: new Map(),
   _filterSet: [],
-  // Coalesce invalidations to avoid repeated flickering.
-  _invalidateTask: new DeferredTask(() => {
-    signonsTree.treeBoxObject.invalidateColumn(signonsTree.columns.siteCol);
-  }, 10, 0),
   _lastSelectedRanges: [],
   selection: null,
 
   rowCount: 0,
   setTree(tree) {},
   getImageSrc(row, column) {
     if (column.element.getAttribute("id") !== "siteCol") {
       return "";
     }
 
     const signon = GetVisibleLogins()[row];
 
-    // We already have the favicon URL or we started to fetch (value is null).
-    if (this._faviconMap.has(signon.hostname)) {
-      return this._faviconMap.get(signon.hostname);
-    }
-
-    // Record the fact that we already starting fetching a favicon for this
-    // origin in order to avoid multiple requests for the same origin.
-    this._faviconMap.set(signon.hostname, null);
-
-    PlacesUtils.promiseFaviconLinkUrl(signon.hostname)
-      .then(faviconURI => {
-        this._faviconMap.set(signon.hostname, faviconURI.spec);
-        this._invalidateTask.arm();
-      }).catch(Cu.reportError);
-
-    return "";
+    return PlacesUtils.urlWithSizeRef(window, "page-icon:" + signon.hostname, 16);
   },
   getCellValue(row, column) {},
   getCellText(row, column) {
     let time;
     let signon = GetVisibleLogins()[row];
     switch (column.id) {
       case "siteCol":
         return signon.httpRealm ?
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1523,39 +1523,16 @@ var PlacesUtils = {
             resolve({ uri, dataLen, data, mimeType });
           } else {
             reject();
           }
         });
     });
   },
 
-  /**
-   * Gets the favicon link url (moz-anno:) for a given page url.
-   *
-   * @param aPageURL url of the page to lookup the favicon for.
-   * @resolves to the nsIURL of the favicon link
-   * @rejects if the given url has no associated favicon.
-   */
-  promiseFaviconLinkUrl(aPageUrl) {
-    return new Promise((resolve, reject) => {
-      if (!(aPageUrl instanceof Ci.nsIURI))
-        aPageUrl = NetUtil.newURI(aPageUrl);
-
-      PlacesUtils.favicons.getFaviconURLForPage(aPageUrl, uri => {
-        if (uri) {
-          uri = PlacesUtils.favicons.getFaviconLinkForIcon(uri);
-          resolve(uri);
-        } else {
-          reject("favicon not found for uri");
-        }
-      });
-    });
-  },
-
    /**
    * Returns the passed URL with a #size ref for the specified size and
    * devicePixelRatio.
    *
    * @param window
    *        The window where the icon will appear.
    * @param href
    *        The string href we should add the ref to.
--- a/toolkit/components/reader/AboutReader.jsm
+++ b/toolkit/components/reader/AboutReader.jsm
@@ -698,17 +698,20 @@ AboutReader.prototype = {
 
   _requestFavicon() {
     let handleFaviconReturn = (message) => {
       this._mm.removeMessageListener("Reader:FaviconReturn", handleFaviconReturn);
       this._loadFavicon(message.data.url, message.data.faviconUrl);
     };
 
     this._mm.addMessageListener("Reader:FaviconReturn", handleFaviconReturn);
-    this._mm.sendAsyncMessage("Reader:FaviconRequest", { url: this._article.url });
+    this._mm.sendAsyncMessage("Reader:FaviconRequest", {
+      url: this._article.url,
+      preferredWidth: 16 * this._win.devicePixelRatio
+    });
   },
 
   _loadFavicon(url, faviconUrl) {
     if (this._article.url !== url)
       return;
 
     let doc = this._doc;