Bug 1474608 - P1 - Clear origin attributes directly for deleteDataFromOriginAttributesPattern if it's possible; r=asuth,baku

The idea of this patch is to try to not use oberver mechanism as possible. To
achieve that, it introduces deleteByOriginAttributes() to cleaners. Different
from other methods, it would only be executed if it's implemented from a
cleaner.

It doesn't remove oberver mechanism entirely since some cleaners are still using
that for other deleteByXXX() functions. So, it only applies removing stuff to
PushService, QuotaManagerService, ServiceWorkerManager, nsPermissionManager,
nsApplicationCacheService, and nsCookieService.

Since the original issue is related to QuotaManagerService, it adds xpcshell
test under the dom/quota/test/unit/ to ensure the behavior won't be changed
accidentally in the future.

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

--HG--
extra : moz-landing-system : lando

dom/interfaces/base/nsIServiceWorkerManager.idl

@@ -142,6 +142,9 @@ interface nsIServiceWorkerManager : nsISupports
   // It returns an array of nsIServiceWorkerRegistrationInfos.
   nsIArray getAllRegistrations();
 
+  // For clear-origin-attributes-data
+  void removeRegistrationsByOriginAttributes(in AString aOriginAttributes);
+
   // It calls softUpdate() for each child process.
   [implicit_jscontext] void propagateSoftUpdate(in jsval aOriginAttributes,
                                                 in AString aScope);

dom/quota/QuotaManagerService.cpp

@@ -812,17 +812,6 @@ QuotaManagerService::Observe(nsISupports* aSubject, const char* aTopic,
     return NS_OK;
   }
 
