Bug 1270680 - Part 1: Double-key the image cache by origin attribute. r=jdm,seth
authorJonathan Hao <jhao@mozilla.com>
Wed, 01 Jun 2016 23:07:00 +0200
changeset 300802 d08891438e5835be6bdf3bbb6de932c845a73604
parent 300801 941fd40d73de36e08efb0590a1ad1900602d1d37
child 300803 20ab9d3966135fb8e3b37660a8c85bf394ebdbda
push id30322
push userdmitchell@mozilla.com
push dateTue, 07 Jun 2016 14:47:17 +0000
treeherdermozilla-central@ec20b463c04f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm, seth
bugs1270680
milestone50.0a1
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
Bug 1270680 - Part 1: Double-key the image cache by origin attribute. r=jdm,seth
embedding/browser/nsContextMenuInfo.cpp
image/ImageCacheKey.cpp
image/ImageCacheKey.h
image/imgLoader.cpp
image/test/unit/async_load_tests.js
image/test/unit/test_private_channel.js
layout/generic/nsImageFrame.cpp
toolkit/system/gnome/nsAlertsIconListener.cpp
--- a/embedding/browser/nsContextMenuInfo.cpp
+++ b/embedding/browser/nsContextMenuInfo.cpp
@@ -266,17 +266,18 @@ nsContextMenuInfo::GetBackgroundImageReq
   auto* piWindow = nsPIDOMWindowOuter::From(window);
   nsPIDOMWindowInner* innerWindow = piWindow->GetCurrentInnerWindow();
   MOZ_ASSERT(innerWindow);
 
   nsCOMPtr<nsIDOMCSSPrimitiveValue> primitiveValue;
   nsAutoString bgStringValue;
 
   nsCOMPtr<nsIDocument> doc(do_QueryInterface(document));
-  nsCOMPtr<nsIPrincipal> principal = doc ? doc->NodePrincipal() : nullptr;
+  NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
+  nsCOMPtr<nsIPrincipal> principal = doc->NodePrincipal();
 
   while (true) {
     nsCOMPtr<Element> domElement(do_QueryInterface(domNode));
     // bail for the parent node of the root element or null argument
     if (!domElement) {
       break;
     }
 
--- a/image/ImageCacheKey.cpp
+++ b/image/ImageCacheKey.cpp
@@ -40,67 +40,79 @@ BlobSerial(ImageURL* aURI)
   if (NS_SUCCEEDED(NS_GetBlobForBlobURISpec(spec, getter_AddRefs(blob))) &&
       blob) {
     return Some(blob->GetSerialNumber());
   }
 
   return Nothing();
 }
 
-ImageCacheKey::ImageCacheKey(nsIURI* aURI, nsIDocument* aDocument)
+ImageCacheKey::ImageCacheKey(nsIURI* aURI,
+                             const PrincipalOriginAttributes& aAttrs,
+                             nsIDocument* aDocument)
   : mURI(new ImageURL(aURI))
+  , mOriginAttributes(aAttrs)
   , mControlledDocument(GetControlledDocumentToken(aDocument))
   , mIsChrome(URISchemeIs(mURI, "chrome"))
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (URISchemeIs(mURI, "blob")) {
     mBlobSerial = BlobSerial(mURI);
   }
 
-  mHash = ComputeHash(mURI, mBlobSerial, mControlledDocument);
+  mHash = ComputeHash(mURI, mBlobSerial, mOriginAttributes, mControlledDocument);
 }
 
-ImageCacheKey::ImageCacheKey(ImageURL* aURI, nsIDocument* aDocument)
+ImageCacheKey::ImageCacheKey(ImageURL* aURI,
+                             const PrincipalOriginAttributes& aAttrs,
+                             nsIDocument* aDocument)
   : mURI(aURI)
