Backed out changeset 897bbf35e781 (bug 1453580) for XPCShell failure on services/sync/tests/unit/test_syncedtabs.js. CLOSED TREE
authorDorel Luca <dluca@mozilla.com>
Wed, 18 Apr 2018 14:06:34 +0300
changeset 467782 09c4cd5c8946259f133b10c0844380712c02c912
parent 467781 40b4fba437ca6e5c136c5bafa22e13ae07427c81
child 467783 fdc9671b51b59f5f06a591e90110892bffe98325
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1453580
milestone61.0a1
backs out897bbf35e781fc7067b7e358379fad54d5342a4f
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 897bbf35e781 (bug 1453580) for XPCShell failure on services/sync/tests/unit/test_syncedtabs.js. CLOSED TREE
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,31 +17,26 @@ 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) {
-          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);
-          }
+          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);
         }
         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,20 +48,22 @@ 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) {
-      // 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;
+      try {
+        icon = (await PlacesUtils.promiseFaviconLinkUrl(url)).spec;
+      } catch (ex) { /* no favicon avaiable */ }
+    }
+    if (!icon) {
+      icon = "";
     }
     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,30 +113,52 @@ 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];
 
-    return PlacesUtils.urlWithSizeRef(window, "page-icon:" + signon.hostname, 16);
+    // 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 "";
   },
   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,16 +1523,39 @@ 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,20 +698,17 @@ 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,
-      preferredWidth: 16 * this._win.devicePixelRatio
-    });
+    this._mm.sendAsyncMessage("Reader:FaviconRequest", { url: this._article.url });
   },
 
   _loadFavicon(url, faviconUrl) {
     if (this._article.url !== url)
       return;
 
     let doc = this._doc;