Bug 1255270, r=mak a=ritu
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 23 May 2016 10:09:13 +0100
changeset 378919 cbabb4769b50dc1b9c56f96141e741db6b89b0b8
parent 378918 583f17047dd780cf9a15b2620f4e52cb870e6003
child 378920 46eb9f7807f371cf7205fbdee59fef6da41157cb
push id21011
push usermak77@bonardo.net
push dateThu, 16 Jun 2016 13:40:45 +0000
reviewersmak, ritu
bugs1255270
milestone47.0
Bug 1255270, r=mak a=ritu ba=IDon'tThinkThisHookIsSupposedToBeEnforcedOnBeta47 MozReview-Commit-ID: 5fInAZiZMhl
browser/base/content/tabbrowser.xml
browser/components/feeds/FeedWriter.js
browser/components/places/PlacesUIUtils.jsm
toolkit/components/places/AsyncFaviconHelpers.cpp
toolkit/content/process-content.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -58,31 +58,24 @@
       <field name="closingTabsEnum" readonly="true">({ ALL: 0, OTHER: 1, TO_END: 2 });</field>
 
       <field name="_visibleTabs">null</field>
 
       <field name="mURIFixup" readonly="true">
         Components.classes["@mozilla.org/docshell/urifixup;1"]
                   .getService(Components.interfaces.nsIURIFixup);
       </field>
-      <field name="mFaviconService" readonly="true">
-        Components.classes["@mozilla.org/browser/favicon-service;1"]
-                  .getService(Components.interfaces.nsIFaviconService);
-      </field>
       <field name="_placesAutocomplete" readonly="true">
          Components.classes["@mozilla.org/autocomplete/search;1?name=history"]
                    .getService(Components.interfaces.mozIPlacesAutoComplete);
       </field>
       <field name="_unifiedComplete" readonly="true">
          Components.classes["@mozilla.org/autocomplete/search;1?name=unifiedcomplete"]
                    .getService(Components.interfaces.mozIPlacesAutoComplete);
       </field>
-      <field name="PlacesUtils" readonly="true">
-        (Components.utils.import("resource://gre/modules/PlacesUtils.jsm", {})).PlacesUtils;
-      </field>
       <field name="AppConstants" readonly="true">
         (Components.utils.import("resource://gre/modules/AppConstants.jsm", {})).AppConstants;
       </field>
       <field name="mTabBox" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "tabbox");
       </field>
       <field name="mPanelContainer" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "panelcontainer");
