Bug 1640709 - Also clear doh-rollout.uri if doh-rollout.trr-selection.commit-result is falsey. r=valentin,johannh

Depends on D76619

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

browser/extensions/doh-rollout/experiments/trrselect/api.js

@@ -72,14 +72,22 @@ this.trrselect = class trrselect extends ExtensionAPI {
           },
 
           async run() {
+            // If persisting the selection is disabled, clear the existing
+            // selection.
+            if (!Services.prefs.getBoolPref(kCommitSelectionPref, false)) {
+              Services.prefs.clearUserPref(kRolloutURIPref);
+            }
+
             if (!Services.prefs.getBoolPref(kEnabledPref, false)) {
               return;
             }
 
+            // If we already have a selection, nothing to be done.
             if (Services.prefs.prefHasUserValue(kRolloutURIPref)) {
               return;
             }
 
+            // Populate the dry-run-result if needed.
             await this.dryRun();
 
             // If persisting the selection is disabled, don't commit the value.
@@ -87,6 +95,7 @@ this.trrselect = class trrselect extends ExtensionAPI {
               return;
             }
 
+            // All good, commit the value!
             Services.prefs.setCharPref(
               kRolloutURIPref,
               Services.prefs.getCharPref(kDryRunResultPref)

browser/extensions/doh-rollout/test/browser/browser_trrSelect.js

@@ -19,10 +19,9 @@ add_task(async function testTRRSelect() {
 
   // Clean start: doh-rollout.uri should be set after init.
   setPassingHeuristics();
+  let prefPromise = TestUtils.waitForPrefChange(prefs.DOH_SELF_ENABLED_PREF);
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
-  await BrowserTestUtils.waitForCondition(() => {
-    return Preferences.get(prefs.DOH_SELF_ENABLED_PREF);
-  });
+  await prefPromise;
   is(Preferences.get(prefs.DOH_SELF_ENABLED_PREF), true, "Breadcrumb saved.");
   is(
     Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
@@ -37,10 +36,10 @@ add_task(async function testTRRSelect() {
   // Reset and restart add-on for good measure.
   Preferences.reset(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF);
   Preferences.reset(prefs.DOH_TRR_SELECT_URI_PREF);
+
+  prefPromise = TestUtils.waitForPrefChange(prefs.DOH_TRR_SELECT_URI_PREF);
   await restartAddon();
-  await BrowserTestUtils.waitForCondition(() => {
-    return Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF);
-  });
+  await prefPromise;
   is(
     Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "https://dummytrr.com/query",
@@ -51,11 +50,38 @@ add_task(async function testTRRSelect() {
   await ensureTRRMode(2);
   await checkHeuristicsTelemetry("enable_doh", "startup");
 
-  // Reset and restart add-on again, but this time with committing disabled.
-  // dry-run-result should be recorded but not be committed.
+  // Disable committing and reset. The committed URI should be cleared but the
+  // dry-run-result should persist.
+  Preferences.set(prefs.DOH_TRR_SELECT_COMMIT_PREF, false);
+  prefPromise = TestUtils.waitForPrefChange(prefs.DOH_TRR_SELECT_URI_PREF);
+  await restartAddon();
+  await prefPromise;
+  ok(
+    !Preferences.isSet(prefs.DOH_TRR_SELECT_URI_PREF),
+    "TRR selection cleared."
+  );
+  try {
+    await BrowserTestUtils.waitForCondition(() => {
+      return !Preferences.isSet(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF);
+    });
+    ok(false, "Dry run result was cleared, fail!");
+  } catch (e) {
+    ok(true, "Dry run result was not cleared.");
+  }
+  is(
+    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    "https://dummytrr.com/query",
+    "dry-run result has the correct value."
+  );
+
+  // Wait for heuristics to complete.
+  await ensureTRRMode(2);
+  await checkHeuristicsTelemetry("enable_doh", "startup");
+
+  // Reset and restart add-on again, dry-run-result should be recorded but not
+  // be committed. Committing is still disabled from above.
   Preferences.reset(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF);
   Preferences.reset(prefs.DOH_TRR_SELECT_URI_PREF);
-  Preferences.set(prefs.DOH_TRR_SELECT_COMMIT_PREF, false);
   await restartAddon();
   try {
     await BrowserTestUtils.waitForCondition(() => {
@@ -83,10 +109,9 @@ add_task(async function testTRRSelect() {
     prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF,
     "https://dummytrr2.com/query"
   );
+  prefPromise = TestUtils.waitForPrefChange(prefs.DOH_TRR_SELECT_URI_PREF);
   await restartAddon();
-  await BrowserTestUtils.waitForCondition(() => {
-    return Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF);
-  });
+  await prefPromise;
   is(
     Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "https://dummytrr2.com/query",
@@ -104,10 +129,9 @@ add_task(async function testTRRSelect() {
     prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF,
     "https://dummytrr3.com/query"
   );
+  prefPromise = TestUtils.waitForPrefChange(prefs.DOH_TRR_SELECT_URI_PREF);
   await restartAddon();
-  await BrowserTestUtils.waitForCondition(() => {
-    return Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF);
-  });
+  await prefPromise;
   is(
     Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "https://dummytrr.com/query",