Bug 1119386 - Part 3: Use document's principal for favicons in browse…

…r (r=billm)
mozilla · Nov 24, 2015 · b0f3027 · b0f3027
1 parent 3c326a3
commit b0f3027
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 19 deletions.
diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
@@ -3418,7 +3418,7 @@ const DOMLinkHandler = {
         break;
 
       case "Link:SetIcon":
-        return this.setIcon(aMsg.target, aMsg.data.url);
+        return this.setIcon(aMsg.target, aMsg.data.url, aMsg.data.loadingPrincipal);
         break;
 
       case "Link:AddSearch":
@@ -3427,15 +3427,15 @@ const DOMLinkHandler = {
     }
   },
 
-  setIcon: function(aBrowser, aURL) {
+  setIcon: function(aBrowser, aURL, aLoadingPrincipal) {
     if (gBrowser.isFailedIcon(aURL))
       return false;
 
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
       return false;
 
-    gBrowser.setIcon(tab, aURL);
+    gBrowser.setIcon(tab, aURL, aLoadingPrincipal);
     return true;
   },
 

diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml
@@ -856,19 +856,28 @@
       <method name="setIcon">
         <parameter name="aTab"/>
         <parameter name="aURI"/>
+        <parameter name="aLoadingPrincipal"/>
         <body>
           <![CDATA[
-            var browser = this.getBrowserForTab(aTab);
+            let browser = this.getBrowserForTab(aTab);
             browser.mIconURL = aURI instanceof Ci.nsIURI ? aURI.spec : aURI;
 
             if (aURI && this.mFaviconService) {
-              if (!(aURI instanceof Ci.nsIURI))
+              if (!(aURI instanceof Ci.nsIURI)) {
                 aURI = makeURI(aURI);
-              this.mFaviconService.setAndFetchFaviconForPage(browser.currentURI,
-                                                             aURI, false,
-                                                             PrivateBrowsingUtils.isWindowPrivate(window) ?
-                                                               this.mFaviconService.FAVICON_LOAD_PRIVATE :
-                                                               this.mFaviconService.FAVICON_LOAD_NON_PRIVATE);
+              }
+              // 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);
             }
 
             let sizedIconUrl = browser.mIconURL || "";
@@ -932,7 +941,7 @@
               if (!this.isFailedIcon(url))
                 icon = url;
             }
-            this.setIcon(aTab, icon);
+            this.setIcon(aTab, icon, browser.contentPrincipal);
           ]]>
         </body>
       </method>
@@ -2522,7 +2531,7 @@
               // Workarounds for bug 458697
               // Icon might have been set on DOMLinkAdded, don't override that.
               if (!ourBrowser.mIconURL && otherBrowser.mIconURL)
-                this.setIcon(aOurTab, otherBrowser.mIconURL);
+                this.setIcon(aOurTab, otherBrowser.mIconURL, otherBrowser.contentPrincipal);
               var isBusy = aOtherTab.hasAttribute("busy");
               if (isBusy) {
                 aOurTab.setAttribute("busy", "true");
@@ -4444,7 +4453,7 @@
           }
 
           tab.removeAttribute("soundplaying");
-          this.setIcon(tab, icon);
+          this.setIcon(tab, icon, browser.contentPrincipal);
         ]]>
       </handler>
       <handler event="DOMAudioPlaybackStarted">

diff --git a/browser/base/content/test/general/browser_bug477014.js b/browser/base/content/test/general/browser_bug477014.js
@@ -10,7 +10,8 @@ add_task(function*() {
   let tabToDetach = gBrowser.addTab(testPage);
   yield waitForDocLoadComplete(tabToDetach.linkedBrowser);
 
-  gBrowser.setIcon(tabToDetach, iconURLSpec);
+  gBrowser.setIcon(tabToDetach, iconURLSpec,
+                   Services.scriptSecurityManager.getSystemPrincipal());
   tabToDetach.setAttribute("busy", "true");
 
   // detach and set the listener on the new window

diff --git a/browser/base/content/test/general/browser_discovery.js b/browser/base/content/test/general/browser_discovery.js
@@ -52,7 +52,8 @@ function runIconDiscoveryTest() {
 function iconDiscovery() {
   if (iconDiscoveryTests.length) {
     setHandlerFunc(runIconDiscoveryTest);
-    gBrowser.setIcon(gBrowser.selectedTab, null);
+    gBrowser.setIcon(gBrowser.selectedTab, null,
+                     Services.scriptSecurityManager.getSystemPrincipal());
 
     var test = iconDiscoveryTests[0];
     var head = doc().getElementById("linkparent");

diff --git a/browser/components/feeds/FeedWriter.js b/browser/components/feeds/FeedWriter.js
@@ -1303,6 +1303,8 @@ FeedWriter.prototype = {
                                          .QueryInterface(Ci.nsIDocShell)
                                          .QueryInterface(Ci.nsILoadContext)
                                          .usePrivateBrowsing;
+    var 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,
@@ -1312,7 +1314,7 @@ FeedWriter.prototype = {
                         btoa(String.fromCharCode.apply(null, aData));
           aMenuItem.setAttribute('image', dataURL);
         }
-      });
+      }, nullPrincipal);
   },
 
   get _mm() {

diff --git a/browser/components/sessionstore/SessionStore.jsm b/browser/components/sessionstore/SessionStore.jsm
@@ -784,7 +784,11 @@ var SessionStoreInternal = {
 
         // Restore the tab icon.
         if ("image" in tabData) {
-          win.gBrowser.setIcon(tab, tabData.image);
+          // Using null as the loadingPrincipal because serializing
+          // the principal would be overkill. Within SetIcon we
+          // default to the systemPrincipal if aLoadingPrincipal is
+          // null which will allow the favicon to load.
+          win.gBrowser.setIcon(tab, tabData.image, null);
           TabStateCache.update(browser, {image: null});
         }
 

diff --git a/browser/modules/ContentLinkHandler.jsm b/browser/modules/ContentLinkHandler.jsm
@@ -108,7 +108,9 @@ this.ContentLinkHandler = {
             }
             sizeHistogramTypes.add(sizesType);
 
-            [iconAdded] = chromeGlobal.sendSyncMessage("Link:SetIcon", {url: uri.spec});
+            [iconAdded] = chromeGlobal.sendSyncMessage(
+                            "Link:SetIcon",
+                            {url: uri.spec, loadingPrincipal: link.ownerDocument.nodePrincipal});
           }
           break;
         case "search":

diff --git a/extensions/cookie/test/browser_test_favicon.js b/extensions/cookie/test/browser_test_favicon.js
@@ -23,5 +23,6 @@ function test() {
   }, "cookie-rejected", false);
 
   // kick off a favicon load
-  gBrowser.setIcon(gBrowser.selectedTab, "http://example.org/tests/extensions/cookie/test/damonbowling.jpg");
+  gBrowser.setIcon(gBrowser.selectedTab, "http://example.org/tests/extensions/cookie/test/damonbowling.jpg",
+                   Services.scriptSecurityManager.getSystemPrincipal());
 }