Bug 1954134 - Simplify ImageCacheKey partitioning. r=necko-reviewers,valentin,arai

Use the same scheme as the JS and CSS caches:

https://searchfox.org/mozilla-central/rev/5c2888b35d56928d252acf84e8816fa89a8a6a61/layout/style/Loader.cpp#2341
https://searchfox.org/mozilla-central/rev/5c2888b35d56928d252acf84e8816fa89a8a6a61/dom/script/ScriptLoader.cpp#329

(And fonts, and plenty of others)

Differential Revision: https://phabricator.services.mozilla.com/D241664

dom/script/SharedScriptCache.cpp

@@ -24,7 +24,7 @@ ScriptHashKey::ScriptHashKey(ScriptLoader* aLoader,
                              const JS::loader::ScriptLoadRequest* aRequest)
     : PLDHashEntryHdr(),
       mURI(aRequest->mURI),
-      mPrincipal(aRequest->TriggeringPrincipal()),
+      mTriggeringPrincipal(aRequest->TriggeringPrincipal()),
       mLoaderPrincipal(aLoader->LoaderPrincipal()),
       mPartitionPrincipal(aLoader->PartitionedPrincipal()),
       mCORSMode(aRequest->CORSMode()),
@@ -56,7 +56,7 @@ bool ScriptHashKey::KeyEquals(const ScriptHashKey& aKey) const {
     }
   }
 
