Bug 1728057 - fix captive portal telemetry, r=nhnt11

Differential Revision: https://phabricator.services.mozilla.com/D123911

browser/base/content/browser-captivePortal.js

@@ -177,11 +177,6 @@ var CaptivePortalWatcher = {
     }
 
     this._showNotification();
-    Services.telemetry.recordEvent(
-      "networking.captive_portal",
-      "login_infobar_shown",
-      "infobar"
-    );
   },
 
   /**
@@ -246,14 +241,6 @@ var CaptivePortalWatcher = {
       gBrowser.removeTab(tab);
     }
     this._captivePortalTab = null;
-
-    if (aSuccess) {
-      Services.telemetry.recordEvent(
-        "networking.captive_portal",
-        "login_successful",
-        "detector"
-      );
-    }
   },
 
   _cancelDelayedCaptivePortal() {
@@ -338,6 +325,12 @@ var CaptivePortalWatcher = {
       closeHandler
     );
 
+    Services.telemetry.recordEvent(
+      "networking.captive_portal",
+      "login_infobar_shown",
+      "infobar"
+    );
+
     gBrowser.tabContainer.addEventListener("TabSelect", this);
   },
 

toolkit/components/captivedetect/CaptiveDetect.jsm

@@ -242,6 +242,7 @@ function CaptivePortalDetector() {
   // Load preference
   this._canonicalSiteURL = null;
   this._canonicalSiteExpectedContent = null;
+  this._telemetryService = Services.telemetry;
 
   try {
     this._canonicalSiteURL = Services.prefs.getCharPref(
@@ -281,7 +282,7 @@ function CaptivePortalDetector() {
   this._requestQueue = []; // Maintain a progress table, store callbacks and the ongoing XHR
   this._interfaceNames = {}; // Maintain names of the requested network interfaces
 
-  Services.telemetry.setEventRecordingEnabled(
+  this._telemetryService.setEventRecordingEnabled(
     "networking.captive_portal",
     true
   );
@@ -442,6 +443,12 @@ CaptivePortalDetector.prototype = {
       // Only when the request has a event id and |success| is true
       // do we need to notify the login-success event.
       if (this._runningRequest.hasOwnProperty("eventId") && success) {
+        this._telemetryService.recordEvent(
+          "networking.captive_portal",
+          "login_successful",
+          "detector"
+        );
+
         let details = {
           type: kCaptivePortalLoginSuccessEvent,
           id: this._runningRequest.eventId,

toolkit/components/captivedetect/test/unit/head_setprefs.js

@@ -7,6 +7,7 @@
 var { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
+const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
 var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 var {
   HTTP_400,
@@ -36,12 +37,9 @@ var {
   HttpServer,
 } = ChromeUtils.import("resource://testing-common/httpd.js");
 
-XPCOMUtils.defineLazyServiceGetter(
-  this,
-  "gCaptivePortalDetector",
-  "@mozilla.org/toolkit/captive-detector;1",
-  "nsICaptivePortalDetector"
-);
+const fakeTelemetryService = {
+  recordEvent: sinon.spy(),
+};
 
 const kCanonicalSitePath = "/canonicalSite.html";
 const kCanonicalSiteContent = "true";
@@ -63,6 +61,7 @@ function setupPrefs() {
   Services.prefs.setIntPref(kPrefsPollingTime, 1);
 }
 
+let gCaptivePortalDetector;
 function run_captivedetect_test(xhr_handler, fakeUIResponse, testfun) {
   gServer = new HttpServer();
   gServer.registerPathHandler(kCanonicalSitePath, xhr_handler);
@@ -71,6 +70,14 @@ function run_captivedetect_test(xhr_handler, fakeUIResponse, testfun) {
 
   setupPrefs();
 
+  // Instead of getting the XPCOM service, we need the real JS object, so that
+  // we can give it a stubbed out telemetry service.
+  const { CaptivePortalDetector } = ChromeUtils.import(
+    "resource:///modules/CaptiveDetect.jsm"
+  );
+  gCaptivePortalDetector = new CaptivePortalDetector();
+  gCaptivePortalDetector._telemetryService = fakeTelemetryService;
+
   fakeUIResponse();
 
   testfun();

toolkit/components/captivedetect/test/unit/test_captive_portal_found.js

@@ -36,6 +36,7 @@ function fakeUIResponse() {
     if (topic === "captive-portal-login-success") {
       Assert.equal(++step, 4);
       gServer.stop(do_test_finished);
+      Assert.ok(fakeTelemetryService.recordEvent.calledOnce);
     }
   }, "captive-portal-login-success");
 }

toolkit/components/captivedetect/test/unit/test_multiple_requests.js

@@ -43,6 +43,7 @@ function fakeUIResponse() {
         );
       }
       Assert.equal(++step, 4);
+      Assert.ok(fakeTelemetryService.recordEvent.calledOnce);
     }
   }, "captive-portal-login-success");
 }