Revert "Bug 1956519 - Shared RemoteSettingsService, r=nanj,adw" for causing browser-chrome crashes.

This reverts commit 0b37f9e94c.

browser/components/urlbar/QuickSuggest.sys.mjs

@@ -816,14 +816,6 @@ class _QuickSuggest {
       this.#initStarted = false;
       this.#initResolvers = Promise.withResolvers();
     }
-
-    if (this.rustBackend) {
-      // Make sure to await any queued ingests before re-initializing.  Otherwise there could be a race
-      // between when that ingestion finishes and when the test finishes and calls
-      // `SharedRemoteSettingsService.updateServer()` to reset the remote settings server.
-      await this.rustBackend.ingestPromise;
-    }
-
     await this.init(testOverrides);
   }
 

browser/components/urlbar/private/SuggestBackendRust.sys.mjs

@@ -13,8 +13,11 @@ ChromeUtils.defineESModuleGetters(lazy, {
   InterruptKind: "resource://gre/modules/RustSuggest.sys.mjs",
   ObjectUtils: "resource://gre/modules/ObjectUtils.sys.mjs",
   QuickSuggest: "resource:///modules/QuickSuggest.sys.mjs",
-  SharedRemoteSettingsService:
-    "resource://gre/modules/RustSharedRemoteSettingsService.sys.mjs",
+  Region: "resource://gre/modules/Region.sys.mjs",
+  RemoteSettingsConfig2: "resource://gre/modules/RustRemoteSettings.sys.mjs",
+  RemoteSettingsContext: "resource://gre/modules/RustRemoteSettings.sys.mjs",
+  RemoteSettingsServer: "resource://gre/modules/RustRemoteSettings.sys.mjs",
+  RemoteSettingsService: "resource://gre/modules/RustRemoteSettings.sys.mjs",
   SuggestIngestionConstraints: "resource://gre/modules/RustSuggest.sys.mjs",
   SuggestStoreBuilder: "resource://gre/modules/RustSuggest.sys.mjs",
   Suggestion: "resource://gre/modules/RustSuggest.sys.mjs",
@@ -94,16 +97,18 @@ export class SuggestBackendRust extends SuggestBackend {
     // which is a "cannot-be-a-base" URL. The error is harmless, but it can be
     // logged many times during a test suite.
     //
-    // To prevent Suggest from using the dummy URL, we skip setting the
-    // remoteSettingsService, which prevents the Suggest store from being
+    // To prevent Suggest from using the dummy URL, we skip setting the initial
+    // RS config here during tests, which prevents the Suggest store from being
     // created, effectively disabling Rust suggestions. Suggest tests manually
     // set the RS config when they set up the mock RS server, so they'll work
     // fine. Alternatively the test harnesses could disable Suggest by default
     // just like they set the server pref to the dummy URL, but Suggest is more
     // than Rust suggestions.
     if (!lazy.Utils.shouldSkipRemoteActivityDueToTests) {
-      this.#remoteSettingsService =
-        lazy.SharedRemoteSettingsService.rustService();
+      this.#setRemoteSettingsConfig({
+        serverUrl: lazy.Utils.SERVER_URL,
+        bucketName: lazy.Utils.actualBucketName("main"),
+      });
     }
   }
 
@@ -454,6 +459,15 @@ export class SuggestBackendRust extends SuggestBackend {
     );
   }
 
