Bug 1638502 - [DoH] Persist TRR-selection dry-run result. r=valentin,johannh
☠☠ backed out by e2117466c528 ☠ ☠
authorNihanth Subramanya <nhnt11@gmail.com>
Mon, 25 May 2020 17:35:17 +0000
changeset 532070 680c68517121400c80dc66bcb3b0e444c989f1b0
parent 532069 e3bde0ccb270cfecf490476ef273af9a6c0f81a0
child 532071 ff9e48641bc68bd55dd7888f73395ff3f7dc093f
push id116974
push usernhnt11@gmail.com
push dateMon, 25 May 2020 17:36:10 +0000
treeherderautoland@680c68517121 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin, johannh
bugs1638502
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1638502 - [DoH] Persist TRR-selection dry-run result. r=valentin,johannh Differential Revision: https://phabricator.services.mozilla.com/D75660
browser/extensions/doh-rollout/background.js
browser/extensions/doh-rollout/experiments/trrselect/api.js
browser/extensions/doh-rollout/experiments/trrselect/schema.json
browser/extensions/doh-rollout/test/browser/browser.ini
browser/extensions/doh-rollout/test/browser/browser_cleanFlow.js
browser/extensions/doh-rollout/test/browser/browser_dirtyEnable.js
browser/extensions/doh-rollout/test/browser/browser_doorhangerUserReject.js
browser/extensions/doh-rollout/test/browser/browser_doorhanger_newProfile.js
browser/extensions/doh-rollout/test/browser/browser_policyOverride.js
browser/extensions/doh-rollout/test/browser/browser_rollback.js
browser/extensions/doh-rollout/test/browser/browser_trrSelect.js
browser/extensions/doh-rollout/test/browser/browser_userInterference.js
browser/extensions/doh-rollout/test/browser/head.js
--- a/browser/extensions/doh-rollout/background.js
+++ b/browser/extensions/doh-rollout/background.js
@@ -511,18 +511,18 @@ const rollout = {
       await this.enterprisePolicyCheck("startup", results);
     }
 
     if (!(await stateManager.shouldRunHeuristics())) {
       return;
     }
 
     // Perform TRR selection before running heuristics.
-    await browser.experiments.trrselect.dryRun();
-    log("TRR selection dry run complete!");
+    await browser.experiments.trrselect.run();
+    log("TRR selection complete!");
 
     let networkStatus = (await browser.networkStatus.getLinkInfo()).status;
     let captiveState = "unknown";
     try {
       captiveState = await browser.captivePortal.getState();
     } catch (e) {
       // Captive Portal Service is disabled.
     }
--- a/browser/extensions/doh-rollout/experiments/trrselect/api.js
+++ b/browser/extensions/doh-rollout/experiments/trrselect/api.js
@@ -2,32 +2,45 @@
  * 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/. */
 
 "use strict";
 
 /* global Cc, Ci, ExtensionAPI, TRRRacer  */
 
 ChromeUtils.import("resource://gre/modules/Services.jsm", this);
-ChromeUtils.import("resource:///modules/TRRPerformance.jsm", this);
 
+const kCommitSelectionPref = "doh-rollout.trr-selection.commit-result";
 const kDryRunResultPref = "doh-rollout.trr-selection.dry-run-result";
