diff --git a/browser/extensions/doh-rollout/background.js b/browser/extensions/doh-rollout/background.js index ee4ea1693839..f034b7281688 100644 --- a/browser/extensions/doh-rollout/background.js +++ b/browser/extensions/doh-rollout/background.js @@ -189,7 +189,7 @@ const stateManager = { return !doorhangerShown; }, - async showDoorHangerAndEnableDoH() { + async showDoorhanger() { browser.experiments.doorhanger.onDoorhangerAccept.addListener( rollout.doorhangerAcceptListener ); @@ -208,10 +208,6 @@ const stateManager = { "doorhangerButtonCancelAccessKey" ), }); - - // By default, enable DoH when showing the doorhanger, - // if heuristics returned no reason to not run. - await stateManager.setState("enabled"); }, }; @@ -246,13 +242,19 @@ const rollout = { }, async heuristics(evaluateReason) { + let shouldRunHeuristics = await stateManager.shouldRunHeuristics(); + + if (!shouldRunHeuristics) { + return; + } + // Run heuristics defined in heuristics.js and experiments/heuristics/api.js let results; if (await rollout.isTesting()) { results = await browser.experiments.preferences.getCharPref( MOCK_HEURISTICS_PREF, - "disable_doh" + `{ "test": "disable_doh" }` ); results = JSON.parse(results); } else { @@ -270,7 +272,14 @@ const rollout = { results.evaluateReason = evaluateReason; browser.experiments.heuristics.sendHeuristicsPing(decision, results); - return decision; + if (decision === "disable_doh") { + await stateManager.setState("disabled"); + } else { + await stateManager.setState("enabled"); + if (await stateManager.shouldShowDoorhanger()) { + await stateManager.showDoorhanger(); + } + } }, async getSetting(name, defaultValue) { @@ -472,81 +481,67 @@ const rollout = { await this.enterprisePolicyCheck("startup", results); } - if (await stateManager.shouldRunHeuristics()) { - await this.runStartupHeuristics(); + if (!(await stateManager.shouldRunHeuristics())) { + return; + } + + let networkStatus = (await browser.networkStatus.getLinkInfo()).status; + let captiveState = "unknown"; + try { + captiveState = await browser.captivePortal.getState(); + } catch (e) { + // Captive Portal Service is disabled. + } + + if (networkStatus == "up" && captiveState != "locked_portal") { + await rollout.heuristics("startup"); } // Listen for network change events to run heuristics again - browser.networkStatus.onConnectionChanged.addListener(async () => { - log("onConnectionChanged"); - - let linkInfo = await browser.networkStatus.getLinkInfo(); - if (linkInfo.status !== "up") { - log("Link down."); - if (rollout.networkSettledTimeout) { - log("Canceling queued heuristics run."); - clearTimeout(rollout.networkSettledTimeout); - rollout.networkSettledTimeout = null; - } - return; - } - - log("Queing a heuristics run in 60s, will cancel if network fluctuates."); - let gracePeriod = (await rollout.isTesting()) ? 0 : 60000; - rollout.networkSettledTimeout = setTimeout(async () => { - log("No network fluctuation for 60 seconds, running heuristics."); - // Only run the heuristics if user hasn't explicitly enabled/disabled DoH - let shouldRunHeuristics = await stateManager.shouldRunHeuristics(); - let shouldShowDoorhanger = await stateManager.shouldShowDoorhanger(); - - if (!shouldRunHeuristics) { - return; - } - - const netChangeDecision = await rollout.heuristics("netChange"); - - if (netChangeDecision === "disable_doh") { - await stateManager.setState("disabled"); - } else if (shouldShowDoorhanger) { - await stateManager.showDoorHangerAndEnableDoH(); - } else { - await stateManager.setState("enabled"); - } - }, gracePeriod); - }); + browser.networkStatus.onConnectionChanged.addListener( + rollout.onConnectionChanged + ); // Listen to the captive portal when it unlocks - browser.captivePortal.onConnectivityAvailable.addListener(async () => { - log("Captive portal onConnectivityAvailable, running heuristics."); - if (rollout.networkSettledTimeout) { - log("Canceling queued heuristics run."); - clearTimeout(rollout.networkSettledTimeout); - rollout.networkSettledTimeout = null; - } - - let shouldRunHeuristics = await stateManager.shouldRunHeuristics(); - - if (!shouldRunHeuristics) { - return; - } - - await this.runStartupHeuristics(); - }); + try { + browser.captivePortal.onStateChange.addListener( + rollout.onCaptiveStateChanged + ); + } catch (e) { + // Captive Portal Service is disabled. + } }, - async runStartupHeuristics() { - let decision = await this.heuristics("startup"); - let shouldShowDoorhanger = await stateManager.shouldShowDoorhanger(); - if (decision === "disable_doh") { - await stateManager.setState("disabled"); + async onConnectionChanged({ status }) { + log("onConnectionChanged", status); - // If the heuristics say to enable DoH, determine if the doorhanger - // should be shown - } else if (shouldShowDoorhanger) { - await stateManager.showDoorHangerAndEnableDoH(); - } else { - // Doorhanger has been shown before and did not opt-out - await stateManager.setState("enabled"); + if (status != "up") { + return; + } + + let captiveState = "unknown"; + try { + captiveState = await browser.captivePortal.getState(); + } catch (e) { + // Captive Portal Service is disabled. + } + + if (captiveState == "locked_portal") { + return; + } + + // The network is up and we don't know that we're in a locked portal. + // Run heuristics. If we detect a portal later, we'll run heuristics again + // when it's unlocked. In that case, this run will likely have failed. + await rollout.heuristics("netchange"); + }, + + async onCaptiveStateChanged({ state }) { + log("onCaptiveStateChanged", state); + // unlocked_portal means we were previously in a locked portal and then + // network access was granted. + if (state == "unlocked_portal") { + await rollout.heuristics("netchange"); } }, }; diff --git a/browser/extensions/doh-rollout/test/browser/browser_cleanFlow.js b/browser/extensions/doh-rollout/test/browser/browser_cleanFlow.js index 2ccfaad5d005..3bdd9d9cb47a 100644 --- a/browser/extensions/doh-rollout/test/browser/browser_cleanFlow.js +++ b/browser/extensions/doh-rollout/test/browser/browser_cleanFlow.js @@ -27,6 +27,7 @@ add_task(async function testCleanFlow() { await promise; await ensureTRRMode(2); + checkHeuristicsTelemetry("enable_doh", "startup"); await BrowserTestUtils.waitForCondition(() => { return Preferences.get(prefs.DOH_DOORHANGER_SHOWN_PREF); @@ -51,24 +52,26 @@ add_task(async function testCleanFlow() { setFailingHeuristics(); simulateNetworkChange(); await ensureTRRMode(0); + checkHeuristicsTelemetry("disable_doh", "netchange"); // Trigger another network change. simulateNetworkChange(); await ensureNoTRRModeChange(0); + checkHeuristicsTelemetry("disable_doh", "netchange"); // Restart the add-on for good measure. await restartAddon(); await ensureNoTRRModeChange(0); + checkHeuristicsTelemetry("disable_doh", "startup"); // Set a passing environment and simulate a network change. setPassingHeuristics(); simulateNetworkChange(); await ensureTRRMode(2); + checkHeuristicsTelemetry("enable_doh", "netchange"); // Again, repeat and check nothing changed. simulateNetworkChange(); await ensureNoTRRModeChange(2); - - // Clean up. - await resetPrefsAndRestartAddon(); + checkHeuristicsTelemetry("enable_doh", "netchange"); }); diff --git a/browser/extensions/doh-rollout/test/browser/browser_dirtyEnable.js b/browser/extensions/doh-rollout/test/browser/browser_dirtyEnable.js index cf5b7a169fa4..87f1d1e475d5 100644 --- a/browser/extensions/doh-rollout/test/browser/browser_dirtyEnable.js +++ b/browser/extensions/doh-rollout/test/browser/browser_dirtyEnable.js @@ -22,19 +22,20 @@ add_task(async function testDirtyEnable() { "Breadcrumb not saved." ); await ensureNoTRRModeChange(2); + checkHeuristicsTelemetry("prefHasUserValue", "first_run"); // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(2); + ensureNoHeuristicsTelemetry(); // Restart for good measure. await restartAddon(); await ensureNoTRRModeChange(2); + ensureNoHeuristicsTelemetry(); // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(2); - - // Clean up. - await resetPrefsAndRestartAddon(); + ensureNoHeuristicsTelemetry(); }); diff --git a/browser/extensions/doh-rollout/test/browser/browser_doorhangerUserReject.js b/browser/extensions/doh-rollout/test/browser/browser_doorhangerUserReject.js index 61dc5a2675cd..cec34a2ef1a0 100644 --- a/browser/extensions/doh-rollout/test/browser/browser_doorhangerUserReject.js +++ b/browser/extensions/doh-rollout/test/browser/browser_doorhangerUserReject.js @@ -21,6 +21,7 @@ add_task(async function testDoorhangerUserReject() { ); await ensureTRRMode(2); + checkHeuristicsTelemetry("enable_doh", "startup"); // Click the doorhanger's "reject" button. let button = panel.querySelector(".popup-notification-secondary-button"); @@ -49,20 +50,21 @@ add_task(async function testDoorhangerUserReject() { ); await ensureTRRMode(5); + checkHeuristicsTelemetry("disable_doh", "doorhangerDecline"); // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(5); + ensureNoHeuristicsTelemetry(); // Restart the add-on for good measure. await restartAddon(); await ensureNoTRRModeChange(5); + ensureNoHeuristicsTelemetry(); // Set failing environment and trigger another network change. setFailingHeuristics(); simulateNetworkChange(); await ensureNoTRRModeChange(5); - - // Clean up. - await resetPrefsAndRestartAddon(); + ensureNoHeuristicsTelemetry(); }); diff --git a/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js b/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js index 0b71b9e50c32..fa74e09abb7c 100644 --- a/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js +++ b/browser/extensions/doh-rollout/test/browser/browser_policyOverride.js @@ -38,25 +38,28 @@ add_task(async function testPolicyOverride() { "Breadcrumb not saved." ); await ensureNoTRRModeChange(0); + checkHeuristicsTelemetry("policy_without_doh", "first_run"); // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(0); + ensureNoHeuristicsTelemetry(); // Restart for good measure. await restartAddon(); await ensureNoTRRModeChange(0); + checkHeuristicsTelemetry("policy_without_doh", "startup"); // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(0); + ensureNoHeuristicsTelemetry(); // Clean up. await EnterprisePolicyTesting.setupPolicyEngineWithJson({ policies: {}, }); EnterprisePolicyTesting.resetRunOnceState(); - await resetPrefsAndRestartAddon(); is( Services.policies.status, diff --git a/browser/extensions/doh-rollout/test/browser/browser_userInterference.js b/browser/extensions/doh-rollout/test/browser/browser_userInterference.js index a8fdbf0d5854..ee1103ddda48 100644 --- a/browser/extensions/doh-rollout/test/browser/browser_userInterference.js +++ b/browser/extensions/doh-rollout/test/browser/browser_userInterference.js @@ -42,6 +42,7 @@ add_task(async function testUserInterference() { ); await ensureTRRMode(2); + checkHeuristicsTelemetry("enable_doh", "startup"); // Set the TRR mode pref manually and ensure we respect this. Preferences.set(prefs.TRR_MODE_PREF, 0); @@ -49,6 +50,7 @@ add_task(async function testUserInterference() { // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(0); + checkHeuristicsTelemetry("disable_doh", "userModified"); is( Preferences.get(prefs.DOH_DISABLED_PREF, false), @@ -64,15 +66,15 @@ add_task(async function testUserInterference() { // Simulate another network change. simulateNetworkChange(); await ensureNoTRRModeChange(0); + ensureNoHeuristicsTelemetry(); // Restart the add-on for good measure. await restartAddon(); await ensureNoTRRModeChange(0); + ensureNoHeuristicsTelemetry(); // Simulate another network change. simulateNetworkChange(); await ensureNoTRRModeChange(0); - - // Clean up. - await resetPrefsAndRestartAddon(); + ensureNoHeuristicsTelemetry(); }); diff --git a/browser/extensions/doh-rollout/test/browser/head.js b/browser/extensions/doh-rollout/test/browser/head.js index 6b990fc7267a..cf3089d427b6 100644 --- a/browser/extensions/doh-rollout/test/browser/head.js +++ b/browser/extensions/doh-rollout/test/browser/head.js @@ -59,12 +59,48 @@ async function setup() { Services.telemetry.canRecordExtended = true; Services.telemetry.clearEvents(); - registerCleanupFunction(() => { + registerCleanupFunction(async () => { Services.telemetry.canRecordExtended = oldCanRecord; Services.telemetry.clearEvents(); + await resetPrefsAndRestartAddon(); }); } +function checkHeuristicsTelemetry(decision, evaluateReason) { + let events = Services.telemetry.snapshotEvents( + Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS + ).dynamic; + events = events.filter( + e => e[1] == "doh" && e[2] == "evaluate" && e[3] == "heuristics" + ); + is(events.length, 1, "Found the expected heuristics event."); + is(events[0][4], decision, "The event records the expected decision"); + if (evaluateReason) { + is(events[0][5].evaluateReason, evaluateReason, "Got the expected reason."); + } + + // After checking the event, clear all telemetry. Since we check for a single + // event above, this ensures all heuristics events are intentional and tested. + // TODO: Test events other than heuristics. Those tests would also work the + // same way, so as to test one event at a time, and this clearEvents() call + // will continue to exist as-is. + Services.telemetry.clearEvents(); +} + +function ensureNoHeuristicsTelemetry() { + let events = Services.telemetry.snapshotEvents( + Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS + ).dynamic; + if (!events) { + ok(true, "Found no heuristics events."); + return; + } + events = events.filter( + e => e[1] == "doh" && e[2] == "evaluate" && e[3] == "heuristics" + ); + is(events.length, 0, "Found no heuristics events."); +} + function setPassingHeuristics() { Preferences.set(prefs.MOCK_HEURISTICS_PREF, fakePassingHeuristics); } @@ -101,7 +137,12 @@ async function waitForDoorhanger() { } function simulateNetworkChange() { - Services.obs.notifyObservers(null, "network:link-status-changed", "down"); + // The networkStatus API does not actually propagate the link status we supply + // here, but rather sends the link status from the NetworkLinkService. + // This means there's no point sending a down and then an up - the extension + // will just receive "up" twice. + // TODO: Implement a mock NetworkLinkService and use it to also simulate + // network down events. Services.obs.notifyObservers(null, "network:link-status-changed", "up"); }