From 62a1db641aeb36a1572b8ae528dd0db671406199 Mon Sep 17 00:00:00 2001
From: Nihanth Subramanya <nhnt11@gmail.com>
Date: Wed, 24 Jun 2020 09:13:28 +0000
Subject: [PATCH] Bug 1630093 - Don't run heuristics until internet
 connectivity has been established. r=valentin

Differential Revision: https://phabricator.services.mozilla.com/D80809
---
 browser/extensions/doh-rollout/background.js  | 21 +++++++++++--------
 .../doh-rollout/test/browser/head.js          |  1 +
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/browser/extensions/doh-rollout/background.js b/browser/extensions/doh-rollout/background.js
index cf3064fb087c..713f4623cbf6 100644
--- a/browser/extensions/doh-rollout/background.js
+++ b/browser/extensions/doh-rollout/background.js
@@ -396,24 +396,27 @@ const rollout = {
     try {
       captiveState = await browser.captivePortal.getState();
     } catch (e) {
-      // Captive Portal Service is disabled.
-    }
-
-    if (captiveState == "locked_portal") {
+      // Captive Portal Service is disabled. Run heuristics optimistically, but
+      // there's a chance the network is unavailable at this point. In that case
+      // we also wouldn't know when the network is back up. Worst case, we don't
+      // enable DoH in this case, but that's better than never enabling it.
+      await rollout.heuristics("netchange");
       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");
+    // The network is up. If we know we are not captive or in an unlocked portal,
+    // run heuristics. If we detect a portal later, we'll run heuristics again
+    // when it's unlocked.
+    if (captiveState == "unlocked_portal" || captiveState == "not_captive") {
+      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") {
+    if (state == "unlocked_portal" || state == "not_captive") {
       await rollout.heuristics("netchange");
     }
   },
diff --git a/browser/extensions/doh-rollout/test/browser/head.js b/browser/extensions/doh-rollout/test/browser/head.js
index 79f6e27832a1..5af8bd9030d6 100644
--- a/browser/extensions/doh-rollout/test/browser/head.js
+++ b/browser/extensions/doh-rollout/test/browser/head.js
@@ -288,6 +288,7 @@ function simulateNetworkChange() {
   // TODO: Implement a mock NetworkLinkService and use it to also simulate
   // network down events.
   Services.obs.notifyObservers(null, "network:link-status-changed", "up");
+  Services.obs.notifyObservers(null, "captive-portal-login-success");
 }
 
 async function ensureTRRMode(mode) {