+  , mOriginAttributes(aAttrs)
   , mControlledDocument(GetControlledDocumentToken(aDocument))
   , mIsChrome(URISchemeIs(mURI, "chrome"))
 {
   MOZ_ASSERT(aURI);
 
   if (URISchemeIs(mURI, "blob")) {
     mBlobSerial = BlobSerial(mURI);
   }
 
-  mHash = ComputeHash(mURI, mBlobSerial, mControlledDocument);
+  mHash = ComputeHash(mURI, mBlobSerial, mOriginAttributes, mControlledDocument);
 }
 
 ImageCacheKey::ImageCacheKey(const ImageCacheKey& aOther)
   : mURI(aOther.mURI)
   , mBlobSerial(aOther.mBlobSerial)
+  , mOriginAttributes(aOther.mOriginAttributes)
   , mControlledDocument(aOther.mControlledDocument)
   , mHash(aOther.mHash)
   , mIsChrome(aOther.mIsChrome)
 { }
 
 ImageCacheKey::ImageCacheKey(ImageCacheKey&& aOther)
   : mURI(Move(aOther.mURI))
   , mBlobSerial(Move(aOther.mBlobSerial))
+  , mOriginAttributes(aOther.mOriginAttributes)
   , mControlledDocument(aOther.mControlledDocument)
   , mHash(aOther.mHash)
   , mIsChrome(aOther.mIsChrome)
 { }
 
 bool
 ImageCacheKey::operator==(const ImageCacheKey& aOther) const
 {
   // Don't share the image cache between a controlled document and anything else.
   if (mControlledDocument != aOther.mControlledDocument) {
     return false;
   }
+  // The origin attributes always have to match.
+  if (mOriginAttributes != aOther.mOriginAttributes) {
+    return false;
+  }
   if (mBlobSerial || aOther.mBlobSerial) {
     // If at least one of us has a blob serial, just compare the blob serial and
     // the ref portion of the URIs.
     return mBlobSerial == aOther.mBlobSerial &&
            mURI->HasSameRef(*aOther.mURI);
   }
 
   // For non-blob URIs, compare the URIs.
@@ -111,37 +123,41 @@ const char*
 ImageCacheKey::Spec() const
 {
   return mURI->Spec();
 }
 
 /* static */ uint32_t
 ImageCacheKey::ComputeHash(ImageURL* aURI,
                            const Maybe<uint64_t>& aBlobSerial,
+                           const PrincipalOriginAttributes& aAttrs,
                            void* aControlledDocument)
 {
   // Since we frequently call Hash() several times in a row on the same
   // ImageCacheKey, as an optimization we compute our hash once and store it.
 
   nsPrintfCString ptr("%p", aControlledDocument);
+  nsAutoCString suffix;
+  aAttrs.CreateSuffix(suffix);
+
   if (aBlobSerial) {
     // For blob URIs, we hash the serial number of the underlying blob, so that
     // different blob URIs which point to the same blob share a cache entry. We
     // also include the ref portion of the URI to support -moz-samplesize, which
     // requires us to create different Image objects even if the source data is
     // the same.
     nsAutoCString ref;
     aURI->GetRef(ref);
-    return HashGeneric(*aBlobSerial, HashString(ref + ptr));
+    return HashGeneric(*aBlobSerial, HashString(ref + suffix + ptr));
   }
 
   // For non-blob URIs, we hash the URI spec.
   nsAutoCString spec;
   aURI->GetSpec(spec);
-  return HashString(spec + ptr);
+  return HashString(spec + suffix + ptr);
 }
 
 /* static */ void*
 ImageCacheKey::GetControlledDocumentToken(nsIDocument* aDocument)
 {
   // For non-controlled documents, we just return null.  For controlled
   // documents, we cast the pointer into a void* to avoid dereferencing
   // it (since we only use it for comparisons), and return it.
--- a/image/ImageCacheKey.h
+++ b/image/ImageCacheKey.h
@@ -5,16 +5,17 @@
 
 /**
  * ImageCacheKey is the key type for the image cache (see imgLoader.h).
  */
 
 #ifndef mozilla_image_src_ImageCacheKey_h
 #define mozilla_image_src_ImageCacheKey_h
 
+#include "mozilla/BasePrincipal.h"
 #include "mozilla/Maybe.h"
 
 class nsIDocument;
 class nsIURI;
 
 namespace mozilla {
 namespace image {
 
@@ -26,18 +27,20 @@ class ImageURL;
  * We key the cache on the initial URI (before any redirects), with some
  * canonicalization applied. See ComputeHash() for the details.
  * Controlled documents do not share their cache entries with
  * non-controlled documents, or other controlled documents.
  */
 class ImageCacheKey final
 {
 public:
-  ImageCacheKey(nsIURI* aURI, nsIDocument* aDocument);
-  ImageCacheKey(ImageURL* aURI, nsIDocument* aDocument);
+  ImageCacheKey(nsIURI* aURI, const PrincipalOriginAttributes& aAttrs,
+                nsIDocument* aDocument);
+  ImageCacheKey(ImageURL* aURI, const PrincipalOriginAttributes& aAttrs,
+                nsIDocument* aDocument);
 
   ImageCacheKey(const ImageCacheKey& aOther);
   ImageCacheKey(ImageCacheKey&& aOther);
 
   bool operator==(const ImageCacheKey& aOther) const;
   uint32_t Hash() const { return mHash; }
 
   /// A weak pointer to the URI spec for this cache entry. For logging only.
@@ -48,21 +51,23 @@ public:
 
   /// A token indicating which service worker controlled document this entry
   /// belongs to, if any.
   void* ControlledDocument() const { return mControlledDocument; }
 
 private:
   static uint32_t ComputeHash(ImageURL* aURI,
                               const Maybe<uint64_t>& aBlobSerial,
+                              const PrincipalOriginAttributes& aAttrs,
                               void* aControlledDocument);
   static void* GetControlledDocumentToken(nsIDocument* aDocument);
 
   RefPtr<ImageURL> mURI;
   Maybe<uint64_t> mBlobSerial;
+  PrincipalOriginAttributes mOriginAttributes;
   void* mControlledDocument;
   uint32_t mHash;
   bool mIsChrome;
 };
 
 } // namespace image
 } // namespace mozilla
 
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -1354,17 +1354,30 @@ imgLoader::ClearCache(bool chrome)
 NS_IMETHODIMP
 imgLoader::FindEntryProperties(nsIURI* uri,
                                nsIDOMDocument* aDOMDoc,
                                nsIProperties** _retval)
 {
   *_retval = nullptr;
 
   nsCOMPtr<nsIDocument> doc = do_QueryInterface(aDOMDoc);
-  ImageCacheKey key(uri, doc);
+  nsCOMPtr<nsIPrincipal> principal;
+  if (doc) {
+    principal = doc->NodePrincipal();
+  } else {
+    nsCOMPtr<nsIScriptSecurityManager> ssm = nsContentUtils::GetSecurityManager();
+    NS_ENSURE_TRUE(ssm, NS_ERROR_FAILURE);
+    ssm->GetSystemPrincipal(getter_AddRefs(principal));
+  }
+  NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE);
+
+  const PrincipalOriginAttributes& attrs =
+    BasePrincipal::Cast(principal)->OriginAttributesRef();
+
+  ImageCacheKey key(uri, attrs, doc);
   imgCacheTable& cache = GetCache(key);
 
   RefPtr<imgCacheEntry> entry;
   if (cache.Get(key, getter_AddRefs(entry)) && entry) {
     if (mCacheTracker && entry->HasNoProxies()) {
       mCacheTracker->MarkUsed(entry);
     }
 
@@ -2111,17 +2124,21 @@ imgLoader::LoadImage(nsIURI* aURI,
   }
 
   RefPtr<imgCacheEntry> entry;
 
   // Look in the cache for our URI, and then validate it.
   // XXX For now ignore aCacheKey. We will need it in the future
   // for correctly dealing with image load requests that are a result
   // of post data.
-  ImageCacheKey key(aURI, aLoadingDocument);
+  NS_ENSURE_TRUE(aLoadingPrincipal, NS_ERROR_FAILURE);
+  const PrincipalOriginAttributes& attrs =
+    BasePrincipal::Cast(aLoadingPrincipal)->OriginAttributesRef();
+
+  ImageCacheKey key(aURI, attrs, aLoadingDocument);
   imgCacheTable& cache = GetCache(key);
 
   if (cache.Get(key, getter_AddRefs(entry)) && entry) {
     if (ValidateEntry(entry, aURI, aInitialDocumentURI, aReferrerURI,
                       aReferrerPolicy, aLoadGroup, aObserver, aLoadingDocument,
                       requestFlags, aContentPolicyType, true, _retval,
                       aLoadingPrincipal, corsmode)) {
       request = entry->GetRequest();
@@ -2315,17 +2332,29 @@ imgLoader::LoadImageWithChannel(nsIChann
 
   MOZ_ASSERT(NS_UsePrivateBrowsing(channel) == mRespectPrivacy);
 
   RefPtr<imgRequest> request;
 
   nsCOMPtr<nsIURI> uri;
   channel->GetURI(getter_AddRefs(uri));
   nsCOMPtr<nsIDocument> doc = do_QueryInterface(aCX);
-  ImageCacheKey key(uri, doc);
+
+  NS_ENSURE_TRUE(channel, NS_ERROR_FAILURE);
+  nsCOMPtr<nsILoadInfo> loadInfo = channel->GetLoadInfo();
+  NS_ENSURE_TRUE(loadInfo, NS_ERROR_FAILURE);
+
+  nsCOMPtr<nsIPrincipal> principal;
+  loadInfo->GetLoadingPrincipal(getter_AddRefs(principal));
+  NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE);
+
+  const PrincipalOriginAttributes& attrs =
+    BasePrincipal::Cast(principal)->OriginAttributesRef();
+
+  ImageCacheKey key(uri, attrs, doc);
 
   nsLoadFlags requestFlags = nsIRequest::LOAD_NORMAL;
   channel->GetLoadFlags(&requestFlags);
 
   RefPtr<imgCacheEntry> entry;
 
   if (requestFlags & nsIRequest::LOAD_BYPASS_CACHE) {
     RemoveFromCache(key);
@@ -2417,17 +2446,17 @@ imgLoader::LoadImageWithChannel(nsIChann
     // We use originalURI here to fulfil the imgIRequest contract on GetURI.
     nsCOMPtr<nsIURI> originalURI;
     channel->GetOriginalURI(getter_AddRefs(originalURI));
 
     // XXX(seth): We should be able to just use |key| here, except that |key| is
     // constructed above with the *current URI* and not the *original URI*. I'm
     // pretty sure this is a bug, and it's preventing us from ever getting a
     // cache hit in LoadImageWithChannel when redirects are involved.
-    ImageCacheKey originalURIKey(originalURI, doc);
+    ImageCacheKey originalURIKey(originalURI, attrs, doc);
 
     // Default to doing a principal check because we don't know who
     // started that load and whether their principal ended up being
     // inherited on the channel.
     NewRequestAndEntry(/* aForcePrincipalCheckForCacheEntry = */ true,
                        this, originalURIKey,
                        getter_AddRefs(request),
                        getter_AddRefs(entry));
--- a/image/test/unit/async_load_tests.js
+++ b/image/test/unit/async_load_tests.js
@@ -8,16 +8,19 @@
 
 var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cu = Components.utils;
 var Cr = Components.results;
 
 Cu.import("resource://testing-common/httpd.js");
 Cu.import("resource://gre/modules/NetUtil.jsm");
+Cu.import("resource://gre/modules/Services.jsm");
+
+var ssm = Services.scriptSecurityManager;
 
 var server = new HttpServer();
 server.registerDirectory("/", do_get_file(''));
 server.registerContentType("sjs", "sjs");
 server.start(-1);
 
 
 load('image_load_helpers.js');
@@ -92,17 +95,19 @@ function secondLoadDone(oldlistener, aRe
 // therefore would be at most risk of being served synchronously.
 function checkSecondLoad()
 {
   do_test_pending();
 
   var listener = new ImageListener(checkClone, secondLoadDone);
   var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools)
                 .createScriptedObserver(listener);
-  requests.push(gCurrentLoader.loadImageXPCOM(uri, null, null, "default", null, null, outer, null, 0, null));
+  var systemPrincipal = ssm.getSystemPrincipal();
+  requests.push(gCurrentLoader.loadImageXPCOM(uri, null, null, "default",
+                                              systemPrincipal, null, outer, null, 0, null));
   listener.synchronous = false;
 }
 
 function firstLoadDone(oldlistener, aRequest)
 {
   checkSecondLoad(uri);
 
   do_test_finished();
@@ -172,17 +177,19 @@ function startImageCallback(otherCb)
 {
   return function(listener, request)
   {
     // Make sure we can load the same image immediately out of the cache.
     do_test_pending();
     var listener2 = new ImageListener(null, function(foo, bar) { do_test_finished(); });
     var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools)
                   .createScriptedObserver(listener2);
-    requests.push(gCurrentLoader.loadImageXPCOM(uri, null, null, "default", null, null, outer, null, 0, null));
+    var systemPrincipal = ssm.getSystemPrincipal();
+    requests.push(gCurrentLoader.loadImageXPCOM(uri, null, null, "default",
+                                                systemPrincipal, null, outer, null, 0, null));
     listener2.synchronous = false;
 
     // Now that we've started another load, chain to the callback.
     otherCb(listener, request);
   }
 }
 
 var gCurrentLoader;
@@ -199,16 +206,18 @@ function run_test()
   do_register_cleanup(cleanup);
 
   gCurrentLoader = Cc["@mozilla.org/image/loader;1"].createInstance(Ci.imgILoader);
 
   do_test_pending();
   var listener = new ImageListener(startImageCallback(checkClone), firstLoadDone);
   var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools)
                 .createScriptedObserver(listener);
-  var req = gCurrentLoader.loadImageXPCOM(uri, null, null, "default", null, null, outer, null, 0, null);
+  var systemPrincipal = ssm.getSystemPrincipal();
+  var req = gCurrentLoader.loadImageXPCOM(uri, null, null, "default",
+                                          systemPrincipal, null, outer, null, 0, null);
   requests.push(req);
 
   // Ensure that we don't cause any mayhem when we lock an image.
   req.lockImage();
 
   listener.synchronous = false;
 }
--- a/image/test/unit/test_private_channel.js
+++ b/image/test/unit/test_private_channel.js
@@ -2,16 +2,18 @@ var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cr = Components.results;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/NetUtil.jsm");
 Cu.import("resource://testing-common/httpd.js");
 
+var ssm = Services.scriptSecurityManager;
+
 var server = new HttpServer();
 server.registerPathHandler('/image.png', imageHandler);
 server.start(-1);
 
 load('image_load_helpers.js');
 
 var gHits = 0;
 
@@ -79,17 +81,19 @@ function setup_chan(path, isPrivate, cal
 function loadImage(isPrivate, callback) {
   var listener = new ImageListener(null, callback);
   var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools)
                 .createScriptedObserver(listener);
   var uri = gIoService.newURI(gImgPath, null, null);
   var loadGroup = Cc["@mozilla.org/network/load-group;1"].createInstance(Ci.nsILoadGroup);
   loadGroup.notificationCallbacks = new NotificationCallbacks(isPrivate);
   var loader = isPrivate ? gPrivateLoader : gPublicLoader;
-  requests.push(loader.loadImageXPCOM(uri, null, null, "default", null, loadGroup, outer, null, 0, null));
+  var systemPrincipal = ssm.getSystemPrincipal();
+  requests.push(loader.loadImageXPCOM(uri, null, null, "default",
+                                      systemPrincipal, loadGroup, outer, null, 0, null));
   listener.synchronous = false;
 }
 
 function run_loadImage_tests() {
   function observer() {
     Services.obs.removeObserver(observer, "cacheservice:empty-cache");
     gHits = 0;
     loadImage(false, function() {
@@ -99,16 +103,19 @@ function run_loadImage_tests() {
             do_check_eq(gHits, 2);
             server.stop(do_test_finished);
           });
         });
       });
     });
   }
 
+  gPrivateLoader.QueryInterface(Ci.imgICache).clearCache(false);
+  gPublicLoader.QueryInterface(Ci.imgICache).clearCache(false);
+
   Services.obs.addObserver(observer, "cacheservice:empty-cache", false);
   let cs = Cc["@mozilla.org/netwerk/cache-storage-service;1"]
              .getService(Ci.nsICacheStorageService);
   cs.clear();
 }
 
 function cleanup()
 {
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -2202,23 +2202,29 @@ nsImageFrame::LoadIcon(const nsAString& 
 
   nsCOMPtr<nsILoadGroup> loadGroup;
   GetLoadGroup(aPresContext, getter_AddRefs(loadGroup));
 
   // For icon loads, we don't need to merge with the loadgroup flags
   nsLoadFlags loadFlags = nsIRequest::LOAD_NORMAL;
   nsContentPolicyType contentPolicyType = nsIContentPolicy::TYPE_INTERNAL_IMAGE;
 
+  nsCOMPtr<nsIScriptSecurityManager> ssm = nsContentUtils::GetSecurityManager();
+  NS_ENSURE_TRUE(ssm, NS_ERROR_FAILURE);
+  nsCOMPtr<nsIPrincipal> systemPrincipal;
+  ssm->GetSystemPrincipal(getter_AddRefs(systemPrincipal));
+  NS_ENSURE_TRUE(systemPrincipal, NS_ERROR_FAILURE);
+
   return il->LoadImage(realURI,     /* icon URI */
                        nullptr,      /* initial document URI; this is only
                                        relevant for cookies, so does not
                                        apply to icons. */
                        nullptr,      /* referrer (not relevant for icons) */
                        mozilla::net::RP_Default,
-                       nullptr,      /* principal (not relevant for icons) */
+                       systemPrincipal, /* principal (not relevant for icons) */
                        loadGroup,
                        gIconLoad,
                        nullptr,      /* No context */
                        nullptr,      /* Not associated with any particular document */
                        loadFlags,
                        nullptr,
                        contentPolicyType,
                        EmptyString(),
--- a/toolkit/system/gnome/nsAlertsIconListener.cpp
+++ b/toolkit/system/gnome/nsAlertsIconListener.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode:nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsAlertsIconListener.h"
 #include "imgIContainer.h"
 #include "imgIRequest.h"
+#include "nsContentUtils.h"
 #include "imgLoader.h"
 #include "nsNetUtil.h"
 #include "nsServiceManagerUtils.h"
 #include "nsSystemAlertsService.h"
 #include "nsIAlertsService.h"
 #include "nsIImageToPixbuf.h"
 #include "nsIStringBundle.h"
 #include "nsIObserverService.h"
@@ -243,24 +244,29 @@ nsAlertsIconListener::StartRequest(const
     mIconRequest = nullptr;
   }
 
   nsCOMPtr<nsIURI> imageUri;
   NS_NewURI(getter_AddRefs(imageUri), aImageUrl);
   if (!imageUri)
     return ShowAlert(nullptr);
 
-  imgLoader* il = aInPrivateBrowsing ? imgLoader::PrivateBrowsingLoader()
-                                     : imgLoader::NormalLoader();
+  nsCOMPtr<imgILoader> il(do_GetService("@mozilla.org/image/loader;1"));
   if (!il)
     return ShowAlert(nullptr);
 
+  nsCOMPtr<nsIScriptSecurityManager> ssm = nsContentUtils::GetSecurityManager();
+  NS_ENSURE_TRUE(ssm, NS_ERROR_FAILURE);
+  nsCOMPtr<nsIPrincipal> systemPrincipal;
+  ssm->GetSystemPrincipal(getter_AddRefs(systemPrincipal));
+  NS_ENSURE_TRUE(systemPrincipal, NS_ERROR_FAILURE);
+
   nsresult rv = il->LoadImageXPCOM(imageUri, nullptr, nullptr,
-                                   NS_LITERAL_STRING("default"), nullptr, nullptr,
-                                   this, nullptr,
+                                   NS_LITERAL_STRING("default"),
+                                   systemPrincipal, nullptr, this, nullptr,
                                    aInPrivateBrowsing ? nsIRequest::LOAD_ANONYMOUS :
                                                         nsIRequest::LOAD_NORMAL,
                                    nullptr, 0 /* use default */,
                                    getter_AddRefs(mIconRequest));
   if (NS_FAILED(rv))
     return rv;
 
   return NS_OK;