+const kRolloutURIPref = "doh-rollout.uri";
+const kTRRListPref = "network.trr.resolvers";
 
 const TRRSELECT_TELEMETRY_CATEGORY = "security.doh.trrPerformance";
 
 Services.telemetry.setEventRecordingEnabled(TRRSELECT_TELEMETRY_CATEGORY, true);
 
 this.trrselect = class trrselect extends ExtensionAPI {
   getAPI() {
     return {
       experiments: {
         trrselect: {
           async dryRun() {
             if (Services.prefs.prefHasUserValue(kDryRunResultPref)) {
-              return;
+              // Check whether the existing dry-run-result is in the default
+              // list of TRRs. If it is, all good. Else, run the dry run again.
+              let dryRunResult = Services.prefs.getCharPref(kDryRunResultPref);
+              let defaultTRRs = JSON.parse(
+                Services.prefs.getDefaultBranch("").getCharPref(kTRRListPref)
+              );
+              let dryRunResultIsValid = defaultTRRs.some(
+                trr => trr.url == dryRunResult
+              );
+              if (dryRunResultIsValid) {
+                return;
+              }
             }
 
             let setDryRunResultAndRecordTelemetry = trr => {
               Services.prefs.setCharPref(kDryRunResultPref, trr);
               Services.telemetry.recordEvent(
                 TRRSELECT_TELEMETRY_CATEGORY,
                 "trrselect",
                 "dryrunresult",
@@ -37,21 +50,45 @@ this.trrselect = class trrselect extends
 
             if (Cu.isInAutomation) {
               // For mochitests, just record telemetry with a dummy result.
               // TRRPerformance.jsm is tested in xpcshell.
               setDryRunResultAndRecordTelemetry("dummyTRR");
               return;
             }
 
+            // Importing the module here saves us from having to do it at add-on
+            // startup, and ensures tests have time to set prefs before the
+            // module initializes.
+            let { TRRRacer } = ChromeUtils.import(
+              "resource:///modules/TRRPerformance.jsm"
+            );
             await new Promise(resolve => {
               let racer = new TRRRacer(() => {
                 setDryRunResultAndRecordTelemetry(racer.getFastestTRR(true));
                 resolve();
               });
               racer.run();
             });
           },
+
+          async run() {
+            if (Services.prefs.prefHasUserValue(kRolloutURIPref)) {
+              return;
+            }
+
+            await this.dryRun();
+
+            // If persisting the selection is disabled, don't commit the value.
+            if (!Services.prefs.getBoolPref(kCommitSelectionPref, false)) {
+              return;
+            }
+
+            Services.prefs.setCharPref(
+              kRolloutURIPref,
+              Services.prefs.getCharPref(kDryRunResultPref)
+            );
+          },
         },
       },
     };
   }
 };
--- a/browser/extensions/doh-rollout/experiments/trrselect/schema.json
+++ b/browser/extensions/doh-rollout/experiments/trrselect/schema.json
@@ -1,15 +1,15 @@
 [
   {
     "namespace": "experiments.trrselect",
     "description": "API for running TRR performance measurement",
     "functions": [
       {
-        "name": "dryRun",
+        "name": "run",
         "type": "function",
-        "description": "Runs TRR performance measurement and stores fastest TRR in a pref",
+        "description": "Runs TRR performance measurement if necessary and commits best TRR for the client",
         "parameters": [],
         "async": true
       }
     ]
   }
 ]
--- a/browser/extensions/doh-rollout/test/browser/browser.ini
+++ b/browser/extensions/doh-rollout/test/browser/browser.ini
@@ -5,10 +5,11 @@ skip-if = debug # Bug 1548006 - reloadin
 [browser_cleanFlow.js]
 [browser_dirtyEnable.js]
 [browser_doorhangerUserReject.js]
 [browser_doorhanger_newProfile.js]
 [browser_policyOverride.js]
 skip-if = (!debug && bits == 64) #Bug 1605297
 [browser_rollback.js]
 skip-if = asan && !debug && os == 'linux' && bits == 64 && os_version == '18.04' # Bug 1613994 for linux1804
+[browser_trrSelect.js]
 [browser_userInterference.js]
 skip-if = asan && !debug && os == 'linux' && bits == 64 && os_version == '18.04' # Bug 1615815 for linux1804
--- a/browser/extensions/doh-rollout/test/browser/browser_cleanFlow.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_cleanFlow.js
@@ -8,19 +8,19 @@ add_task(async function testCleanFlow() 
   let promise = waitForDoorhanger();
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await BrowserTestUtils.waitForCondition(() => {
     return Preferences.get(prefs.DOH_SELF_ENABLED_PREF);
   });
   is(Preferences.get(prefs.DOH_SELF_ENABLED_PREF), true, "Breadcrumb saved.");
   is(
-    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "dummyTRR",
-    "TRR selection dry run complete."
+    "TRR selection complete."
   );
   await checkTRRSelectionTelemetry();
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, EXAMPLE_URL);
   let panel = await promise;
   is(
     Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF),
     undefined,
--- a/browser/extensions/doh-rollout/test/browser/browser_dirtyEnable.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_dirtyEnable.js
@@ -17,19 +17,19 @@ add_task(async function testDirtyEnable(
     "Disabled state recorded."
   );
   is(
     Preferences.get(prefs.DOH_SELF_ENABLED_PREF),
     undefined,
     "Breadcrumb not saved."
   );
   is(
-    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     undefined,
-    "TRR selection dry run not performed."
+    "TRR selection not performed."
   );
   ensureNoTRRSelectionTelemetry();
   await ensureNoTRRModeChange(2);
   checkHeuristicsTelemetry("prefHasUserValue", "first_run");
 
   // Simulate a network change.
   simulateNetworkChange();
   await ensureNoTRRModeChange(2);
--- a/browser/extensions/doh-rollout/test/browser/browser_doorhangerUserReject.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_doorhangerUserReject.js
@@ -8,19 +8,19 @@ add_task(async function testDoorhangerUs
   let promise = waitForDoorhanger();
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await BrowserTestUtils.waitForCondition(() => {
     return Preferences.get(prefs.DOH_SELF_ENABLED_PREF);
   });
   is(Preferences.get(prefs.DOH_SELF_ENABLED_PREF), true, "Breadcrumb saved.");
   is(
-    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "dummyTRR",
-    "TRR selection dry run complete."
+    "TRR selection complete."
   );
   await checkTRRSelectionTelemetry();
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, EXAMPLE_URL);
   let panel = await promise;
   is(
     Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF),
     undefined,
--- a/browser/extensions/doh-rollout/test/browser/browser_doorhanger_newProfile.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_doorhanger_newProfile.js
@@ -8,19 +8,19 @@ add_task(async function testDoorhanger()
   setPassingHeuristics();
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await BrowserTestUtils.waitForCondition(() => {
     return Preferences.get(prefs.DOH_SELF_ENABLED_PREF);
   });
   is(Preferences.get(prefs.DOH_SELF_ENABLED_PREF), true, "Breadcrumb saved.");
   is(
-    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "dummyTRR",
-    "TRR selection dry run complete."
+    "TRR selection complete."
   );
   await checkTRRSelectionTelemetry();
 
   await BrowserTestUtils.waitForCondition(() => {
     return Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF);
   });
   is(
     Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF),
--- a/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js
@@ -33,19 +33,19 @@ add_task(async function testPolicyOverri
     "skipHeuristicsCheck pref is set to remember not to run heuristics."
   );
   is(
     Preferences.get(prefs.DOH_SELF_ENABLED_PREF),
     undefined,
     "Breadcrumb not saved."
   );
   is(
-    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     undefined,
-    "TRR selection dry run not performed."
+    "TRR selection not performed."
   );
   ensureNoTRRSelectionTelemetry();
   await ensureNoTRRModeChange(0);
   await checkHeuristicsTelemetry("policy_without_doh", "first_run");
 
   // Simulate a network change.
   simulateNetworkChange();
   await ensureNoTRRModeChange(0);
--- a/browser/extensions/doh-rollout/test/browser/browser_rollback.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_rollback.js
@@ -10,19 +10,19 @@ add_task(async function testRollback() {
   let promise = waitForDoorhanger();
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await BrowserTestUtils.waitForCondition(() => {
     return Preferences.get(prefs.DOH_SELF_ENABLED_PREF);
   });
   is(Preferences.get(prefs.DOH_SELF_ENABLED_PREF), true, "Breadcrumb saved.");
   is(
-    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "dummyTRR",
-    "TRR selection dry run complete."
+    "TRR selection complete."
   );
   await checkTRRSelectionTelemetry();
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, EXAMPLE_URL);
   let panel = await promise;
   is(
     Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF),
     undefined,
new file mode 100644
--- /dev/null
+++ b/browser/extensions/doh-rollout/test/browser/browser_trrSelect.js
@@ -0,0 +1,119 @@
+"use strict";
+
+add_task(setup);
+
+add_task(async function testTRRSelect() {
+  // Set up the resolver lists in the default and user pref branches.
+  // dummyTRR3 which only exists in the user-branch value should be ignored.
+  let oldResolverList = Services.prefs.getCharPref("network.trr.resolvers");
+  Services.prefs
+    .getDefaultBranch("")
+    .setCharPref(
+      "network.trr.resolvers",
+      `[{"url": "dummyTRR"}, {"url": "dummyTRR2"}]`
+    );
+  Services.prefs.setCharPref(
+    "network.trr.resolvers",
+    `[{"url": "dummyTRR"}, {"url": "dummyTRR2"}, {"url": "dummyTRR3"}]`
+  );
+
+  // Clean start: doh-rollout.uri should be set after init.
+  setPassingHeuristics();
+  Preferences.set(prefs.DOH_ENABLED_PREF, true);
+  await BrowserTestUtils.waitForCondition(() => {
+    return Preferences.get(prefs.DOH_SELF_ENABLED_PREF);
+  });
+  is(Preferences.get(prefs.DOH_SELF_ENABLED_PREF), true, "Breadcrumb saved.");
+  is(
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
+    "dummyTRR",
+    "TRR selection complete."
+  );
+
+  // Wait for heuristics to complete.
+  await ensureTRRMode(2);
+  await checkHeuristicsTelemetry("enable_doh", "startup");
+
+  // 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);
+  await restartAddon();
+  await BrowserTestUtils.waitForCondition(() => {
+    return Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF);
+  });
+  is(
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
+    "dummyTRR",
+    "TRR selection complete."
+  );
+
+  // Wait for heuristics to complete.
+  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.
+  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(() => {
+      return Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF);
+    });
+    ok(false, "Dry run result got committed, fail!");
+  } catch (e) {
+    ok(true, "Dry run result did not get committed");
+  }
+  is(
+    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    "dummyTRR",
+    "TRR selection complete, dry-run result recorded."
+  );
+  Preferences.set(prefs.DOH_TRR_SELECT_COMMIT_PREF, true);
+
+  // Wait for heuristics to complete.
+  await ensureTRRMode(2);
+  await checkHeuristicsTelemetry("enable_doh", "startup");
+
+  // Reset doh-rollout.uri, and change the dry-run-result to another one on the
+  // default list. After init, the existing dry-run-result should be committed.
+  Preferences.reset(prefs.DOH_TRR_SELECT_URI_PREF);
+  Preferences.set(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF, "dummyTRR2");
+  await restartAddon();
+  await BrowserTestUtils.waitForCondition(() => {
+    return Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF);
+  });
+  is(
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
+    "dummyTRR2",
+    "TRR selection complete, existing dry-run-result committed."
+  );
+
+  // Wait for heuristics to complete.
+  await ensureTRRMode(2);
+  await checkHeuristicsTelemetry("enable_doh", "startup");
+
+  // Reset doh-rollout.uri, and change the dry-run-result to another one NOT on
+  // default list. After init, a new TRR should be selected and committed.
+  Preferences.reset(prefs.DOH_TRR_SELECT_URI_PREF);
+  Preferences.set(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF, "dummyTRR3");
+  await restartAddon();
+  await BrowserTestUtils.waitForCondition(() => {
+    return Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF);
+  });
+  is(
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
+    "dummyTRR",
+    "TRR selection complete, existing dry-run-result discarded and refreshed."
+  );
+
+  // Wait for heuristics to complete.
+  await ensureTRRMode(2);
+  await checkHeuristicsTelemetry("enable_doh", "startup");
+
+  Services.prefs
+    .getDefaultBranch("")
+    .setCharPref("network.trr.resolvers", oldResolverList);
+  Services.prefs.clearUserPref("network.trr.resolvers");
+});
--- a/browser/extensions/doh-rollout/test/browser/browser_userInterference.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_userInterference.js
@@ -8,19 +8,19 @@ add_task(async function testUserInterfer
   let promise = waitForDoorhanger();
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await BrowserTestUtils.waitForCondition(() => {
     return Preferences.get(prefs.DOH_SELF_ENABLED_PREF);
   });
   is(Preferences.get(prefs.DOH_SELF_ENABLED_PREF), true, "Breadcrumb saved.");
   is(
-    Preferences.get(prefs.DOH_TRR_SELECT_DRY_RUN_RESULT_PREF),
+    Preferences.get(prefs.DOH_TRR_SELECT_URI_PREF),
     "dummyTRR",
-    "TRR selection dry run complete."
+    "TRR selection complete."
   );
   await checkTRRSelectionTelemetry();
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, EXAMPLE_URL);
   let panel = await promise;
   is(
     Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF),
     undefined,