-  if (!mPrincipal->Equals(aKey.mPrincipal)) {
+  if (!mTriggeringPrincipal->Equals(aKey.mTriggeringPrincipal)) {
     return false;
   }
 

dom/script/SharedScriptCache.h

@@ -38,7 +38,7 @@ class ScriptHashKey : public PLDHashEntryHdr {
   explicit ScriptHashKey(const ScriptHashKey& aKey)
       : PLDHashEntryHdr(),
         mURI(aKey.mURI),
-        mPrincipal(aKey.mPrincipal),
+        mTriggeringPrincipal(aKey.mTriggeringPrincipal),
         mLoaderPrincipal(aKey.mLoaderPrincipal),
         mPartitionPrincipal(aKey.mPartitionPrincipal),
         mCORSMode(aKey.mCORSMode),
@@ -55,7 +55,7 @@ class ScriptHashKey : public PLDHashEntryHdr {
   ScriptHashKey(ScriptHashKey&& aKey)
       : PLDHashEntryHdr(),
         mURI(std::move(aKey.mURI)),
-        mPrincipal(std::move(aKey.mPrincipal)),
+        mTriggeringPrincipal(std::move(aKey.mTriggeringPrincipal)),
         mLoaderPrincipal(std::move(aKey.mLoaderPrincipal)),
         mPartitionPrincipal(std::move(aKey.mPartitionPrincipal)),
         mCORSMode(std::move(aKey.mCORSMode)),
@@ -87,7 +87,7 @@ class ScriptHashKey : public PLDHashEntryHdr {
     return nsURIHashKey::HashKey(aKey->mURI);
   }
 
-  nsIPrincipal* Principal() const { return mPrincipal; }
+  nsIPrincipal* TriggeringPrincipal() const { return mTriggeringPrincipal; }
   nsIPrincipal* LoaderPrincipal() const { return mLoaderPrincipal; }
   nsIPrincipal* PartitionPrincipal() const { return mPartitionPrincipal; }
 
@@ -97,7 +97,7 @@ class ScriptHashKey : public PLDHashEntryHdr {
 
  protected:
   const nsCOMPtr<nsIURI> mURI;
-  const nsCOMPtr<nsIPrincipal> mPrincipal;
+  const nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
   const nsCOMPtr<nsIPrincipal> mLoaderPrincipal;
   const nsCOMPtr<nsIPrincipal> mPartitionPrincipal;
   const CORSMode mCORSMode;

image/ImageCacheKey.cpp

@@ -13,15 +13,12 @@
 #include "mozilla/StoragePrincipalHelper.h"
 #include "mozilla/Unused.h"
 #include "mozilla/dom/Document.h"
-#include "mozilla/dom/File.h"
 #include "mozilla/dom/ServiceWorkerManager.h"
 #include "mozilla/StaticPrefs_privacy.h"
 #include "mozilla/StorageAccess.h"
 #include "nsContentUtils.h"
 #include "nsHashKeys.h"
 #include "nsLayoutUtils.h"
-#include "nsPrintfCString.h"
-#include "nsString.h"
 
 namespace mozilla {
 
@@ -29,33 +26,33 @@ using namespace dom;
 
 namespace image {
 
+static nsIPrincipal* GetLoaderPrincipal(Document* aDocument) {
+  return aDocument ? aDocument->NodePrincipal()
+                   : nsContentUtils::GetSystemPrincipal();
+}
+
+static nsIPrincipal* GetPartitionPrincipal(Document* aDocument) {
+  if (aDocument && StaticPrefs::privacy_partition_network_state()) {
+    return aDocument->PartitionedPrincipal();
+  }
+  return GetLoaderPrincipal(aDocument);
+}
+
 ImageCacheKey::ImageCacheKey(nsIURI* aURI, CORSMode aCORSMode,
                              const OriginAttributes& aAttrs,
                              Document* aDocument)
     : mURI(aURI),
-      mOriginAttributes(aAttrs),
       mControlledDocument(GetSpecialCaseDocumentToken(aDocument)),
-      mIsolationKey(GetIsolationKey(aDocument, aURI)),
+      mLoaderPrincipal(GetLoaderPrincipal(aDocument)),
+      mPartitionPrincipal(GetPartitionPrincipal(aDocument)),
       mCORSMode(aCORSMode),
-      mAppType(GetAppType(aDocument)) {}
+      mAppType(GetAppType(aDocument)) {
+  MOZ_ASSERT(mLoaderPrincipal);
+  MOZ_ASSERT(mPartitionPrincipal);
+}
 
-ImageCacheKey::ImageCacheKey(const ImageCacheKey& aOther)
+ImageCacheKey::ImageCacheKey(const ImageCacheKey& aOther) = default;
-    : mURI(aOther.mURI),
+ImageCacheKey::ImageCacheKey(ImageCacheKey&& aOther) = default;
-      mOriginAttributes(aOther.mOriginAttributes),
-      mControlledDocument(aOther.mControlledDocument),
-      mIsolationKey(aOther.mIsolationKey),
-      mHash(aOther.mHash),
-      mCORSMode(aOther.mCORSMode),
-      mAppType(aOther.mAppType) {}
-
-ImageCacheKey::ImageCacheKey(ImageCacheKey&& aOther)
-    : mURI(std::move(aOther.mURI)),
-      mOriginAttributes(aOther.mOriginAttributes),
-      mControlledDocument(aOther.mControlledDocument),
-      mIsolationKey(aOther.mIsolationKey),
-      mHash(aOther.mHash),
-      mCORSMode(aOther.mCORSMode),
-      mAppType(aOther.mAppType) {}
 
 bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const {
   // Don't share the image cache between a controlled document and anything
@@ -63,14 +60,8 @@ bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const {
   if (mControlledDocument != aOther.mControlledDocument) {
     return false;
   }
-  // Don't share the image cache between two top-level documents of different
+
-  // base domains.
+  if (!mPartitionPrincipal->Equals(aOther.mPartitionPrincipal)) {
-  if (!mIsolationKey.Equals(aOther.mIsolationKey,
-                            nsCaseInsensitiveCStringComparator)) {
-    return false;
-  }
-  // The origin attributes always have to match.
-  if (mOriginAttributes != aOther.mOriginAttributes) {
     return false;
   }
 
@@ -90,22 +81,11 @@ bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const {
 
 void ImageCacheKey::EnsureHash() const {
   MOZ_ASSERT(mHash.isNothing());
-  PLDHashNumber hash = 0;
-
-  // 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", mControlledDocument);
-  nsAutoCString suffix;
-  mOriginAttributes.CreateSuffix(suffix);
 
   nsAutoCString spec;
   Unused << mURI->GetSpec(spec);
-  hash = HashString(spec);
+  mHash.emplace(AddToHash(mPartitionPrincipal->GetHashValue(), HashString(spec),
-
+                          mControlledDocument, mAppType, mCORSMode));
-  hash = AddToHash(hash, HashString(suffix), HashString(mIsolationKey),
-                   HashString(ptr), mAppType);
-  mHash.emplace(hash);
 }
 
 /* static */
@@ -127,53 +107,6 @@ void* ImageCacheKey::GetSpecialCaseDocumentToken(Document* aDocument) {
   return nullptr;
 }
 
-/* static */
-nsCString ImageCacheKey::GetIsolationKey(Document* aDocument, nsIURI* aURI) {
-  if (!aDocument || !aDocument->GetInnerWindow()) {
-    return ""_ns;
-  }
-
-  // Network-state isolation
-  if (StaticPrefs::privacy_partition_network_state()) {
-    OriginAttributes oa;
-    StoragePrincipalHelper::GetOriginAttributesForNetworkState(aDocument, oa);
-
-    nsAutoCString suffix;
-    oa.CreateSuffix(suffix);
-
-    return std::move(suffix);
-  }
-
-  // If the window is 3rd party resource, let's see if first-party storage
-  // access is granted for this image.
-  if (AntiTrackingUtils::IsThirdPartyWindow(aDocument->GetInnerWindow(),
-                                            nullptr)) {
-    uint32_t rejectedReason = 0;
-    Unused << rejectedReason;
-    return ShouldAllowAccessFor(aDocument->GetInnerWindow(), aURI, true,
-                                &rejectedReason)
-               ? ""_ns
-               : aDocument->GetBaseDomain();
-  }
-
-  // Another scenario is if this image is a 3rd party resource loaded by a
-  // first party context. In this case, we should check if the nsIChannel has
-  // been marked as tracking resource, but we don't have the channel yet at
-  // this point.  The best approach here is to be conservative: if we are sure
-  // that the permission is granted, let's return 0. Otherwise, let's make a
-  // unique image cache per the top-level document eTLD+1.
-  if (!ApproximateAllowAccessForWithoutChannel(aDocument->GetInnerWindow(),
-                                               aURI)) {
-    // If we are here, the image is a 3rd-party resource loaded by a first-party
-    // context. We can just use the document's base domain as the key because it
-    // should be the same as the top-level document's base domain.
-    return aDocument
-        ->GetBaseDomain();  // because we don't have anything better!
-  }
-
-  return ""_ns;
-}
-
 /* static */
 nsIDocShell::AppType ImageCacheKey::GetAppType(Document* aDocument) {
   if (!aDocument) {

image/ImageCacheKey.h

@@ -12,7 +12,6 @@
 
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/Maybe.h"
-#include "mozilla/RefPtr.h"
 #include "PLDHashTable.h"
 #include "nsIDocShell.h"
 
@@ -50,14 +49,11 @@ class ImageCacheKey final {
   /// A weak pointer to the URI.
   nsIURI* URI() const { return mURI; }
 
+  nsIPrincipal* PartitionPrincipal() const { return mPartitionPrincipal; }
+  nsIPrincipal* LoaderPrincipal() const { return mLoaderPrincipal; }
+
   CORSMode GetCORSMode() const { return mCORSMode; }
 
-  const OriginAttributes& OriginAttributesRef() const {
-    return mOriginAttributes;
-  }
-
-  const nsCString& IsolationKeyRef() const { return mIsolationKey; }
-
   /// A token indicating which service worker controlled document this entry
   /// belongs to, if any.
   void* ControlledDocument() const { return mControlledDocument; }
@@ -67,11 +63,6 @@ class ImageCacheKey final {
   // token for the key. All those exceptions are handled by this method.
   static void* GetSpecialCaseDocumentToken(dom::Document* aDocument);
 
-  // For anti-tracking we need to use an isolation key. It can be the suffix of
-  // the PatitionedPrincipal (see StoragePrincipalHelper.h) or the top-level
-  // document's base domain. This is handled by this method.
-  static nsCString GetIsolationKey(dom::Document* aDocument, nsIURI* aURI);
-
   // The AppType of the docshell an image is loaded in can influence whether the
   // image is allowed to load. The specific AppType is fetched by this method.
   static nsIDocShell::AppType GetAppType(dom::Document* aDocument);
@@ -79,9 +70,9 @@ class ImageCacheKey final {
   void EnsureHash() const;
 
   nsCOMPtr<nsIURI> mURI;
-  const OriginAttributes mOriginAttributes;
   void* mControlledDocument;
-  nsCString mIsolationKey;
+  nsCOMPtr<nsIPrincipal> mLoaderPrincipal;
+  nsCOMPtr<nsIPrincipal> mPartitionPrincipal;
   mutable Maybe<PLDHashNumber> mHash;
   const CORSMode mCORSMode;
   const nsIDocShell::AppType mAppType;

image/imgLoader.cpp

@@ -1376,8 +1376,7 @@ imgLoader::ClearCache(JS::Handle<JS::Value> chrome) {
   return ClearCache(chrome.isBoolean() ? Some(chrome.toBoolean()) : Nothing());
 }
 
-nsresult
+nsresult imgLoader::ClearCache(mozilla::Maybe<bool> chrome) {
-imgLoader::ClearCache(mozilla::Maybe<bool> chrome) {
   if (XRE_IsParentProcess()) {
     bool privateLoader = this == gPrivateBrowsingLoader;
     for (auto* cp : ContentParent::AllProcesses(ContentParent::eLive)) {
@@ -1447,90 +1446,46 @@ nsresult imgLoader::RemoveEntriesInternal(
     return NS_ERROR_INVALID_ARG;
   }
 
-  nsCOMPtr<nsIEffectiveTLDService> tldService;
   AutoTArray<RefPtr<imgCacheEntry>, 128> entriesToBeRemoved;
 
-  Maybe<OriginAttributesPattern> patternWithPartitionKey = Nothing();
-  if (aPattern) {
-    // Used for checking for cache entries partitioned under aSchemelessSite.
-    OriginAttributesPattern pattern(aPattern.ref());
-    pattern.mPartitionKeyPattern.Construct();
-    pattern.mPartitionKeyPattern.Value().mBaseDomain.Construct(
-        NS_ConvertUTF8toUTF16(aSchemelessSite.ref()));
-
-    patternWithPartitionKey.emplace(std::move(pattern));
-  }
-
   // For base domain we only clear the non-chrome cache.
   for (const auto& entry : mCache) {
     const auto& key = entry.GetKey();
 
+    // TODO(emilio): Deduplicate this with SharedSubresourceCache.
     const bool shouldRemove = [&] {
-      // The isolation key is either just the site, or an origin suffix
-      // which contains the partitionKey holding the baseDomain.
-
       if (aPrincipal) {
-        nsCOMPtr<nsIPrincipal> keyPrincipal =
+        return key.LoaderPrincipal()->Equals(aPrincipal.ref());
-            BasePrincipal::CreateContentPrincipal(key.URI(),
-                                                  key.OriginAttributesRef());
-        return keyPrincipal->Equals(aPrincipal.ref());
       }
 
       if (!aSchemelessSite) {
         return false;
       }
-      // Clear by site and pattern.
+      // Clear by site
-      nsAutoCString host;
+      nsIPrincipal* partitionPrincipal = key.PartitionPrincipal();
-      nsresult rv = key.URI()->GetHost(host);
+      nsAutoCString principalBaseDomain;
-      if (NS_FAILED(rv) || host.IsEmpty()) {
+      nsresult rv = partitionPrincipal->GetBaseDomain(principalBaseDomain);
-        return false;
-      }
-
-      if (!tldService) {
-        tldService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID);
-      }
-      if (NS_WARN_IF(!tldService)) {
-        return false;
-      }
-
-      bool hasRootDomain = false;
-      rv = tldService->HasRootDomain(host, aSchemelessSite.ref(),
-                                     &hasRootDomain);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return false;
       }
 
-      if (hasRootDomain && aPattern->Matches(key.OriginAttributesRef())) {
+      if (principalBaseDomain.Equals(aSchemelessSite.ref()) &&
+          aPattern.ref().Matches(partitionPrincipal->OriginAttributesRef())) {
         return true;
       }
 
-      // Attempt to parse isolation key into origin attributes.
+      // Clear entries partitioned under aSchemelessSite. We need to add the
-      Maybe<OriginAttributes> originAttributesWithPartitionKey;
+      // partition key filter to aPattern so that we include any OA filtering
-      {
+      // specified by the caller. For example the caller may pass aPattern = {
-        OriginAttributes attrs;
+      // privateBrowsingId: 1 } which means we may only clear partitioned
-        if (attrs.PopulateFromSuffix(key.IsolationKeyRef())) {
+      // private browsing data.
-          OriginAttributes attrsWithPartitionKey(key.OriginAttributesRef());
+      OriginAttributesPattern patternWithPartitionKey(aPattern.ref());
-          attrsWithPartitionKey.mPartitionKey = attrs.mPartitionKey;
+      patternWithPartitionKey.mPartitionKeyPattern.Construct();
-          originAttributesWithPartitionKey.emplace(
+      patternWithPartitionKey.mPartitionKeyPattern.Value()
-              std::move(attrsWithPartitionKey));
+          .mBaseDomain.Construct(NS_ConvertUTF8toUTF16(aSchemelessSite.ref()));
-        }
-      }
 
-      // Match it against the pattern that contains the partition key and any
+      return patternWithPartitionKey.Matches(
-      // fields set by the caller pattern.
+          partitionPrincipal->OriginAttributesRef());
-      if (originAttributesWithPartitionKey.isSome()) {
-        nsAutoCString oaSuffixForPrinting;
-        originAttributesWithPartitionKey->CreateSuffix(oaSuffixForPrinting);
-
-        nsAutoString patternForPrinting;
-        patternWithPartitionKey->ToJSON(patternForPrinting);
-
-        return patternWithPartitionKey.ref().Matches(
-            originAttributesWithPartitionKey.ref());
-      }
-
-      // The isolation key is the site.
-      return aSchemelessSite->Equals(key.IsolationKeyRef());
     }();
 
     if (shouldRemove) {

layout/style/Loader.cpp

@@ -144,7 +144,7 @@ namespace mozilla {
 
 SheetLoadDataHashKey::SheetLoadDataHashKey(const css::SheetLoadData& aLoadData)
     : mURI(aLoadData.mURI),
-      mPrincipal(aLoadData.mTriggeringPrincipal),
+      mTriggeringPrincipal(aLoadData.mTriggeringPrincipal),
       mLoaderPrincipal(aLoadData.mLoader->LoaderPrincipal()),
       mPartitionPrincipal(aLoadData.mLoader->PartitionedPrincipal()),
       mEncodingGuess(aLoadData.mGuessedEncoding),
@@ -154,7 +154,7 @@ SheetLoadDataHashKey::SheetLoadDataHashKey(const css::SheetLoadData& aLoadData)
       mIsLinkRelPreloadOrEarlyHint(aLoadData.IsLinkRelPreloadOrEarlyHint()) {
   MOZ_COUNT_CTOR(SheetLoadDataHashKey);
   MOZ_ASSERT(mURI);
-  MOZ_ASSERT(mPrincipal);
+  MOZ_ASSERT(mTriggeringPrincipal);
   MOZ_ASSERT(mLoaderPrincipal);
   MOZ_ASSERT(mPartitionPrincipal);
   aLoadData.mSheet->GetIntegrity(mSRIMetadata);
@@ -180,16 +180,20 @@ bool SheetLoadDataHashKey::KeyEquals(const SheetLoadDataHashKey& aKey) const {
     return true;
   }
 
-  if (!mPrincipal->Equals(aKey.mPrincipal)) {
+  if (!mTriggeringPrincipal->Equals(aKey.mTriggeringPrincipal)) {
-    LOG((" > Principal mismatch\n"));
+    LOG((" > Triggering principal mismatch\n"));
     return false;
   }
 
   // We only check for partition principal equality if any of the loads are
   // triggered by a document rather than e.g. an extension (which have different
   // origins than the loader principal).
-  if (mPrincipal->Equals(mLoaderPrincipal) ||
+  //
-      aKey.mPrincipal->Equals(aKey.mLoaderPrincipal)) {
+  // TODO(emilio): The idea of this code is allowing sharing extension
+  // stylesheets on different origins, see bug 1645987 comment 5, but I think
+  // this is less important (maybe completely irrelevant?) with Fission.
+  if (mTriggeringPrincipal->Equals(mLoaderPrincipal) ||
+      aKey.mTriggeringPrincipal->Equals(aKey.mLoaderPrincipal)) {
     if (!mPartitionPrincipal->Equals(aKey.mPartitionPrincipal)) {
       LOG((" > Partition principal mismatch\n"));
       return false;

layout/style/Loader.h

@@ -55,7 +55,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr {
 
   explicit SheetLoadDataHashKey(const SheetLoadDataHashKey* aKey)
       : mURI(aKey->mURI),
-        mPrincipal(aKey->mPrincipal),
+        mTriggeringPrincipal(aKey->mTriggeringPrincipal),
         mLoaderPrincipal(aKey->mLoaderPrincipal),
         mPartitionPrincipal(aKey->mPartitionPrincipal),
         mEncodingGuess(aKey->mEncodingGuess),
@@ -76,7 +76,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr {
                        const dom::SRIMetadata& aSRIMetadata,
                        css::StylePreloadKind aPreloadKind)
       : mURI(aURI),
-        mPrincipal(aPrincipal),
+        mTriggeringPrincipal(aPrincipal),
         mLoaderPrincipal(aLoaderPrincipal),
         mPartitionPrincipal(aPartitionPrincipal),
         mEncodingGuess(aEncodingGuess),
@@ -94,7 +94,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr {
 
   SheetLoadDataHashKey(SheetLoadDataHashKey&& toMove)
       : mURI(std::move(toMove.mURI)),
-        mPrincipal(std::move(toMove.mPrincipal)),
+        mTriggeringPrincipal(std::move(toMove.mTriggeringPrincipal)),
         mLoaderPrincipal(std::move(toMove.mLoaderPrincipal)),
         mPartitionPrincipal(std::move(toMove.mPartitionPrincipal)),
         mEncodingGuess(std::move(toMove.mEncodingGuess)),
@@ -130,7 +130,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr {
 
   nsIURI* URI() const { return mURI; }
 
-  nsIPrincipal* Principal() const { return mPrincipal; }
+  nsIPrincipal* TriggeringPrincipal() const { return mTriggeringPrincipal; }
 
   nsIPrincipal* LoaderPrincipal() const { return mLoaderPrincipal; }
 
@@ -142,7 +142,7 @@ class SheetLoadDataHashKey : public PLDHashEntryHdr {
 
  protected:
   const nsCOMPtr<nsIURI> mURI;
-  const nsCOMPtr<nsIPrincipal> mPrincipal;
+  const nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
   const nsCOMPtr<nsIPrincipal> mLoaderPrincipal;
   const nsCOMPtr<nsIPrincipal> mPartitionPrincipal;
   // The encoding guess is the encoding the sheet would get if the request

layout/style/SharedSubResourceCache.h

@@ -295,7 +295,10 @@ void SharedSubResourceCache<Traits, Derived>::ClearInProcess(
         }
       }
 
-      if (aPrincipal && iter.Key().Principal()->Equals(aPrincipal.ref())) {
+      if (aPrincipal &&
+          iter.Key().TriggeringPrincipal()->Equals(aPrincipal.ref())) {
+        // FIXME(emilio): I don't think we want the triggering principal, but
+        // the loader principal (or the partition principal), most likely?
         return true;
       }
       if (!aSchemelessSite) {