Bug 1942810 - Initialize remote permission service directly on "profile-after-change" r=asuth,permissions-reviewers,emz

With the previous implementation, the permission manager may miss the
"profile-after-change" event and won't start the remote permission service.

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

extensions/permissions/PermissionManager.cpp

@@ -782,7 +782,6 @@ nsresult PermissionManager::Init() {
   nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
   if (observerService) {
     observerService->AddObserver(this, "profile-do-change", true);
-    observerService->AddObserver(this, "profile-after-change", true);
     observerService->AddObserver(this, "testonly-reload-permissions-from-disk",
                                  true);
   }
@@ -1716,7 +1715,6 @@ PermissionManager::AddDefaultFromPrincipal(nsIPrincipal* aPrincipal,
                                            const nsACString& aType,
                                            uint32_t aPermission) {
   ENSURE_NOT_CHILD_PROCESS;
-  MOZ_ASSERT(mState == eReady);
 
   bool isValidPermissionPrincipal = false;
   nsresult rv = ShouldHandlePrincipalForPermission(aPrincipal,
@@ -2804,8 +2802,6 @@ NS_IMETHODIMP PermissionManager::Observe(nsISupports* aSubject,
     // profile startup is complete, and we didn't have the permissions file
     // before; init the db from the new location
     InitDB(false);
-  } else if (!nsCRT::strcmp(aTopic, "profile-after-change")) {
-    InitRemotePermissionService();
   } else if (!nsCRT::strcmp(aTopic, "testonly-reload-permissions-from-disk")) {
     // Testing mechanism to reload all permissions from disk. Because the
     // permission manager automatically initializes itself at startup, tests
@@ -2817,7 +2813,6 @@ NS_IMETHODIMP PermissionManager::Observe(nsISupports* aSubject,
     RemoveAllFromMemory();
     CloseDB(eNone);
     InitDB(false);
-    InitRemotePermissionService();
   } else if (!nsCRT::strcmp(aTopic, OBSERVER_TOPIC_IDLE_DAILY)) {
     PerformIdleDailyMaintenance();
   }
@@ -3264,33 +3259,6 @@ void PermissionManager::CompleteRead() {
   }
 }
 
-void PermissionManager::InitRemotePermissionService() {
-  // Check if this service is disabled by pref, and abort if it is.
-  if (!StaticPrefs::permissions_manager_remote_enabled()) {
-    return;
-  }
-
-  // Also abort if we are in a background task. We do not want to call remote
-  // settings there, because we do not want to pollute the background task
-  // profile, and because we don't need the remote permissions there anyways.
-#ifdef MOZ_BACKGROUNDTASKS
-  if (BackgroundTasks::IsBackgroundTaskMode()) {
-    return;
-  }
-#endif
-
-  NS_DispatchToCurrentThreadQueue(
-      NS_NewRunnableFunction(
-          "RemotePermissionService::Init",
-          [&] {
-            nsCOMPtr<nsIRemotePermissionService> remotePermissionService =
-                do_GetService(NS_REMOTEPERMISSIONSERVICE_CONTRACTID);
-            NS_ENSURE_TRUE_VOID(remotePermissionService);
-            remotePermissionService->Init();
-          }),
-      EventQueuePriority::Idle);
-}
-
 void PermissionManager::MaybeAddReadEntryFromMigration(
     const nsACString& aOrigin, const nsCString& aType, uint32_t aPermission,
     uint32_t aExpireType, int64_t aExpireTime, int64_t aModificationTime,

extensions/permissions/PermissionManager.h

@@ -651,10 +651,6 @@ class PermissionManager final : public nsIPermissionManager,
 
   void CompleteMigrations();
 
-  // Initialize service used for importing default permissions from remote
-  // settings
-  void InitRemotePermissionService();
-
   bool mMemoryOnlyDB;
 
   nsTHashtable<PermissionHashKey> mPermissionTable;

extensions/permissions/RemotePermissionService.sys.mjs

@@ -35,14 +35,17 @@ export class RemotePermissionService {
   QueryInterface = ChromeUtils.generateQI(["nsIRemotePermissionService"]);
 
   #rs = RemoteSettings(COLLECTION_NAME);
-  #onSyncCallback = null;
   #initialized = Promise.withResolvers();
   #allowedPermissionValues = ALLOWED_PERMISSION_VALUES;
 
+  constructor() {
+    this.init();
+  }
+
   /**
    * Asynchonously import all default permissions from remote settings into the
-   * permission manager. Also, if not already done, set up remote settings event
-   * listener to keep remote permissions in sync.
+   * permission manager and set up remote settings event listener to keep
+   * remote permissions in sync.
    */
   async init() {
     try {
@@ -53,9 +56,7 @@ export class RemotePermissionService {
       if (
         !Services.prefs.getBoolPref("permissions.manager.remote.enabled", false)
       ) {
-        throw Error(
-          "Tried to initialize remote permission service despite being disabled by pref"
-        );
+        return;
       }
 
       let remotePermissions = await this.#rs.get();
@@ -63,15 +64,7 @@ export class RemotePermissionService {
         this.#addDefaultPermission(permission);
       }
 
-      // Init could be called multiple times if the permission manager is
-      // reinitializing itself due to "testonly-reload-permissions-from-disk"
-      // being emitted. In that case, we don't shouldn't set up the RS listener
-      // again. We may also land in that situtation when "profile-do-change" is
-      // emitted.
-      if (!this.#onSyncCallback) {
-        this.#onSyncCallback = this.#onSync.bind(this);
-        this.#rs.on("sync", this.#onSyncCallback);
-      }
+      this.#rs.on("sync", this.#onSync.bind(this));
 
       this.#initialized.resolve();
     } catch (e) {

extensions/permissions/components.conf

@@ -29,5 +29,8 @@ Classes = [
         'constructor': 'RemotePermissionService',
         'singleton': True,
         'processes': ProcessSelector.MAIN_PROCESS_ONLY,
+        'categories': {
+            'profile-after-change': 'RemotePermissionService',
+        },
     },
 ]

extensions/permissions/test/unit/test_permmanager_remote.js

@@ -75,10 +75,6 @@ add_setup(async function () {
     rps.testAllowedPermissionValues
   );
 
-  // Initialize remote permission service
-  Services.prefs.setBoolPref("permissions.manager.remote.enabled", true);
-  let permObserver = Services.perms.QueryInterface(Ci.nsIObserver);
-  permObserver.observe(null, "profile-after-change", "");
   await rps.isInitialized;
 
   registerCleanupFunction(async () => {