Backed out 3 changesets (bug 1724108) as requested by Dan. CLOSED TREE

Backed out changeset 0bdbea37f84f (bug 1724108)
Backed out changeset 91aa3dd65624 (bug 1724108)
Backed out changeset 5e57386eae33 (bug 1724108)

browser/base/content/browser-captivePortal.js

@@ -26,22 +26,6 @@ var CaptivePortalWatcher = {
   // after successful login if we're redirected to the canonicalURL.
   _previousCaptivePortalTab: null,
 
-  // We will only show the VPN promo if we're pretty sure that the captive
-  // portal "Login" button has been pressed relatively recently
-  // before the captive-portal-login-success notification was received so that
-  // we can have reasonable degree of confidence that the user has some mental
-  // context why when we might be showing them the promo.
-  _loginButtonPressedTimeStamp: 0, // 0 is magic value meaning "not set"
-
-  // Here's where we define "recently" for the button above:
-  //
-  // On the one hand, if for some reason the login doesn't happen (eg user
-  // decides airplane wifi charge is too high), the next time they try to
-  // log into either this or another captive portal, we could end up showing
-  // the VPN promo when it's been a long time since they saw the UI, and
-  // they've lost mental context.
-  _LOGIN_BUTTON_PRESSED_TIMEOUT: 60 * 1000 * 1000, // 60 mins
-
   get _captivePortalNotification() {
     return gNotificationBox.getNotificationWithValue(
       this.PORTAL_NOTIFICATION_VALUE
@@ -64,7 +48,6 @@ var CaptivePortalWatcher = {
     Services.obs.addObserver(this, "captive-portal-login");
     Services.obs.addObserver(this, "captive-portal-login-abort");
     Services.obs.addObserver(this, "captive-portal-login-success");
-    Services.obs.addObserver(this, "captive-portal-login-button-pressed");
 
     this._cps = Cc["@mozilla.org/network/captive-portal-service;1"].getService(
       Ci.nsICaptivePortalService
@@ -100,7 +83,6 @@ var CaptivePortalWatcher = {
     Services.obs.removeObserver(this, "captive-portal-login");
     Services.obs.removeObserver(this, "captive-portal-login-abort");
     Services.obs.removeObserver(this, "captive-portal-login-success");
-    Services.obs.removeObserver(this, "captive-portal-login-button-pressed");
 
     this._cancelDelayedCaptivePortal();
   },
@@ -123,23 +105,8 @@ var CaptivePortalWatcher = {
       case "captive-portal-login-abort":
         this._captivePortalGone(false);
         break;
-      case "captive-portal-login-button-pressed":
-        this._loginButtonPressedTimeStamp = Cu.now();
-        break;
       case "captive-portal-login-success":
         this._captivePortalGone(true);
-
-        if (
-          this._loginButtonPressedTimeStamp &&
-          Cu.now() - this._loginButtonPressedTimeStamp <
-            this._LOGIN_BUTTON_PRESSED_TIMEOUT
-        ) {
-          Services.obs.notifyObservers(
-            null,
-            "captive-portal-login-success-after-button-pressed"
-          );
-          this._loginButtonPressedTimeStamp = 0;
-        }
         break;
       case "delayed-captive-portal-handled":
         this._cancelDelayedCaptivePortal();
@@ -339,11 +306,6 @@ var CaptivePortalWatcher = {
         callback: () => {
           this.ensureCaptivePortalTab();
 
-          Services.obs.notifyObservers(
-            null,
-            "captive-portal-login-button-pressed"
-          );
-
           Services.telemetry.recordEvent(
             "networking.captive_portal",
             "login_button_pressed",

browser/base/content/test/captivePortal/browser_CaptivePortalWatcher_1.js

@@ -51,18 +51,10 @@ let testcases = [
     let button = notification.buttonContainer.querySelector(
       "button.notification-button"
     );
-    async function clickButtonAndExpectNewPortalTabAndNotification() {
-      let newTabCreation = BrowserTestUtils.waitForNewTab(
-        win.gBrowser,
-        CANONICAL_URL
-      );
-      let buttonPressedEvent = TestUtils.topicObserved(
-        "captive-portal-login-button-pressed"
-      );
-
+    async function clickButtonAndExpectNewPortalTab() {
+      let p = BrowserTestUtils.waitForNewTab(win.gBrowser, CANONICAL_URL);
       button.click();
-
-      const [tab] = await Promise.all([newTabCreation, buttonPressedEvent]);
+      let tab = await p;
       is(
         win.gBrowser.selectedTab,
         tab,
@@ -73,7 +65,7 @@ let testcases = [
 
     // Simulate clicking the button. The portal tab should be opened and
     // selected and the button should hide.
-    let tab = await clickButtonAndExpectNewPortalTabAndNotification();
+    let tab = await clickButtonAndExpectNewPortalTab();
     testPortalTabSelectedAndButtonNotVisible();
 
     // Close the tab. The button should become visible.
@@ -83,7 +75,7 @@ let testcases = [
 
     // When the button is clicked, a new portal tab should be opened and
     // selected.
-    tab = await clickButtonAndExpectNewPortalTabAndNotification();
+    tab = await clickButtonAndExpectNewPortalTab();
 
     // Open another arbitrary tab. The button should become visible. When it's clicked,
     // the portal tab should be selected.
@@ -101,7 +93,7 @@ let testcases = [
     BrowserTestUtils.removeTab(tab);
     win.gBrowser.selectedTab = anotherTab;
     testShowLoginPageButtonVisibility(notification, "visible");
-    tab = await clickButtonAndExpectNewPortalTabAndNotification();
+    tab = await clickButtonAndExpectNewPortalTab();
 
     BrowserTestUtils.removeTab(anotherTab);
     await freePortal(true);
@@ -109,67 +101,6 @@ let testcases = [
     ensureNoPortalNotification(win);
     await closeWindowAndWaitForWindowActivate(win);
   },
-  async function testLoginSuccessAfterButtonPress() {
-    // test that button not clicked generates no notification
-    let loginSuccessAfterButtonPress = TestUtils.topicObserved(
-      "captive-portal-login-success-after-button-pressed"
-    );
-    window.CaptivePortalWatcher.observe(null, "captive-portal-login-success");
-
-    let exception = undefined;
-    try {
-      await waitForPromiseWithTimeout(loginSuccessAfterButtonPress);
-    } catch (ex) {
-      exception = ex;
-    }
-    isnot(
-      exception,
-      undefined,
-      "captive-portal-login-success-after-button-pressed should not have" +
-        " been sent, because button was not pressed"
-    );
-
-    // test that button clicked does generate a notification
-    loginSuccessAfterButtonPress = TestUtils.topicObserved(
-      "captive-portal-login-success-after-button-pressed"
-    );
-
-    window.CaptivePortalWatcher.observe(
-      null,
-      "captive-portal-login-button-pressed"
-    );
-    window.CaptivePortalWatcher.observe(null, "captive-portal-login-success");
-
-    await loginSuccessAfterButtonPress;
-
-    // test that subsequent captive-portal-login-success without a click
-    // does not send a notification
-
-    // Normally, we would stub Cu.now() and make it move the clock forward.
-    // Unfortunately, since it's in C++, we can't do that here, so instead
-    // we move the time stamp into the past so that things will have expired.
-    window.CaptivePortalWatcher._loginButtonPressedTimeStamp -=
-      window.CaptivePortalWatcher._LOGIN_BUTTON_PRESSED_TIMEOUT;
-
-    loginSuccessAfterButtonPress = TestUtils.topicObserved(
-      "captive-portal-login-success-after-button-pressed"
-    );
-
-    window.CaptivePortalWatcher.observe(null, "captive-portal-login-success");
-    exception = undefined;
-    try {
-      await waitForPromiseWithTimeout(loginSuccessAfterButtonPress);
-    } catch (ex) {
-      exception = ex;
-    }
-
-    isnot(
-      exception,
-      undefined,
-      "captive-portal-login-success-after-button-pressed should not have" +
-        " been sent, button was not pressed again"
-    );
-  },
 ];
 
 for (let testcase of testcases) {

browser/base/content/test/captivePortal/head.js

@@ -268,15 +268,3 @@ async function openCaptivePortalLoginTab(
 
   return portalTab;
 }
-
-// XXX This likely wants to move to TestUtils, mostly borrowed from
-// browser_doorhanger_form_password_edit.js
-async function waitForPromiseWithTimeout(promise, timeoutMs = 5000) {
-  let timedOut = new Promise((resolve, reject) => {
-    /* eslint-disable-next-line mozilla/no-arbitrary-setTimeout */
-    setTimeout(() => {
-      reject(new Error(`Timed out in ${timeoutMs} ms.`));
-    }, timeoutMs);
-  });
-  await Promise.race([promise, timedOut]);
-}

browser/components/newtab/lib/ASRouterTriggerListeners.jsm

@@ -595,10 +595,7 @@ this.ASRouterTriggerListeners = new Map([
 
       init(triggerHandler) {
         if (!this._initialized) {
-          Services.obs.addObserver(
-            this,
-            "captive-portal-login-success-after-button-pressed"
-          );
+          Services.obs.addObserver(this, "captive-portal-login-success");
           this._initialized = true;
         }
         this._triggerHandler = triggerHandler;
@@ -606,7 +603,7 @@ this.ASRouterTriggerListeners = new Map([
 
       observe(aSubject, aTopic, aData) {
         switch (aTopic) {
-          case "captive-portal-login-success-after-button-pressed":
+          case "captive-portal-login-success":
             const browser = Services.wm.getMostRecentBrowserWindow();
             if (browser) {
               this._triggerHandler(browser.gBrowser.selectedBrowser, {
@@ -621,10 +618,7 @@ this.ASRouterTriggerListeners = new Map([
         if (this._initialized) {
           this._triggerHandler = null;
           this._initialized = false;
-          Services.obs.removeObserver(
-            this,
-            "captive-portal-login-success-after-button-pressed"
-          );
+          Services.obs.removeObserver(this, "captive-portal-login-success");
         }
       },
     },

browser/components/newtab/test/browser/browser_trigger_listeners.js

@@ -72,21 +72,13 @@ add_task(async function test_captivePortalLogin() {
 
   captivePortalTrigger.init(stub);
 
-  Services.obs.notifyObservers(
-    this,
-    "captive-portal-login-success-after-button-pressed",
-    {}
-  );
+  Services.obs.notifyObservers(this, "captive-portal-login-success", {});
 
-  Assert.ok(stub.called, "Called after button press and login");
+  Assert.ok(stub.called, "Called after login event");
 
   captivePortalTrigger.uninit();
 
-  Services.obs.notifyObservers(
-    this,
-    "captive-portal-login-success-after-button-pressed",
-    {}
-  );
+  Services.obs.notifyObservers(this, "captive-portal-login-success", {});
 
   Assert.equal(stub.callCount, 1, "Not called after uninit");
 });