+  /**
+   * @returns {string}
+   *   The path of the directory that should contain the remote settings cache
+   *   used internally by the Rust component.
+   */
+  get #remoteSettingsStoragePath() {
+    return Services.dirsvc.get("ProfLD", Ci.nsIFile).path;
+  }
+
   /**
    * @returns {Array}
    *   Each item in this array identifies an enabled Rust suggestion type and
@@ -534,8 +548,44 @@ export class SuggestBackendRust extends SuggestBackend {
   }
 
   #makeStore() {
-    this.logger.info("Creating SuggestStore");
-    if (!this.#remoteSettingsService) {
+    this.logger.info("Creating SuggestStore", {
+      server: this.#remoteSettingsServer,
+      bucketName: this.#remoteSettingsBucketName,
+      dataPath: this.#storeDataPath,
+      storagePath: this.#remoteSettingsStoragePath,
+    });
+
+    if (!this.#remoteSettingsServer) {
+      return null;
+    }
+
+    let rsContext = {
+      formFactor: "desktop",
+      appId: Services.appinfo.ID || "",
+      channel: AppConstants.IS_ESR ? "esr" : AppConstants.MOZ_UPDATE_CHANNEL,
+      appVersion: Services.appinfo.version,
+      locale: Services.locale.appLocaleAsBCP47,
+      os: AppConstants.platform,
+      osVersion: Services.sysinfo.get("version"),
+    };
+
+    // We assume `QuickSuggest` init already awaited `Region.init()`.
+    if (lazy.Region.home) {
+      rsContext.country = lazy.Region.home;
+    }
+
+    let rsService;
+    try {
+      rsService = lazy.RemoteSettingsService.init(
+        this.#remoteSettingsStoragePath,
+        new lazy.RemoteSettingsConfig2({
+          server: this.#remoteSettingsServer,
+          bucketName: this.#remoteSettingsBucketName,
+          appContext: new lazy.RemoteSettingsContext(rsContext),
+        })
+      );
+    } catch (error) {
+      this.logger.error("Error creating RemoteSettingsService", error);
       return null;
     }
 
@@ -543,7 +593,7 @@ export class SuggestBackendRust extends SuggestBackend {
     try {
       builder = lazy.SuggestStoreBuilder.init()
         .dataPath(this.#storeDataPath)
-        .remoteSettingsService(this.#remoteSettingsService)
+        .remoteSettingsService(rsService)
         .loadExtension(
           AppConstants.SQLITE_LIBRARY_FILENAME,
           "sqlite3_fts5_init"
@@ -677,6 +727,14 @@ export class SuggestBackendRust extends SuggestBackend {
     return lazy.SuggestionProvider[key];
   }
 
+  #setRemoteSettingsConfig(options) {
+    let { serverUrl, bucketName } = options || {};
+    this.#remoteSettingsServer = serverUrl
+      ? new lazy.RemoteSettingsServer.Custom(serverUrl)
+      : null;
+    this.#remoteSettingsBucketName = bucketName;
+  }
+
   /**
    * Dismissals are stored in the Rust component but were previously stored as
    * URL digests in a pref. This method migrates the pref to the Rust component
@@ -750,8 +808,8 @@ export class SuggestBackendRust extends SuggestBackend {
     return this.#enabledSuggestionTypes;
   }
 
-  async _test_setRemoteSettingsService(remoteSettingsService) {
-    this.#remoteSettingsService = remoteSettingsService;
+  async _test_setRemoteSettingsConfig(options) {
+    this.#setRemoteSettingsConfig(options);
     if (this.isEnabled) {
       // Recreate the store and re-ingest.
       Services.prefs.clearUserPref(INGEST_TIMER_LAST_UPDATE_PREF);
@@ -782,7 +840,8 @@ export class SuggestBackendRust extends SuggestBackend {
 
   #ingestQueue;
   #shutdownBlocker;
-  #remoteSettingsService;
+  #remoteSettingsServer;
+  #remoteSettingsBucketName;
 }
 
 /**

browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs

@@ -14,8 +14,6 @@ ChromeUtils.defineESModuleGetters(lazy, {
   RemoteSettingsServer:
     "resource://testing-common/RemoteSettingsServer.sys.mjs",
   SearchUtils: "moz-src:///toolkit/components/search/SearchUtils.sys.mjs",
-  SharedRemoteSettingsService:
-    "resource://gre/modules/RustSharedRemoteSettingsService.sys.mjs",
   Suggestion: "resource://gre/modules/RustSuggest.sys.mjs",
   TestUtils: "resource://testing-common/TestUtils.sys.mjs",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.sys.mjs",
@@ -240,13 +238,10 @@ class _QuickSuggestTestUtils {
     }
 
     // Tell the Rust backend to use the local remote setting server.
-    lazy.SharedRemoteSettingsService.updateServer({
-      url: this.#remoteSettingsServer.url.toString(),
+    await lazy.QuickSuggest.rustBackend._test_setRemoteSettingsConfig({
       bucketName: "main",
+      serverUrl: this.#remoteSettingsServer.url.toString(),
     });
-    await lazy.QuickSuggest.rustBackend._test_setRemoteSettingsService(
-      lazy.SharedRemoteSettingsService.rustService()
-    );
 
     // Wait for the Rust backend to finish syncing.
     await this.forceSync();
@@ -296,7 +291,7 @@ class _QuickSuggestTestUtils {
       lazy.UrlbarPrefs.clear("quicksuggest.dataCollection.enabled");
     }
 
-    await lazy.QuickSuggest.rustBackend._test_setRemoteSettingsService(null);
+    await lazy.QuickSuggest.rustBackend._test_setRemoteSettingsConfig(null);
 
     this.#log("#uninitQuickSuggest", "Done");
   }
@@ -1419,7 +1414,7 @@ class _QuickSuggestTestUtils {
 
     let originalHome = lazy.Region.home;
     if (homeRegion) {
-      lazy.Region._setHomeRegion(homeRegion, true);
+      lazy.Region._setHomeRegion(homeRegion, false);
     }
 
     let available = Services.locale.availableLocales;
@@ -1440,7 +1435,7 @@ class _QuickSuggestTestUtils {
     await callback();
 
     if (homeRegion) {
-      lazy.Region._setHomeRegion(originalHome, true);
+      lazy.Region._setHomeRegion(originalHome, false);
     }
 
     promise = promiseChanges(requested);

toolkit/components/contentrelevancy/ContentRelevancyManager.sys.mjs

@@ -14,8 +14,8 @@ ChromeUtils.defineESModuleGetters(lazy, {
   Interest: "resource://gre/modules/RustRelevancy.sys.mjs",
   InterestVector: "resource://gre/modules/RustRelevancy.sys.mjs",
   RelevancyStore: "resource://gre/modules/RustRelevancy.sys.mjs",
-  SharedRemoteSettingsService:
-    "resource://gre/modules/RustSharedRemoteSettingsService.sys.mjs",
+  RemoteSettingsConfig2: "resource://gre/modules/RustRemoteSettings.sys.mjs",
+  RemoteSettingsService: "resource://gre/modules/RustRemoteSettings.sys.mjs",
   score: "resource://gre/modules/RustRelevancy.sys.mjs",
 });
 
@@ -376,7 +376,6 @@ class RelevancyManager {
       ) ??
       false
     ) {
-      await this.#storeManager.store.ensureInterestDataPopulated();
       interestVector = await this.#storeManager.store.ingest(urls);
     }
 
@@ -499,10 +498,18 @@ class RustRelevancyStoreManager {
     if (rustRelevancyStore === undefined) {
       rustRelevancyStore = lazy.RelevancyStore;
     }
-    this.#store = rustRelevancyStore.init(
-      path,
-      lazy.SharedRemoteSettingsService.rustService()
+    // Initialize a RemoteSettingsService for the relevancy store
+    // TODO (1956519): consolidate this with the Suggest code and only create a single app-wide remote settings
+    // service.  For now this duplication is okay though because we're not really shipping Relevancy -- it's only enabled via a
+    // pref.
+    const rsService = lazy.RemoteSettingsService.init(
+      PathUtils.join(
+        Services.dirsvc.get("ProfLD", Ci.nsIFile).path,
+        "remote-settings"
+      ),
+      new lazy.RemoteSettingsConfig2({})
     );
+    this.#store = rustRelevancyStore.init(path, rsService);
   }
 
   get store() {

toolkit/components/contentrelevancy/tests/xpcshell/test_ContentRelevancyManager.js

@@ -7,8 +7,6 @@ ChromeUtils.defineESModuleGetters(this, {
   AsyncShutdown: "resource://gre/modules/AsyncShutdown.sys.mjs",
   ContentRelevancyManager:
     "resource://gre/modules/ContentRelevancyManager.sys.mjs",
-  SharedRemoteSettingsService:
-    "resource://gre/modules/RustSharedRemoteSettingsService.sys.mjs",
   exposedForTesting: "resource://gre/modules/ContentRelevancyManager.sys.mjs",
   TestUtils: "resource://testing-common/TestUtils.sys.mjs",
   setTimeout: "resource://gre/modules/Timer.sys.mjs",
@@ -64,10 +62,8 @@ add_task(function test_store_manager() {
   );
   Assert.equal(fakeRustRelevancyStore.init.callCount, 1);
   Assert.deepEqual(fakeRustRelevancyStore.init.firstCall.args[0], "test-path");
-  Assert.strictEqual(
-    fakeRustRelevancyStore.init.firstCall.args[1],
-    SharedRemoteSettingsService.rustService()
-  );
+  // TODO (1956519): test that the second arg is the app-wide RemoteSettingsService
+  // store should throw before the manager is enabled
   Assert.throws(() => storeManager.store, /StoreDisabledError/);
   // Once the manager is enabled, store should return the RustRelevancyStore
   storeManager.enable();

toolkit/components/uniffi-bindgen-gecko-js/components/generated/RustRemoteSettings.sys.mjs

@@ -941,13 +941,17 @@ export class RemoteSettingsService {
                 }
                 throw e;
             }
-            return UniFFIScaffolding.callSync(
+            return UniFFIScaffolding.callAsyncWrapper(
                 29, // remote_settings:uniffi_remote_settings_fn_method_remotesettingsservice_update_config
                 FfiConverterTypeRemoteSettingsService.lower(this),
                 FfiConverterTypeRemoteSettingsConfig2.lower(config),
             )
         }
-        return handleRustResult(functionCall(), liftResult, liftError);
+        try {
+            return functionCall().then((result) => handleRustResult(result, liftResult, liftError));
+        }  catch (error) {
+            return Promise.reject(error)
+        }
     }
 
 }

toolkit/components/uniffi-bindgen-gecko-js/config.toml

@@ -62,7 +62,6 @@ enable = true
 main_thread = [
   "RemoteSettings.new",
   "RemoteSettingsService.new",
-  "RemoteSettingsService.update_config",
 ]
 
 [error_support.async_wrappers]

toolkit/modules/RustSharedRemoteSettingsService.sys.mjs

@@ -1,99 +0,0 @@
-/* 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/. */
-
-// Import these directly, since we're going to be using them immediately to construct SharedRemoteSettingsService
-import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs";
-import { Region } from "resource://gre/modules/Region.sys.mjs";
-import {
-  RemoteSettingsConfig2,
-  RemoteSettingsContext,
-  RemoteSettingsServer,
-  RemoteSettingsService,
-} from "resource://gre/modules/RustRemoteSettings.sys.mjs";
-import { Utils } from "resource://services-settings/Utils.sys.mjs";
-
-/**
- * Rust RemoteSettingsService singleton
- *
- * This component manages the app-wide Rust RemoteSetingsService that's
- * shared by various other Rust components.
- *
- * This is only intended to be passed to Rust code. If you want a
- * general-purpose Remote settings client, use the JS one:
- *
- * - https://firefox-source-docs.mozilla.org/services/settings/index.html
- * - https://searchfox.org/mozilla-central/source/services/settings/remote-settings.sys.mjs
- */
-class _SharedRemoteSettingsService {
-  #config;
-  #rustService;
-
-  constructor() {
-    const storageDir = PathUtils.join(
-      Services.dirsvc.get("ProfLD", Ci.nsIFile).path,
-      "remote-settings"
-    );
-
-    this.#config = new RemoteSettingsConfig2({
-      server: new RemoteSettingsServer.Custom(Utils.SERVER_URL),
-      bucketName: Utils.actualBucketName("main"),
-      appContext: new RemoteSettingsContext({
-        formFactor: "desktop",
-        appId: Services.appinfo.ID || "",
-        channel: AppConstants.IS_ESR ? "esr" : AppConstants.MOZ_UPDATE_CHANNEL,
-        appVersion: Services.appinfo.version,
-        locale: Services.locale.appLocaleAsBCP47,
-        os: AppConstants.platform,
-        osVersion: Services.sysinfo.get("version"),
-        country: Region.home ?? undefined,
-      }),
-    });
-
-    Services.obs.addObserver(this, Region.REGION_TOPIC);
-
-    this.#rustService = RemoteSettingsService.init(storageDir, this.#config);
-  }
-
-  /**
-   * Update the Remote Settings server
-   *
-   * @param {object} opts object with the following fields:
-   * - `url`: server URL (defaults to the production URL)
-   * - `bucketName`: bucket name (defaults to "main")
-   */
-  updateServer(opts) {
-    this.#config.server = opts.url
-      ? new RemoteSettingsServer.Custom(opts.url)
-      : undefined;
-    this.#config.bucketName = opts.bucketName ?? Utils.actualBucketName("main");
-    this.#rustService.updateConfig(this.#config);
-  }
-
-  /**
-   * Get a reference to the Rust RemoteSettingsService object
-   */
-  rustService() {
-    return this.#rustService;
-  }
-
-  /**
-   * Sync server data for all active clients
-   */
-  async sync() {
-    // TODO (1966163): Hook this up to a timer.  There's currently no mechanism that calls this.
-    await this.#rustService.sync();
-  }
-
-  observe(subj, topic) {
-    if (topic == Region.REGION_TOPIC) {
-      const newCountry = subj.data;
-      if (newCountry != this.#config.appContext.country) {
-        this.#config.appContext.country = newCountry;
-        this.#rustService.updateConfig(this.#config);
-      }
-    }
-  }
-}
-
-export const SharedRemoteSettingsService = new _SharedRemoteSettingsService();

