Bug 1638502 - [DoH] Persist TRR-selection dry-run result. r=valentin,johannh
authorNihanth Subramanya <nhnt11@gmail.com>
Tue, 26 May 2020 15:43:30 +0000
changeset 532191 a192fd4ed14b35c6a6f56c1e528abaefe5cad6b1
parent 532190 795f01419fc5492ca005234d3a36b288f4199055
child 532192 7c7cebdd58fce2dff64834f00deb6dec2e118d46
push id37451
push usercsabou@mozilla.com
push dateTue, 26 May 2020 21:37:52 +0000
treeherdermozilla-central@e803948bb3cd [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
@@ -7,19 +7,19 @@ add_task(async function testCleanFlow() 
   setPassingHeuristics();
   let promise = waitForDoorhanger();
   let prefPromise = TestUtils.waitForPrefChange(prefs.DOH_SELF_ENABLED_PREF);
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await prefPromise;
   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
@@ -16,19 +16,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
@@ -7,19 +7,19 @@ add_task(async function testDoorhangerUs
   setPassingHeuristics();
   let promise = waitForDoorhanger();
   let prefPromise = TestUtils.waitForPrefChange(prefs.DOH_SELF_ENABLED_PREF);
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await prefPromise;
   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
@@ -10,19 +10,19 @@ add_task(async function testDoorhanger()
   let doorhangerPrefPromise = TestUtils.waitForPrefChange(
     prefs.DOH_DOORHANGER_SHOWN_PREF
   );
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await prefPromise;
   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 doorhangerPrefPromise;
   is(
     Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF),
     true,
     "Doorhanger shown pref saved."
--- a/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js
+++ b/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js
@@ -32,19 +32,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
@@ -9,19 +9,19 @@ add_task(async function testRollback() {
   setPassingHeuristics();
   let promise = waitForDoorhanger();
   let prefPromise = TestUtils.waitForPrefChange(prefs.DOH_SELF_ENABLED_PREF);
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await prefPromise;
   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
@@ -7,19 +7,19 @@ add_task(async function testUserInterfer
   setPassingHeuristics();
   let promise = waitForDoorhanger();
   let prefPromise = TestUtils.waitForPrefChange(prefs.DOH_SELF_ENABLED_PREF);
   Preferences.set(prefs.DOH_ENABLED_PREF, true);
 
   await prefPromise;
   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() {