-  if (!strcmp(aTopic, "clear-origin-attributes-data")) {
-    nsCOMPtr<nsIQuotaRequest> request;
-    nsresult rv = ClearStoragesForOriginAttributesPattern(
-        nsDependentString(aData), getter_AddRefs(request));
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-
-    return NS_OK;
-  }
-
   if (!strcmp(aTopic, OBSERVER_TOPIC_IDLE_DAILY)) {
     PerformIdleMaintenance();
     return NS_OK;

dom/quota/test/unit/head.js

@@ -328,14 +328,14 @@ function getCurrentUsage(usageHandler) {
   return request;
 }
 
-function getPrincipal(url) {
+function getPrincipal(url, attr = {}) {
   let uri = Cc["@mozilla.org/network/io-service;1"]
     .getService(Ci.nsIIOService)
     .newURI(url);
   let ssm = Cc["@mozilla.org/scriptsecuritymanager;1"].getService(
     Ci.nsIScriptSecurityManager
   );
-  return ssm.createContentPrincipal(uri, {});
+  return ssm.createContentPrincipal(uri, attr);
 }
 
 function getCurrentPrincipal() {

dom/quota/test/unit/test_clearOriginAttributesData.js

@@ -0,0 +1,48 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+async function testSteps() {
+  const origins = [
+    {
+      origin: "https://example.com",
+      attribute: { userContextId: 1 },
+      path: "https+++example.com^userContextId=1",
+    },
+    {
+      origin: "https://example.com",
+      attribute: { userContextId: 2 },
+      path: "https+++example.com^userContextId=2",
+    },
+  ];
+
+  let request;
+  for (let origin of origins) {
+    request = initStorageAndOrigin(
+      getPrincipal(origin.origin, origin.attribute),
+      "default"
+    );
+    await requestFinished(request);
+  }
+
+  const path = "storage/default/";
+  for (let origin of origins) {
+    let originDir = getRelativeFile(path + origin.path);
+    ok(originDir.exists(), "Origin directory should have been created");
+  }
+
+  const removingId = 2;
+  Services.clearData.deleteDataFromOriginAttributesPattern({
+    userContextId: removingId,
+  });
+
+  for (let origin of origins) {
+    let originDir = getRelativeFile(path + origin.path);
+    if (origin.attribute.userContextId === removingId) {
+      ok(!originDir.exists(), "Origin directory should have been removed");
+    } else {
+      ok(originDir.exists(), "Origin directory shouldn't have been removed");
+    }
+  }
+}

dom/quota/test/unit/xpcshell.ini

@@ -33,6 +33,7 @@ support-files =
 [test_basics.js]
 [test_bad_origin_directory.js]
 [test_caching_groupMismatch.js]
+[test_clearOriginAttributesData.js]
 [test_createLocalStorage.js]
 [test_clearStorageForPrincipal.js]
 [test_defaultStorageUpgrade.js]

dom/serviceworkers/ServiceWorkerManager.cpp

@@ -89,8 +89,6 @@ using namespace mozilla::ipc;
 namespace mozilla {
 namespace dom {
 
-#define CLEAR_ORIGIN_DATA "clear-origin-attributes-data"
-
 static_assert(
     nsIHttpChannelInternal::CORS_MODE_SAME_ORIGIN ==
         static_cast<uint32_t>(RequestMode::Same_origin),
@@ -359,12 +357,6 @@ void ServiceWorkerManager::Init(ServiceWorkerRegistrar* aRegistrar) {
     nsTArray<ServiceWorkerRegistrationData> data;
     aRegistrar->GetRegistrations(data);
     LoadRegistrations(data);
-
-    if (obs) {
-      DebugOnly<nsresult> rv;
-      rv = obs->AddObserver(this, CLEAR_ORIGIN_DATA, false /* ownsWeak */);
-      MOZ_ASSERT(NS_SUCCEEDED(rv));
-    }
   }
 
   PBackgroundChild* actorChild = BackgroundChild::GetOrCreateForCurrentThread();
@@ -524,10 +516,6 @@ void ServiceWorkerManager::MaybeStartShutdown() {
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     obs->RemoveObserver(this, GetXPCOMShutdownTopic());
-
-    if (XRE_IsParentProcess()) {
-      obs->RemoveObserver(this, CLEAR_ORIGIN_DATA);
-    }
   }
 
   if (!mActor) {
@@ -2677,6 +2665,41 @@ ServiceWorkerManager::GetAllRegistrations(nsIArray** aResult) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+ServiceWorkerManager::RemoveRegistrationsByOriginAttributes(
+    const nsAString& aPattern) {
+  MOZ_ASSERT(XRE_IsParentProcess());
+  MOZ_ASSERT(NS_IsMainThread());
+
+  MOZ_ASSERT(!aPattern.IsEmpty());
+
+  OriginAttributesPattern pattern;
+  MOZ_ALWAYS_TRUE(pattern.Init(aPattern));
+
+  for (auto it1 = mRegistrationInfos.Iter(); !it1.Done(); it1.Next()) {
+    ServiceWorkerManager::RegistrationDataPerPrincipal* data = it1.UserData();
+
+    // We can use iteration because ForceUnregister (and Unregister) are
+    // async. Otherwise doing some R/W operations on an hashtable during
+    // iteration will crash.
+    for (auto it2 = data->mInfos.Iter(); !it2.Done(); it2.Next()) {
+      ServiceWorkerRegistrationInfo* reg = it2.UserData();
+
+      MOZ_ASSERT(reg);
+      MOZ_ASSERT(reg->Principal());
+
+      bool matches = pattern.Matches(reg->Principal()->OriginAttributesRef());
+      if (!matches) {
+        continue;
+      }
+
+      ForceUnregister(data, reg);
+    }
+  }
+
+  return NS_OK;
+}
+
 // MUST ONLY BE CALLED FROM Remove(), RemoveAll() and RemoveAllRegistrations()!
 void ServiceWorkerManager::ForceUnregister(
     RegistrationDataPerPrincipal* aRegistrationData,
@@ -2764,34 +2787,6 @@ void ServiceWorkerManager::PropagateRemoveAll() {
   mActor->SendPropagateRemoveAll();
 }
 
-void ServiceWorkerManager::RemoveAllRegistrations(
-    OriginAttributesPattern* aPattern) {
-  MOZ_ASSERT(NS_IsMainThread());
-
-  MOZ_ASSERT(aPattern);
-
-  for (auto it1 = mRegistrationInfos.Iter(); !it1.Done(); it1.Next()) {
-    ServiceWorkerManager::RegistrationDataPerPrincipal* data = it1.UserData();
-
-    // We can use iteration because ForceUnregister (and Unregister) are
-    // async. Otherwise doing some R/W operations on an hashtable during
-    // iteration will crash.
-    for (auto it2 = data->mInfos.Iter(); !it2.Done(); it2.Next()) {
-      ServiceWorkerRegistrationInfo* reg = it2.UserData();
-
-      MOZ_ASSERT(reg);
-      MOZ_ASSERT(reg->Principal());
-
-      bool matches = aPattern->Matches(reg->Principal()->OriginAttributesRef());
-      if (!matches) {
-        continue;
-      }
-
-      ForceUnregister(data, reg);
-    }
-  }
-}
-
 NS_IMETHODIMP
 ServiceWorkerManager::AddListener(nsIServiceWorkerManagerListener* aListener) {
   MOZ_ASSERT(NS_IsMainThread());
@@ -2822,15 +2817,6 @@ ServiceWorkerManager::RemoveListener(
 NS_IMETHODIMP
 ServiceWorkerManager::Observe(nsISupports* aSubject, const char* aTopic,
                               const char16_t* aData) {
-  if (strcmp(aTopic, CLEAR_ORIGIN_DATA) == 0) {
-    MOZ_ASSERT(XRE_IsParentProcess());
-    OriginAttributesPattern pattern;
-    MOZ_ALWAYS_TRUE(pattern.Init(nsAutoString(aData)));
-
-    RemoveAllRegistrations(&pattern);
-    return NS_OK;
-  }
-
   if (strcmp(aTopic, GetXPCOMShutdownTopic()) == 0) {
     MaybeStartShutdown();
     return NS_OK;

extensions/permissions/nsPermissionManager.cpp

@@ -979,7 +979,6 @@ nsresult nsPermissionManager::Init() {
     observerService->AddObserver(this, "profile-do-change", true);
     observerService->AddObserver(this, "testonly-reload-permissions-from-disk",
                                  true);
-    observerService->AddObserver(this, "clear-origin-attributes-data", true);
   }
 
   // ignore failure here, since it's non-fatal (we can run fine without
@@ -2543,8 +2542,6 @@ NS_IMETHODIMP nsPermissionManager::Observe(nsISupports* aSubject,
     RemoveAllFromMemory();
     CloseDB(false);
     InitDB(false);
-  } else if (!nsCRT::strcmp(aTopic, "clear-origin-attributes-data")) {
-    return RemovePermissionsWithAttributes(nsDependentString(someData));
   }
 
   return NS_OK;
@@ -2560,7 +2557,8 @@ nsresult nsPermissionManager::RemoveAllModifiedSince(
       });
 }
 
-nsresult nsPermissionManager::RemovePermissionsWithAttributes(
+NS_IMETHODIMP
+nsPermissionManager::RemovePermissionsWithAttributes(
     const nsAString& aPattern) {
   ENSURE_NOT_CHILD_PROCESS;
   mozilla::OriginAttributesPattern pattern;

extensions/permissions/nsPermissionManager.h

@@ -209,6 +209,9 @@ class nsPermissionManager final : public nsIPermissionManager,
    */
   static void Startup();
 
+  nsresult RemovePermissionsWithAttributes(
+      mozilla::OriginAttributesPattern& aAttrs);
+
   /**
    * See `nsIPermissionManager::GetPermissionsWithKey` for more info on
    * permission keys.
@@ -589,10 +592,6 @@ class nsPermissionManager final : public nsIPermissionManager,
   template <class T>
   nsresult RemovePermissionEntries(T aCondition);
 
-  nsresult RemovePermissionsWithAttributes(const nsAString& aPattern);
-  nsresult RemovePermissionsWithAttributes(
-      mozilla::OriginAttributesPattern& aAttrs);
-
   nsRefPtrHashtable<nsCStringHashKey,
                     mozilla::GenericNonExclusivePromise::Private>
       mPermissionKeyPromiseMap;

extensions/permissions/test/unit/test_permmanager_cleardata.js

@@ -11,13 +11,12 @@ function createPrincipal(aOrigin, aOriginAttributes) {
   );
 }
 
-// Return the data required by 'clear-origin-attributes-data' notification.
 function getData(aPattern) {
   return JSON.stringify(aPattern);
 }
 
 // Use aEntries to create principals, add permissions to them and check that they have them.
-// Then, it is notifying 'clear-origin-attributes-data' with the given aData and check if the permissions
+// Then, it is removing origin attributes with the given aData and check if the permissions
 // of principals[i] matches the permission in aResults[i].
 function test(aEntries, aData, aResults) {
   let principals = [];
@@ -44,7 +43,9 @@ function test(aEntries, aData, aResults) {
     );
   }
 
-  Services.obs.notifyObservers(null, "clear-origin-attributes-data", aData);
+  // `clear-origin-attributes-data` notification is removed from permission
+  // manager
+  pm.removePermissionsWithAttributes(aData);
 
   var length = aEntries.length;
   for (let i = 0; i < length; ++i) {

layout/build/nsLayoutModule.cpp

@@ -247,7 +247,6 @@ nsresult SessionStorageManagerConstructor(nsISupports* aOuter, REFNSIID aIID,
 
 static const mozilla::Module::CategoryEntry kLayoutCategories[] = {
     // clang-format off
-  { "clear-origin-attributes-data", "QuotaManagerService", "service," QUOTAMANAGER_SERVICE_CONTRACTID },
     {nullptr}
     // clang-format on
 };

layout/build/nsLayoutStatics.cpp

@@ -252,9 +252,6 @@ nsresult nsLayoutStatics::Initialize() {
 
   nsPermissionManager::Startup();
 
-  nsCookieService::AppClearDataObserverInit();
-  nsApplicationCacheService::AppClearDataObserverInit();
-
 #ifdef MOZ_XUL
   nsMenuBarListener::InitializeStatics();
 #endif

netwerk/base/nsIPermissionManager.idl

@@ -188,6 +188,11 @@ interface nsIPermissionManager : nsISupports
    */
   readonly attribute Array<nsIPermission> all;
 
+  /**
+   * Remove all permissions that will match the origin pattern.
+   */
+  void removePermissionsWithAttributes(in AString patternAsJSON);
+
   /**
    * Broadcasts permissions for the given principal to all content processes.
    *

netwerk/cache/nsApplicationCacheService.cpp

@@ -187,48 +187,3 @@ nsApplicationCacheService::GetGroupsTimeOrdered(nsTArray<nsCString>& keys) {
   NS_ENSURE_SUCCESS(rv, rv);
   return device->GetGroupsTimeOrdered(keys);
 }
-
-//-----------------------------------------------------------------------------
-// AppCacheClearDataObserver: handles clearing appcache data for app uninstall
-// and clearing user data events.
-//-----------------------------------------------------------------------------
-
-namespace {
-
-class AppCacheClearDataObserver final : public nsIObserver {
- public:
-  NS_DECL_ISUPPORTS
-
-  // nsIObserver implementation.
-  NS_IMETHOD
-  Observe(nsISupports* aSubject, const char* aTopic,
-          const char16_t* aData) override {
-    MOZ_ASSERT(!nsCRT::strcmp(aTopic, "clear-origin-attributes-data"));
-
-    nsresult rv;
-
-    nsCOMPtr<nsIApplicationCacheService> cacheService =
-        do_GetService(NS_APPLICATIONCACHESERVICE_CONTRACTID, &rv);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    return cacheService->EvictMatchingOriginAttributes(
-        nsDependentString(aData));
-  }
-
- private:
-  ~AppCacheClearDataObserver() = default;
-};
-
-NS_IMPL_ISUPPORTS(AppCacheClearDataObserver, nsIObserver)
-
-}  // namespace
-
-// Instantiates and registers AppCacheClearDataObserver for notifications
-void nsApplicationCacheService::AppClearDataObserverInit() {
-  nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
-  if (observerService) {
-    RefPtr<AppCacheClearDataObserver> obs = new AppCacheClearDataObserver();
-    observerService->AddObserver(obs, "clear-origin-attributes-data",
-                                 /*ownsWeak=*/false);
-  }
-}

netwerk/cookie/nsCookieService.cpp

@@ -102,8 +102,6 @@ static StaticRefPtr<nsCookieService> gCookieService;
 #define IDX_SAME_SITE 11
 #define IDX_RAW_SAME_SITE 12
 
-#define TOPIC_CLEAR_ORIGIN_DATA "clear-origin-attributes-data"
-
 static const int64_t kCookiePurgeAge =
     int64_t(30 * 24 * 60 * 60) * PR_USEC_PER_SEC;  // 30 days in microseconds
 
@@ -458,31 +456,6 @@ NS_IMPL_ISUPPORTS(CloseCookieDBListener, mozIStorageCompletionCallback)
 
 namespace {
 
-class AppClearDataObserver final : public nsIObserver {
-  ~AppClearDataObserver() = default;
-
- public:
-  NS_DECL_ISUPPORTS
-
-  // nsIObserver implementation.
-  NS_IMETHOD
-  Observe(nsISupports* aSubject, const char* aTopic,
-          const char16_t* aData) override {
-    MOZ_ASSERT(!nsCRT::strcmp(aTopic, TOPIC_CLEAR_ORIGIN_DATA));
-
-    MOZ_ASSERT(XRE_IsParentProcess());
-
-    nsCOMPtr<nsICookieManager> cookieManager =
-        do_GetService(NS_COOKIEMANAGER_CONTRACTID);
-    MOZ_ASSERT(cookieManager);
-
-    return cookieManager->RemoveCookiesWithOriginAttributes(
-        nsDependentString(aData), EmptyCString());
-  }
-};
-
-NS_IMPL_ISUPPORTS(AppClearDataObserver, nsIObserver)
-
 // comparator class for sorting cookies by entry and index.
 class CompareCookiesByIndex {
  public:
@@ -558,14 +531,6 @@ already_AddRefed<nsCookieService> nsCookieService::GetSingleton() {
   return do_AddRef(gCookieService);
 }
 
-/* static */
-void nsCookieService::AppClearDataObserverInit() {
-  nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
-  nsCOMPtr<nsIObserver> obs = new AppClearDataObserver();
-  observerService->AddObserver(obs, TOPIC_CLEAR_ORIGIN_DATA,
-                               /* ownsWeak= */ false);
-}
-
 /******************************************************************************
  * nsCookieService impl:
  * public methods

netwerk/cookie/nsCookieService.h

@@ -191,7 +191,6 @@ class nsCookieService final : public nsICookieService,
    * (thus instantiating it, if necessary) and clear all the cookies for that
    * app.
    */
-  static void AppClearDataObserverInit();
   static nsAutoCString GetPathFromURI(nsIURI* aHostURI);
   static nsresult GetBaseDomain(nsIEffectiveTLDService* aTLDService,
                                 nsIURI* aHostURI, nsCString& aBaseDomain,

toolkit/components/cleardata/ClearDataService.jsm

@@ -31,7 +31,7 @@ XPCOMUtils.defineLazyServiceGetter(
   "nsITrackingDBService"
 );
 
-// A Cleaner is an object with 4 methods. These methods must return a Promise
+// A Cleaner is an object with 5 methods. These methods must return a Promise
 // object. Here a description of these methods:
 // * deleteAll() - this method _must_ exist. When called, it deletes all the
 //                 data owned by the cleaner.
@@ -50,6 +50,9 @@ XPCOMUtils.defineLazyServiceGetter(
 //                          **no fallback is used**, as for a number of
 //                          cleaners, no such data will ever exist and
 //                          therefore clearing it does not make sense.
+// * deleteByOriginAttributes() - this method is implemented only if the cleaner
+//                                knows how to delete data by originAttributes
+//                                pattern.
 
 const CookieCleaner = {
   deleteByLocalFiles(aOriginAttributes) {
@@ -76,6 +79,17 @@ const CookieCleaner = {
     return Services.cookies.removeAllSince(aFrom);
   },
 
+  deleteByOriginAttributes(aOriginAttributesString) {
+    return new Promise(aResolve => {
+      try {
+        Services.cookies.removeCookiesWithOriginAttributes(
+          aOriginAttributesString
+        );
+      } catch (ex) {}
+      aResolve();
+    });
+  },
+
   deleteAll() {
     return new Promise(aResolve => {
       Services.cookies.removeAll();
@@ -383,6 +397,18 @@ const MediaDevicesCleaner = {
 };
 
 const AppCacheCleaner = {
+  deleteByOriginAttributes(aOriginAttributesString) {
+    return new Promise(aResolve => {
+      let appCacheService = Cc[
+        "@mozilla.org/network/application-cache-service;1"
+      ].getService(Ci.nsIApplicationCacheService);
+      try {
+        appCacheService.evictMatchingOriginAttributes(aOriginAttributesString);
+      } catch (ex) {}
+      aResolve();
+    });
+  },
+
   deleteAll() {
     // AppCache: this doesn't wait for the cleanup to be complete.
     OfflineAppCacheHelper.clear();
@@ -573,6 +599,36 @@ const QuotaCleaner = {
     return Promise.all(promises);
   },
 
+  deleteByOriginAttributes(aOriginAttributesString) {
+    // The legacy LocalStorage implementation that will eventually be removed.
+    // And it should've been cleared while notifying observers with
+    // clear-origin-attributes-data.
+
+    return ServiceWorkerCleanUp.removeFromOriginAttributes(
+      aOriginAttributesString
+    )
+      .then(
+        _ => /* exceptionThrown = */ false,
+        _ => /* exceptionThrown = */ true
+      )
+      .then(exceptionThrown => {
+        // QuotaManager: In the event of a failure, we call reject to propagate
+        // the error upwards.
+        return new Promise((aResolve, aReject) => {
+          let req = Services.qms.clearStoragesForOriginAttributesPattern(
+            aOriginAttributesString
+          );
+          req.callback = () => {
+            if (req.resultCode == Cr.NS_OK) {
+              aResolve();
+            } else {
+              aReject({ message: "Delete by origin attributes failed" });
+            }
+          };
+        });
+      });
+  },
+
   deleteAll() {
     // localStorage
     Services.obs.notifyObservers(null, "extension:purge-localStorage");
@@ -851,6 +907,11 @@ const PermissionsCleaner = {
     return Promise.resolve();
   },
 
+  deleteByOriginAttributes(aOriginAttributesString) {
+    Services.perms.removePermissionsWithAttributes(aOriginAttributesString);
+    return Promise.resolve();
+  },
+
   deleteAll() {
     Services.perms.removeAll();
     return Promise.resolve();
@@ -1180,10 +1241,32 @@ ClearDataService.prototype = Object.freeze({
   },
 
   deleteDataFromOriginAttributesPattern(aPattern) {
+    if (!aPattern) {
+      return Cr.NS_ERROR_INVALID_ARG;
+    }
+
+    let patternString = JSON.stringify(aPattern);
+    // XXXtt remove clear-origin-attributes-data entirely
     Services.obs.notifyObservers(
       null,
       "clear-origin-attributes-data",
-      JSON.stringify(aPattern)
+      patternString
+    );
+
+    let dummyCallback = {
+      onDataDeleted: () => {},
+    };
+    return this._deleteInternal(
+      Ci.nsIClearDataService.CLEAR_ALL,
+      dummyCallback,
+      aCleaner => {
+        if (aCleaner.deleteByOriginAttributes) {
+          return aCleaner.deleteByOriginAttributes(patternString);
+        }
+
+        // We don't want to delete more than what is strictly required.
+        return Promise.resolve();
+      }
     );
   },
 

toolkit/components/cleardata/ServiceWorkerCleanUp.jsm

@@ -72,6 +72,13 @@ this.ServiceWorkerCleanUp = {
     return Promise.all(promises);
   },
 
+  removeFromOriginAttributes(aOriginAttributesString) {
+    serviceWorkerManager.removeRegistrationsByOriginAttributes(
+      aOriginAttributesString
+    );
+    return Promise.resolve();
+  },
+
   removeAll() {
     let promises = [];
     let serviceWorkers = serviceWorkerManager.getAllRegistrations();