--- a/browser/extensions/doh-rollout/test/browser/head.js
+++ b/browser/extensions/doh-rollout/test/browser/head.js
@@ -27,16 +27,18 @@ const prefs = {
   DOH_PREVIOUS_TRR_MODE_PREF: "doh-rollout.previous.trr.mode",
   DOH_DOORHANGER_SHOWN_PREF: "doh-rollout.doorhanger-shown",
   DOH_DOORHANGER_USER_DECISION_PREF: "doh-rollout.doorhanger-decision",
   DOH_DISABLED_PREF: "doh-rollout.disable-heuristics",
   DOH_SKIP_HEURISTICS_PREF: "doh-rollout.skipHeuristicsCheck",
   DOH_DONE_FIRST_RUN_PREF: "doh-rollout.doneFirstRun",
   DOH_BALROG_MIGRATION_PREF: "doh-rollout.balrog-migration-done",
   DOH_DEBUG_PREF: "doh-rollout.debug",
+  DOH_TRR_SELECT_URI_PREF: "doh-rollout.uri",
+  DOH_TRR_SELECT_COMMIT_PREF: "doh-rollout.trr-selection.commit-result",
   DOH_TRR_SELECT_DRY_RUN_RESULT_PREF:
     "doh-rollout.trr-selection.dry-run-result",
   MOCK_HEURISTICS_PREF: "doh-rollout.heuristics.mockValues",
   PROFILE_CREATION_THRESHOLD_PREF: "doh-rollout.profileCreationThreshold",
 };
 
 const fakePassingHeuristics = JSON.stringify({
   google: "enable_doh",
@@ -68,16 +70,20 @@ async function setup() {
   Services.telemetry.canRecordExtended = true;
   Services.telemetry.clearEvents();
 
   // Set the profile creation threshold to very far in the future by defualt,
   // so that we can test the doorhanger. browser_doorhanger_newProfile.js
   // overrides this.
   Preferences.set(prefs.PROFILE_CREATION_THRESHOLD_PREF, "99999999999999");
 
+  // Enable committing the TRR selection. This pref ships false by default so
+  // it can be controlled e.g. via Normandy, but for testing let's set enable.
+  Preferences.set(prefs.DOH_TRR_SELECT_COMMIT_PREF, true);
+
   registerCleanupFunction(async () => {
     Services.telemetry.canRecordExtended = oldCanRecord;
     Services.telemetry.clearEvents();
     await resetPrefsAndRestartAddon();
   });
 }
 
 async function checkTRRSelectionTelemetry() {