toolkit/modules/moz.build

@@ -108,9 +108,6 @@ with Files("PrivateBrowsingUtils.sys.mjs"):
 with Files("Promise*.sys.mjs"):
     BUG_COMPONENT = ("Toolkit", "Async Tooling")
 
-with Files("RustSharedRemoteSettingsService.sys.mjs"):
-    BUG_COMPONENT = ("Application Services", "Remote Settings")
-
 with Files("ShortcutUtils.sys.mjs"):
     BUG_COMPONENT = ("Firefox", "Toolbars and Customization")
 
@@ -129,6 +126,7 @@ with Files("WindowsLaunchOnLogin.sys.mjs"):
 with Files("WindowsRegistry.sys.mjs"):
     BUG_COMPONENT = ("Toolkit", "General")
 
+
 XPCSHELL_TESTS_MANIFESTS += ["tests/xpcshell/xpcshell.toml"]
 BROWSER_CHROME_MANIFESTS += ["tests/browser/browser.toml"]
 MOCHITEST_CHROME_MANIFESTS += ["tests/chrome/chrome.toml"]
@@ -201,7 +199,6 @@ EXTRA_JS_MODULES += [
     "Region.sys.mjs",
     "RemotePageAccessManager.sys.mjs",
     "ResetProfile.sys.mjs",
-    "RustSharedRemoteSettingsService.sys.mjs",
     "SelectionUtils.sys.mjs",
     "ServiceRequest.sys.mjs",
     "ShortcutUtils.sys.mjs",