Bug 1453580 - Remove promiseFaviconLinkUrl and fix its consumers. r=gijs draft
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 16 Apr 2018 18:24:06 +0200
changeset 784371 2fc932f26c329fb52a60c5ebb46b31fa549d4dd3
parent 783966 789e30ff2e3d6e1fcfce1a373c1e5635488d24da
push id106911
push usermak77@bonardo.net
push dateWed, 18 Apr 2018 13:45:39 +0000
reviewersgijs
bugs1453580
milestone61.0a1
Bug 1453580 - Remove promiseFaviconLinkUrl and fix its consumers. r=gijs MozReview-Commit-ID: GrY8s3l71Mp
browser/modules/ReaderParent.jsm
services/sync/modules/SyncedTabs.jsm
services/sync/tests/unit/test_syncedtabs.js
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/services/sync/tests/unit/test_syncedtabs.js
+++ b/services/sync/tests/unit/test_syncedtabs.js
@@ -187,18 +187,18 @@ add_task(async function test_clientWithT
     },
   });
 
   let clients = await SyncedTabs.getTabClients();
   equal(clients.length, 1);
   clients.sort((a, b) => { return a.name.localeCompare(b.name); });
   equal(clients[0].tabs.length, 1);
   equal(clients[0].tabs[0].url, "http://foo.com/");
-  // expect the default favicon (empty string) due to the pref being false.
-  equal(clients[0].tabs[0].icon, "");
+  // Expect the default favicon due to the pref being false.
+  equal(clients[0].tabs[0].icon, "page-icon:http://foo.com/");
   Services.prefs.clearUserPref("services.sync.syncedTabs.showRemoteIcons");
 });
 
 add_task(async function test_filter() {
   // Nothing matches.
   await configureClients({
     guid_desktop: {
       clientName: "My Desktop",
--- 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;