Backed out changeset 6e2a4ab6ed34 (bug 1255270) on a CLOSED TREE for leaking the world.
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 23 May 2016 17:32:56 +0100
changeset 323030 602500597e5f03371d43cf9ac9c3abac77342476
parent 323029 8c9c8720264d65050a97c6a9ba93b7914b96b8d5
child 323031 7aba31d799f29fe0747a8409fe62b6d113b0deb2
push id9671
push userraliiev@mozilla.com
push dateMon, 06 Jun 2016 20:27:52 +0000
treeherdermozilla-aurora@cea65ca3d0bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1255270
milestone49.0a1
backs out6e2a4ab6ed341f0250422d8f9ba9f4a206cfc527
Backed out changeset 6e2a4ab6ed34 (bug 1255270) on a CLOSED TREE for leaking the world. MozReview-Commit-ID: GlbH8KDcerY
browser/base/content/tabbrowser.xml
browser/components/feeds/FeedWriter.js
browser/components/places/PlacesUIUtils.jsm
toolkit/components/places/FaviconHelpers.cpp
toolkit/content/process-content.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -58,20 +58,27 @@
       <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="_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");
@@ -850,27 +857,32 @@
         <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) {
+            if (aURI && this.mFaviconService) {
               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();
-              PlacesUIUtils.loadFavicon(browser, loadingPrincipal, aURI);
+              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);
             }
 
             let sizedIconUrl = browser.mIconURL || "";
             if (sizedIconUrl != aTab.getAttribute("image")) {
               if (sizedIconUrl)
                 aTab.setAttribute("image", sizedIconUrl);
               else
                 aTab.removeAttribute("image");
@@ -933,19 +945,22 @@
           ]]>
         </body>
       </method>
 
       <method name="isFailedIcon">
         <parameter name="aURI"/>
         <body>
           <![CDATA[
-            if (!(aURI instanceof Ci.nsIURI))
-              aURI = makeURI(aURI);
-            return PlacesUtils.favicons.isFailedFavicon(aURI);
+            if (this.mFaviconService) {
+              if (!(aURI instanceof Ci.nsIURI))
+                aURI = makeURI(aURI);
+              return this.mFaviconService.isFailedFavicon(aURI);
+            }
+            return null;
           ]]>
         </body>
       </method>
 
       <method name="getWindowTitleForBrowser">
         <parameter name="aBrowser"/>
         <body>
           <![CDATA[
--- a/browser/components/feeds/FeedWriter.js
+++ b/browser/components/feeds/FeedWriter.js
@@ -209,16 +209,25 @@ 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;
@@ -1013,16 +1022,17 @@ 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;
   },
 
@@ -1153,16 +1163,62 @@ 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 docShell = this._window.QueryInterface(Ci.nsIInterfaceRequestor)
+                               .getInterface(Ci.nsIWebNavigation)
+                               .QueryInterface(Ci.nsIDocShell);
+    let usePrivateBrowsing = docShell.QueryInterface(Ci.nsILoadContext)
+                                     .usePrivateBrowsing;
+
+    // We probably need to call InheritFromDocShellToDoc for this, but right now
+    // we can't call it from JS.
+    let attrs = docShell.getOriginAttributes();
+    let ssm = Services.scriptSecurityManager;
+    let nullPrincipal = ssm.createNullPrincipal(attrs);
+
+    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,17 +7,16 @@ 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");
@@ -33,21 +32,16 @@ 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.
@@ -78,154 +72,16 @@ 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({request, uri, innerWindowID, timerID}, reason) {
-    try {
-      request.cancel();
-      clearTimeout(timerID);
-    } 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.request == this.request;
-          });
-          if (itemIndex != -1) {
-            let loadData = loadDataForWindow[itemIndex];
-            clearTimeout(loadData.timerID);
-            loadDataForWindow.splice(itemIndex, 1);
-          }
-        }
-      },
-    };
-  },
-
-  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, request};
-    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",
 
@@ -621,29 +477,16 @@ 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/FaviconHelpers.cpp
+++ b/toolkit/components/places/FaviconHelpers.cpp
@@ -506,19 +506,17 @@ AsyncFetchAndSetIconForPage::GetInterfac
 NS_IMETHODIMP
 AsyncFetchAndSetIconForPage::AsyncOnChannelRedirect(
   nsIChannel* oldChannel
 , nsIChannel* newChannel
 , uint32_t flags
 , nsIAsyncVerifyRedirectCallback *cb
 )
 {
-  // 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);
+  (void)cb->OnRedirectVerifyCallback(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
@@ -13,16 +13,8 @@ Cu.import("resource://gre/modules/Remote
 Cu.import("resource://gre/modules/Services.jsm");
 
 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
-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);