Bug 1279650 - kill off useless warning for timed out favicon requests that have already been cancelled when the window closes, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 17 Aug 2016 18:34:50 +0100
changeset 402650 ffd59bf34cf232a94fd69d98f815ff9777320c29
parent 402649 a6f67ab4a4ec4dd66f53c4b4423e5b8b63023388
child 528727 49d000599530488f36763428e0d3ec364b1a1100
push id26717
push usergijskruitbosch@gmail.com
push dateThu, 18 Aug 2016 15:14:18 +0000
reviewersmikedeboer
bugs1279650
milestone51.0a1
Bug 1279650 - kill off useless warning for timed out favicon requests that have already been cancelled when the window closes, r?mikedeboer As it is, when favicon requests time out, the timeout handler doesn't remove the loadData and request from gFaviconLoadDataMap after cancelling the request, which means that when the tab or the parent window is closed we try and cancel the same thing again, which leads to errors in the console. This patch addresses that issue by making timeouts also remove the loadData from gFaviconLoadDataMap. MozReview-Commit-ID: FCeRFXjeXdJ
browser/components/places/PlacesUIUtils.jsm
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -140,37 +140,60 @@ let InternalFaviconLoader = {
       for (let loadData of loadDataForWindow) {
         this._cancelRequest(loadData, "the chrome window went away");
       }
     }
     gFaviconLoadDataMap.delete(win);
   },
 
   /**
+   * Remove a particular favicon load's loading data from our map tracking
+   * load data per chrome window.
+   *
+   * @param win
+   *        the chrome window in which we should look for this load
+   * @param filterData ({innerWindowID, uri, callback})
+   *        the data we should use to find this particular load to remove.
+   *
+   * @return the loadData object we removed, or null if we didn't find any.
+   */
+  _removeLoadDataFromWindowMap(win, {innerWindowID, uri, callback}) {
+    let loadDataForWindow = gFaviconLoadDataMap.get(win);
+    if (loadDataForWindow) {
+      let itemIndex = loadDataForWindow.findIndex(loadData => {
+        return loadData.innerWindowID == innerWindowID &&
+               loadData.uri.equals(uri) &&
+               loadData.callback.request == callback.request;
+      });
+      if (itemIndex != -1) {
+        let loadData = loadDataForWindow[itemIndex];
+        loadDataForWindow.splice(itemIndex, 1);
+        return loadData;
+      }
+    }
+    return null;
+  },
+
+  /**
    * Create a function to use as a nsIFaviconDataCallback, so we can remove cancelling
    * information when the request succeeds. Note that right now there are some edge-cases,
    * such as about: URIs with chrome:// favicons where the success callback is not invoked.
    * This is OK: we will 'cancel' the request after the timeout (or when the window goes
    * away) but that will be a no-op in such cases.
    */
   _makeCompletionCallback(win, id) {
     return {
       onComplete(uri) {
-        let loadDataForWindow = gFaviconLoadDataMap.get(win);
-        if (loadDataForWindow) {
-          let itemIndex = loadDataForWindow.findIndex(loadData => {
-            return loadData.innerWindowID == id &&
-                   loadData.uri.equals(uri) &&
-                   loadData.callback.request == this.request;
-          });
-          if (itemIndex != -1) {
-            let loadData = loadDataForWindow[itemIndex];
-            clearTimeout(loadData.timerID);
-            loadDataForWindow.splice(itemIndex, 1);
-          }
+        let loadData = InternalFaviconLoader._removeLoadDataFromWindowMap(win, {
+          uri,
+          innerWindowID: id,
+          callback: this,
+        });
+        if (loadData) {
+          clearTimeout(loadData.timerID);
         }
         delete this.request;
       },
     };
   },
 
   ensureInitialized() {
     if (this._initialized) {
@@ -219,16 +242,17 @@ let InternalFaviconLoader = {
       // as null) if the icon is the same as the page (e.g. for images) or if it is
       // the favicon for an error page. In this case, we do not need to do anything else.
       return;
     }
     callback.request = request;
     let loadData = {innerWindowID, uri, callback};
     loadData.timerID = setTimeout(() => {
       this._cancelRequest(loadData, "it timed out");
+      this._removeLoadDataFromWindowMap(win, loadData);
     }, FAVICON_REQUEST_TIMEOUT);
     let loadDataForWindow = gFaviconLoadDataMap.get(win);
     loadDataForWindow.push(loadData);
   },
 };
 
 this.PlacesUIUtils = {
   ORGANIZER_LEFTPANE_VERSION: 7,