@@ -861,32 +854,27 @@
         <parameter name="aTab"/>
         <parameter name="aURI"/>
         <parameter name="aLoadingPrincipal"/>
         <body>
           <![CDATA[
             let browser = this.getBrowserForTab(aTab);
             browser.mIconURL = aURI instanceof Ci.nsIURI ? aURI.spec : aURI;
 
-            if (aURI && this.mFaviconService) {
+            if (aURI) {
               if (!(aURI instanceof Ci.nsIURI)) {
                 aURI = makeURI(aURI);
               }
               // We do not serialize the principal from within SessionStore.jsm,
               // hence if aLoadingPrincipal is null we default to the
               // systemPrincipal which will allow the favicon to load.
               let loadingPrincipal = aLoadingPrincipal
                 ? aLoadingPrincipal
                 : Services.scriptSecurityManager.getSystemPrincipal();
-              let loadType = PrivateBrowsingUtils.isWindowPrivate(window)
-                ? this.mFaviconService.FAVICON_LOAD_PRIVATE
-                : this.mFaviconService.FAVICON_LOAD_NON_PRIVATE;
-
-              this.mFaviconService.setAndFetchFaviconForPage(
-                browser.currentURI, aURI, false, loadType, null, loadingPrincipal);
+              PlacesUIUtils.loadFavicon(browser, loadingPrincipal, aURI);
             }
 
             let sizedIconUrl = browser.mIconURL || "";
             if (sizedIconUrl != aTab.getAttribute("image")) {
               if (sizedIconUrl)
                 aTab.setAttribute("image", sizedIconUrl);
               else
                 aTab.removeAttribute("image");
@@ -949,22 +937,19 @@
           ]]>
         </body>
       </method>
 
       <method name="isFailedIcon">
         <parameter name="aURI"/>
         <body>
           <![CDATA[
-            if (this.mFaviconService) {
-              if (!(aURI instanceof Ci.nsIURI))
-                aURI = makeURI(aURI);
-              return this.mFaviconService.isFailedFavicon(aURI);
-            }
-            return null;
+            if (!(aURI instanceof Ci.nsIURI))
+              aURI = makeURI(aURI);
+            return PlacesUtils.favicons.isFailedFavicon(aURI);
           ]]>
         </body>
       </method>
 
       <method name="getWindowTitleForBrowser">
         <parameter name="aBrowser"/>
         <body>
           <![CDATA[
--- a/browser/components/feeds/FeedWriter.js
+++ b/browser/components/feeds/FeedWriter.js
@@ -209,25 +209,16 @@ FeedWriter.prototype = {
     catch (e) {
       // Not allowed to load this link because secman.checkLoadURIStr threw
       return;
     }
 
     element.setAttribute(attribute, uri);
   },
 
-  __faviconService: null,
-  get _faviconService() {
-    if (!this.__faviconService)
-      this.__faviconService = Cc["@mozilla.org/browser/favicon-service;1"].
-                              getService(Ci.nsIFaviconService);
-
-    return this.__faviconService;
-  },
-
   __bundle: null,
   get _bundle() {
     if (!this.__bundle) {
       this.__bundle = Cc["@mozilla.org/intl/stringbundle;1"].
                       getService(Ci.nsIStringBundleService).
                       createBundle(URI_BUNDLE);
     }
     return this.__bundle;
@@ -1018,17 +1009,16 @@ FeedWriter.prototype = {
     prefs.removeObserver(PREF_VIDEO_SELECTED_APP, this);
 
     prefs.removeObserver(PREF_AUDIO_SELECTED_ACTION, this);
     prefs.removeObserver(PREF_AUDIO_SELECTED_READER, this);
     prefs.removeObserver(PREF_AUDIO_SELECTED_WEB, this);
     prefs.removeObserver(PREF_AUDIO_SELECTED_APP, this);
 
     this._removeFeedFromCache();
-    this.__faviconService = null;
     this.__bundle = null;
     this._feedURI = null;
 
     this._selectedApp = undefined;
     this._selectedAppMenuItem = null;
     this._defaultHandlerMenuItem = null;
   },
 
@@ -1159,56 +1149,16 @@ FeedWriter.prototype = {
         case PREF_SELECTED_ACTION:
         case PREF_VIDEO_SELECTED_ACTION:
         case PREF_AUDIO_SELECTED_ACTION:
           this._setAlwaysUseCheckedState(feedType);
       }
     }
   },
 
-  /**
-   * Sets the icon for the given web-reader item in the readers menu.
-   * The icon is fetched and stored through the favicon service.
-   *
-   * @param aReaderUrl
-   *        the reader url.
-   * @param aMenuItem
-   *        the reader item in the readers menulist.
-   *
-   * @note For privacy reasons we cannot set the image attribute directly
-   *       to the icon url.  See Bug 358878 for details.
-   */
-  _setFaviconForWebReader:
-  function FW__setFaviconForWebReader(aReaderUrl, aMenuItem) {
-    let readerURI = makeURI(aReaderUrl);
-    if (!/^https?$/.test(readerURI.scheme)) {
-      // Don't try to get a favicon for non http(s) URIs.
-      return;
-    }
-    let faviconURI = makeURI(readerURI.prePath + "/favicon.ico");
-    let self = this;
-    let usePrivateBrowsing = this._window.QueryInterface(Ci.nsIInterfaceRequestor)
-                                         .getInterface(Ci.nsIWebNavigation)
-                                         .QueryInterface(Ci.nsIDocShell)
-                                         .QueryInterface(Ci.nsILoadContext)
-                                         .usePrivateBrowsing;
-    let nullPrincipal = Cc["@mozilla.org/nullprincipal;1"]
-                          .createInstance(Ci.nsIPrincipal);
-    this._faviconService.setAndFetchFaviconForPage(readerURI, faviconURI, false,
-      usePrivateBrowsing ? this._faviconService.FAVICON_LOAD_PRIVATE
-                         : this._faviconService.FAVICON_LOAD_NON_PRIVATE,
-      function (aURI, aDataLen, aData, aMimeType) {
-        if (aDataLen > 0) {
-          let dataURL = "data:" + aMimeType + ";base64," +
-                        btoa(String.fromCharCode.apply(null, aData));
-          aMenuItem.setAttribute('image', dataURL);
-        }
-      }, nullPrincipal);
-  },
-
   get _mm() {
     let mm = this._window.QueryInterface(Ci.nsIInterfaceRequestor).
                           getInterface(Ci.nsIDocShell).
                           QueryInterface(Ci.nsIInterfaceRequestor).
                           getInterface(Ci.nsIContentFrameMessageManager);
     delete this._mm;
     return this._mm = mm;
   },
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -7,16 +7,17 @@ this.EXPORTED_SYMBOLS = ["PlacesUIUtils"
 
 var Ci = Components.interfaces;
 var Cc = Components.classes;
 var Cr = Components.results;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/Timer.jsm");
 
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
                                   "resource://gre/modules/PluralForm.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
@@ -32,16 +33,21 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/PlacesTransactions.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "CloudSync",
                                   "resource://gre/modules/CloudSync.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Weave",
                                   "resource://services-sync/main.js");
 
+const gInContentProcess = Services.appinfo.processType == Ci.nsIXULRuntime.PROCESS_TYPE_CONTENT;
+const FAVICON_REQUEST_TIMEOUT = 60 * 1000;
+// Map from windows to arrays of data about pending favicon loads.
+let gFaviconLoadDataMap = new Map();
+
 // copied from utilityOverlay.js
 const TAB_DROP_TYPE = "application/x-moz-tabbrowser-tab";
 
 // This function isn't public both because it's synchronous and because it is
 // going to be removed in bug 1072833.
 function IsLivemark(aItemId) {
   // Since this check may be done on each dragover event, it's worth maintaining
   // a cache.
@@ -72,16 +78,159 @@ function IsLivemark(aItemId) {
     PlacesUtils.annotations.addObserver(obs);
     PlacesUtils.registerShutdownFunction(() => {
       PlacesUtils.annotations.removeObserver(obs);
     });
   }
   return self.ids.has(aItemId);
 }
 
+let InternalFaviconLoader = {
+  /**
+   * This gets called for every inner window that is destroyed.
+   * In the parent process, we process the destruction ourselves. In the child process,
+   * we notify the parent which will then process it based on that message.
+   */
+  observe(subject, topic, data) {
+    let innerWindowID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
+    this.onInnerDestroyed(innerWindowID);
+  },
+
+  /**
+   * Actually cancel the request, and clear the timeout for cancelling it.
+   */
+  _cancelRequest({uri, innerWindowID, timerID, callback}, reason) {
+    // Break cycle
+    let request = callback.request;
+    delete callback.request;
+    // Ensure we don't time out.
+    clearTimeout(timerID);
+    try {
+      request.cancel();
+    } catch (ex) {
+      Cu.reportError("When cancelling a request for " + uri.spec + " because " + reason + ", it was already canceled!");
+    }
+  },
+
+  /**
+   * Called for every inner that gets destroyed, only in the parent process.
+   */
+  onInnerDestroyed(innerID) {
+    for (let [window, loadDataForWindow] of gFaviconLoadDataMap) {
+      let newLoadDataForWindow = loadDataForWindow.filter(loadData => {
+        let innerWasDestroyed = loadData.innerWindowID == innerID;
+        if (innerWasDestroyed) {
+          this._cancelRequest(loadData, "the inner window was destroyed");
+        }
+        // Keep the items whose inner is still alive.
+        return !innerWasDestroyed;
+      });
+      // Map iteration with for...of is safe against modification, so
+      // now just replace the old value:
+      gFaviconLoadDataMap.set(window, newLoadDataForWindow);
+    }
+  },
+
+  /**
+   * Called when a toplevel chrome window unloads. We use this to tidy up after ourselves,
+   * avoid leaks, and cancel any remaining requests. The last part should in theory be
+   * handled by the inner-window-destroyed handlers. We clean up just to be on the safe side.
+   */
+  onUnload(win) {
+    let loadDataForWindow = gFaviconLoadDataMap.get(win);
+    if (loadDataForWindow) {
+      for (let loadData of loadDataForWindow) {
+        this._cancelRequest(loadData, "the chrome window went away");
+      }
+    }
+    gFaviconLoadDataMap.delete(win);
+  },
+
+  /**
+   * 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);
+          }
+        }
+        delete this.request;
+      },
+    };
+  },
+
+  ensureInitialized() {
+    if (this._initialized) {
+      return;
+    }
+    this._initialized = true;
+
+    Services.obs.addObserver(this, "inner-window-destroyed", false);
+    Services.ppmm.addMessageListener("Toolkit:inner-window-destroyed", msg => {
+      this.onInnerDestroyed(msg.data);
+    });
+  },
+
+  loadFavicon(browser, principal, uri) {
+    this.ensureInitialized();
+    let win = browser.ownerDocument.defaultView;
+    if (!gFaviconLoadDataMap.has(win)) {
+      gFaviconLoadDataMap.set(win, []);
+      let unloadHandler = event => {
+        let doc = event.target;
+        let eventWin = doc.defaultview;
+        if (win == win.top && doc.documentURI != "about:blank") {
+          win.removeEventListener("unload", unloadHandler);
+          this.onUnload(win);
+        }
+      };
+      win.addEventListener("unload", unloadHandler, true);
+    }
+
+    // First we do the actual setAndFetch call:
+    let {innerWindowID, currentURI} = browser;
+    let loadType = PrivateBrowsingUtils.isWindowPrivate(win)
+      ? PlacesUtils.favicons.FAVICON_LOAD_PRIVATE
+      : PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE;
+    let callback = this._makeCompletionCallback(win, innerWindowID);
+    let request = PlacesUtils.favicons.setAndFetchFaviconForPage(currentURI, uri, false,
+                                                                 loadType, callback, principal);
+
+    // Now register the result so we can cancel it if/when necessary.
+    if (!request) {
+      // The favicon service can return with success but no-op (and leave request
+      // 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");
+    }, FAVICON_REQUEST_TIMEOUT);
+    let loadDataForWindow = gFaviconLoadDataMap.get(win);
+    loadDataForWindow.push(loadData);
+  },
+};
+
 this.PlacesUIUtils = {
   ORGANIZER_LEFTPANE_VERSION: 7,
   ORGANIZER_FOLDER_ANNO: "PlacesOrganizer/OrganizerFolder",
   ORGANIZER_QUERY_ANNO: "PlacesOrganizer/OrganizerQuery",
 
   LOAD_IN_SIDEBAR_ANNO: "bookmarkProperties/loadInSidebar",
   DESCRIPTION_ANNO: "bookmarkProperties/description",
 
@@ -477,16 +626,29 @@ this.PlacesUIUtils = {
     return ("performed" in aInfo && aInfo.performed);
   },
 
   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
     return RecentWindow.getMostRecentBrowserWindow();
   },
 
   /**
+   * set and fetch a favicon. Can only be used from the parent process.
+   * @param browser   {Browser}   The XUL browser element for which we're fetching a favicon.
+   * @param principal {Principal} The loading principal to use for the fetch.
+   * @param uri       {URI}       The URI to fetch.
+   */
+  loadFavicon(browser, principal, uri) {
+    if (gInContentProcess) {
+      throw new Error("Can't track loads from within the child process!");
+    }
+    InternalFaviconLoader.loadFavicon(browser, principal, uri);
+  },
+
+  /**
    * Returns the closet ancestor places view for the given DOM node
    * @param aNode
    *        a DOM node
    * @return the closet ancestor places view if exists, null otherwsie.
    */
   getViewForNode: function PUIU_getViewForNode(aNode) {
     let node = aNode;
 
--- a/toolkit/components/places/AsyncFaviconHelpers.cpp
+++ b/toolkit/components/places/AsyncFaviconHelpers.cpp
@@ -600,17 +600,19 @@ AsyncFetchAndSetIconForPage::GetInterfac
 NS_IMETHODIMP
 AsyncFetchAndSetIconForPage::AsyncOnChannelRedirect(
   nsIChannel* oldChannel
 , nsIChannel* newChannel
 , uint32_t flags
 , nsIAsyncVerifyRedirectCallback *cb
 )
 {
-  (void)cb->OnRedirectVerifyCallback(NS_OK);
+  // If we've been canceled, stop the redirect with NS_BINDING_ABORTED, and
+  // handle the cancel on the original channel.
+  (void)cb->OnRedirectVerifyCallback(mCanceled ? NS_BINDING_ABORTED : NS_OK);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 AsyncFetchAndSetIconForPage::OnStopRequest(nsIRequest* aRequest,
                                            nsISupports* aContext,
                                            nsresult aStatusCode)
 {
--- a/toolkit/content/process-content.js
+++ b/toolkit/content/process-content.js
@@ -7,14 +7,26 @@
 var { classes: Cc, interfaces: Ci, utils: Cu } = Components;
 
 // Creates a new PageListener for this process. This will listen for page loads
 // and for those that match URLs provided by the parent process will set up
 // a dedicated message port and notify the parent process.
 Cu.import("resource://gre/modules/RemotePageManager.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
+const gInContentProcess = Services.appinfo.processType == Ci.nsIXULRuntime.PROCESS_TYPE_CONTENT;
+
 Services.cpmm.addMessageListener("gmp-plugin-crash", msg => {
   let gmpservice = Cc["@mozilla.org/gecko-media-plugin-service;1"]
                      .getService(Ci.mozIGeckoMediaPluginService);
 
   gmpservice.RunPluginCrashCallbacks(msg.data.pluginID, msg.data.pluginName);
 });
+
+// Forward inner-window-destroyed notifications with the inner window ID,
+// so that code in the parent that should do something when content
+// windows go away can do it
+if (gInContentProcess) {
+  Services.obs.addObserver((subject, topic, data) => {
+    let innerWindowID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
+    Services.cpmm.sendAsyncMessage("Toolkit:inner-window-destroyed", innerWindowID);
+  }, "inner-window-destroyed", false);
+}