Commit 61dd9a48 authored by Andrei Oprea's avatar Andrei Oprea
Browse files

Bug 1701948 - Early startup cache should work for Nimbus remote defaults r=k88hudson

parent 17884fe1
Loading
Loading
Loading
Loading
+16 −4
Original line number Diff line number Diff line
@@ -3,6 +3,13 @@ http://creativecommons.org/publicdomain/zero/1.0/ */

"use strict";

const { ExperimentFakes } = ChromeUtils.import(
  "resource://testing-common/NimbusTestUtils.jsm"
);
const { NimbusFeatures, ExperimentFeature } = ChromeUtils.import(
  "resource://nimbus/ExperimentAPI.jsm"
);

const BROWSER_GLUE = Cc["@mozilla.org/browser/browserglue;1"].getService()
  .wrappedJSObject;

@@ -204,9 +211,10 @@ add_task(async function not_major_upgrade() {
add_task(async function remote_disabled() {
  Services.telemetry.clearEvents();

  // TODO(bug 1701948): Set up actual remote defaults.
  NimbusFeatures.upgradeDialog._onRemoteReady();
  Services.prefs.setBoolPref("browser.startup.upgradeDialog.enabled", false);
  await ExperimentFakes.remoteDefaultsHelper({
    feature: NimbusFeatures.upgradeDialog,
    configuration: { enabled: false, variables: {} },
  });

  // Simulate starting from a previous version.
  await SpecialPowers.pushPrefEnv({
@@ -220,7 +228,11 @@ add_task(async function remote_disabled() {
    ["upgrade_dialog", "trigger", "reason", "disabled"],
    "Feature disabled for upgrade dialog requirements"
  );
  Services.prefs.clearUserPref("browser.startup.upgradeDialog.enabled");
  // Re-enable back
  await ExperimentFakes.remoteDefaultsHelper({
    feature: NimbusFeatures.upgradeDialog,
    configuration: { enabled: true, variables: {} },
  });
});

add_task(async function show_major_upgrade() {
+38 −35
Original line number Diff line number Diff line
@@ -367,7 +367,7 @@ class ExperimentFeature {
    this._waitForRemote = new Promise(
      resolve => (this._onRemoteReady = resolve)
    );
    this._remoteReady = false;
    this._listenForRemoteDefaults = this._listenForRemoteDefaults.bind(this);
    const variables = this.manifest?.variables || {};

    // Add special enabled flag
@@ -404,6 +404,36 @@ class ExperimentFeature {
        );
      }
    });

    /**
     * There are multiple events that can resolve the wait for remote defaults:
     * 1. The feature can receive data via the RS update cycle
     * 2. The RS update cycle finished; no record exists for this feature
     * 3. User was enrolled in an experiment that targets this feature, resolve
     * because experiments take priority.
     */
    ExperimentAPI._store.on(
      "remote-defaults-finalized",
      this._listenForRemoteDefaults
    );
    this.onUpdate(this._listenForRemoteDefaults);
  }

  _listenForRemoteDefaults(eventName, reason) {
    if (
      // When the update cycle finished
      eventName === "remote-defaults-finalized" ||
      // remote default or experiment available
      reason === "experiment-updated" ||
      reason === "remote-defaults-update"
    ) {
      ExperimentAPI._store.off(
        "remote-defaults-updated",
        this._listenForRemoteDefaults
      );
      this.off(this._listenForRemoteDefaults);
      this._onRemoteReady();
    }
  }

  _getUserPrefsValues() {
@@ -423,32 +453,7 @@ class ExperimentFeature {
  }

  async ready() {
    await ExperimentAPI.ready();
    // If Remote Defaults or Experiment Value are already available
    // we can proceed
    if (
      // We need to check for remote configs using the store directly
      // this instance won't do it until `ready()` completed.
      ExperimentAPI._store.getRemoteConfig(this.featureId) ||
      ExperimentAPI.activateBranch({ featureId: this.featureId })
    ) {
      this._remoteReady = true;
      this._onRemoteReady();
    } else {
      // We need to wait for the updates to come in
      let resolveEvent = (featureId, reason) => {
        if (
          reason === "remote-defaults-update" ||
          reason === "experiment-updated"
        ) {
          this._remoteReady = true;
          this._onRemoteReady();
          this.off(resolveEvent);
        }
      };
      this.onUpdate(resolveEvent);
    }
    return this._waitForRemote;
    return Promise.all([ExperimentAPI.ready(), this._waitForRemote]);
  }

  /**
@@ -517,17 +522,16 @@ class ExperimentFeature {
  }

  getRemoteConfig() {
    if (this._remoteReady) {
    let remoteConfig = ExperimentAPI._store.getRemoteConfig(this.featureId);
    if (!remoteConfig) {
      return null;
    }
    // Used to select a matching client config
    delete remoteConfig.targeting;

    return remoteConfig;
  }

    return null;
  }

  recordExposureEvent() {
    if (this._sendExposureEventOnce) {
      let experimentData = ExperimentAPI.activateBranch({
@@ -552,7 +556,6 @@ class ExperimentFeature {

  debug() {
    return {
      _remoteReady: this._remoteReady,
      enabled: this.isEnabled(),
      value: this.getValue(),
      experiment: ExperimentAPI.getExperimentMetaData({
+76 −7
Original line number Diff line number Diff line
@@ -18,7 +18,10 @@ const IS_MAIN_PROCESS =
  Services.appinfo.processType === Services.appinfo.PROCESS_TYPE_DEFAULT;
const REMOTE_DEFAULTS_KEY = "__REMOTE_DEFAULTS";

// This branch is used to store experiment data
const SYNC_DATA_PREF_BRANCH = "nimbus.syncdatastore.";
// This branch is used to store remote rollouts
const SYNC_DEFAULTS_PREF_BRANCH = "nimbus.syncdefaultsstore.";
let tryJSONParse = data => {
  try {
    return JSON.parse(data);
@@ -35,26 +38,49 @@ XPCOMUtils.defineLazyGetter(this, "syncDataStore", () => {
    }
  } catch (e) {}

  let prefBranch = Services.prefs.getBranch(SYNC_DATA_PREF_BRANCH);
  let experimentsPrefBranch = Services.prefs.getBranch(SYNC_DATA_PREF_BRANCH);
  let defaultsPrefBranch = Services.prefs.getBranch(SYNC_DEFAULTS_PREF_BRANCH);
  return {
    get(featureId) {
    _tryParsePrefValue(branch, pref) {
      try {
        return tryJSONParse(prefBranch.getStringPref(featureId, ""));
        return tryJSONParse(branch.getStringPref(pref, ""));
      } catch (e) {
        /* This is expected if we don't have anything stored */
      }

      return null;
    },
    set(featureId, value) {
    _trySetPrefValue(branch, pref, value) {
      try {
        prefBranch.setStringPref(featureId, JSON.stringify(value));
        branch.setStringPref(pref, JSON.stringify(value));
      } catch (e) {
        Cu.reportError(e);
      }
    },
    get(featureId) {
      return this._tryParsePrefValue(experimentsPrefBranch, featureId);
    },
    getDefault(featureId) {
      return this._tryParsePrefValue(defaultsPrefBranch, featureId);
    },
    set(featureId, value) {
      this._trySetPrefValue(experimentsPrefBranch, featureId, value);
    },
    setDefault(featureId, value) {
      this._trySetPrefValue(defaultsPrefBranch, featureId, value);
    },
    getAllDefaultBranches() {
      return defaultsPrefBranch.getChildList("");
    },
    delete(featureId) {
      prefBranch.clearUserPref(featureId);
      try {
        experimentsPrefBranch.clearUserPref(featureId);
      } catch (e) {}
    },
    deleteDefault(featureId) {
      try {
        defaultsPrefBranch.clearUserPref(featureId);
      } catch (e) {}
    },
  };
});
@@ -67,6 +93,11 @@ const SYNC_ACCESS_FEATURES = ["newtab", "aboutwelcome"];
class ExperimentStore extends SharedDataMap {
  constructor(sharedDataKey, options = { isParent: IS_MAIN_PROCESS }) {
    super(sharedDataKey || DEFAULT_STORE_ID, options);

    // Keep a session cache of all remote default configurations processed.
    // This is used to clear the preference cache when a configuration is
    // removed.
    this.remoteDefaultsSession = new Set();
  }

  /**
@@ -197,6 +228,31 @@ class ExperimentStore extends SharedDataMap {
    this._emitExperimentUpdates(updatedExperiment);
  }

  finalizeRemoteConfigs() {
    for (let featureId of syncDataStore.getAllDefaultBranches()) {
      if (
        !this.remoteDefaultsSession.has(featureId) &&
        this.getRemoteConfig(featureId)
      ) {
        // If we haven't seen this feature in any of the configurations
        // processed then we should clear the matching pref cache and session
        // data.
        const remoteConfigState = this.get(REMOTE_DEFAULTS_KEY);
        delete remoteConfigState?.[featureId];
        this.setNonPersistent(REMOTE_DEFAULTS_KEY, { ...remoteConfigState });
        syncDataStore.deleteDefault(featureId);
        this._emitFeatureUpdate(featureId, "remote-defaults-update");
      }
    }

    // Reset the cache to prepare for the next update cycle.
    this.remoteDefaultsSession = new Set();
    // Notify all ExperimentFeature instances that the Remote Defaults cycle finished
    // this will resolve the `onRemoteReady` promise for features that do not
    // have any remote data available.
    this.emit("remote-defaults-finalized");
  }

  /**
   * Store the remote configuration once loaded from Remote Settings.
   * @param {string} featureId The feature we want to update with remote defaults
@@ -208,6 +264,10 @@ class ExperimentStore extends SharedDataMap {
      ...remoteConfigState,
      [featureId]: { ...configuration },
    });
    if (SYNC_ACCESS_FEATURES.includes(featureId)) {
      syncDataStore.setDefault(featureId, configuration);
    }
    this.remoteDefaultsSession.add(featureId);
    this._emitFeatureUpdate(featureId, "remote-defaults-update");
  }

@@ -217,6 +277,15 @@ class ExperimentStore extends SharedDataMap {
   * @returns {{RemoteDefaults}|undefined} Remote defaults if available
   */
  getRemoteConfig(featureId) {
    return this.get(REMOTE_DEFAULTS_KEY)?.[featureId];
    return (
      this.get(REMOTE_DEFAULTS_KEY)?.[featureId] ||
      syncDataStore.getDefault(featureId)
    );
  }

  _deleteForTests(featureId) {
    super._deleteForTests(featureId);
    syncDataStore.deleteDefault(featureId);
    syncDataStore.delete(featureId);
  }
}
+2 −3
Original line number Diff line number Diff line
@@ -61,12 +61,11 @@ XPCOMUtils.defineLazyPreferenceGetter(
 * Remote Settings.
 */
const RemoteDefaultsLoader = {
  _initialized: false,

  async syncRemoteDefaults() {
    log.debug("Fetching remote defaults for NimbusFeatures.");
    try {
      this._onUpdatesReady(await this._remoteSettingsClient.get());
      await this._onUpdatesReady(await this._remoteSettingsClient.get());
      ExperimentManager.store.finalizeRemoteConfigs();
    } catch (e) {
      Cu.reportError(e);
    }
+7 −4
Original line number Diff line number Diff line
@@ -141,12 +141,15 @@ class SharedDataMap extends EventEmitter {
    }
    if (this.has(key)) {
      delete this._store.data[key];
      delete this._nonPersistentStore[key];
    }
    if (this._nonPersistentStore) {
      delete this._nonPersistentStore.__REMOTE_DEFAULTS?.[key];
    }

    this._store.saveSoon();
    this._syncToChildren();
    this._notifyUpdate();
  }
  }

  has(key) {
    return Boolean(this.get(key));
Loading