Bug 1548878 - Add telemetry for when a generated password is first filled. r=MattN
authorSam Foster <sfoster@mozilla.com>
Fri, 28 Jun 2019 18:12:24 +0000
changeset 543424 292c5fa99131a701ac56e1a055649a3fd160b0f9
parent 543423 8776353e9c153cc21048d0760449aa0843a1f4fe
child 543425 2086a08e77feeeaddcff0399deef434ac2e8c17e
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1548878
milestone69.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 1548878 - Add telemetry for when a generated password is first filled. r=MattN * recordEvent for telemetry from the parent process when a generated password is filled via the context menu * Extend existing mochitest-plain test to verify the expected telemetry events are recorded * Add a filled flag to the generated password we store in the per-origin cache - And use it to distinguish a first-fill from a subsequent re-use of each generated password Differential Revision: https://phabricator.services.mozilla.com/D33364
toolkit/components/passwordmgr/LoginManagerParent.jsm
toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js
toolkit/components/passwordmgr/test/mochitest/pwmgr_common_parent.js
toolkit/components/passwordmgr/test/mochitest/test_autocomplete_new_password.html
toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_getGeneratedPassword.js
toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_onGeneratedPasswordFilled.js
toolkit/components/telemetry/Events.yaml
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -27,18 +27,18 @@ XPCOMUtils.defineLazyGetter(this, "log",
 
 XPCOMUtils.defineLazyPreferenceGetter(this, "INCLUDE_OTHER_SUBDOMAINS_IN_LOOKUP",
                                       "signon.includeOtherSubdomainsInLookup", false);
 
 const EXPORTED_SYMBOLS = [ "LoginManagerParent" ];
 
 this.LoginManagerParent = {
   /**
-   * A map of a principal's origin (including suffixes) to a generated password string so that we
-   * can offer the same password later (e.g. in a confirmation field).
+   * A map of a principal's origin (including suffixes) to a generated password string and filled flag
+   * so that we can offer the same password later (e.g. in a confirmation field).
    *
    * We don't currently evict from this cache so entries should last until the end of the browser
    * session. That may change later but for now a typical session would max out at a few entries.
    */
   _generatedPasswordsByPrincipalOrigin: new Map(),
 
   /**
    * Reference to the default LoginRecipesParent (instead of the initialization promise) for
@@ -386,21 +386,25 @@ this.LoginManagerParent = {
     if (!browsingContext) {
       return null;
     }
     let framePrincipalOrigin = browsingContext.currentWindowGlobal.documentPrincipal.origin;
     // Use the same password if we already generated one for this origin so that it doesn't change
     // with each search/keystroke and the user can easily re-enter a password in a confirmation field.
     let generatedPW = this._generatedPasswordsByPrincipalOrigin.get(framePrincipalOrigin);
     if (generatedPW) {
-      return generatedPW;
+      return generatedPW.value;
     }
-    generatedPW = PasswordGenerator.generatePassword();
+
+    generatedPW = {
+      value: PasswordGenerator.generatePassword(),
+      filled: false,
+    };
     this._generatedPasswordsByPrincipalOrigin.set(framePrincipalOrigin, generatedPW);
-    return generatedPW;
+    return generatedPW.value;
   },
 
   onFormSubmit(browser, {
     origin,
     formActionOrigin,
     autoFilledLoginGuid,
     usernameField,
     newPasswordField,
@@ -575,16 +579,26 @@ this.LoginManagerParent = {
     let {originNoSuffix} = browsingContext.currentWindowGlobal.documentPrincipal;
     let formOrigin = LoginHelper.getLoginOrigin(originNoSuffix);
     if (!formOrigin) {
       log("_onGeneratedPasswordFilled: Invalid form origin:",
           browsingContext.currentWindowGlobal.documentPrincipal);
       return;
     }
 
+    let framePrincipalOrigin = browsingContext.currentWindowGlobal.documentPrincipal.origin;
+    let generatedPW = this._generatedPasswordsByPrincipalOrigin.get(framePrincipalOrigin);
+    // This will throw if we can't look up the entry in the password/origin map
+    if (!generatedPW.filled) {
+      // record first use of this generated password
+      Services.telemetry.recordEvent("pwmgr", "autocomplete_field", "generatedpassword");
+      log("autocomplete_field telemetry event recorded");
+      generatedPW.filled = true;
+    }
+
     if (!Services.logins.getLoginSavingEnabled(formOrigin)) {
       log("_onGeneratedPasswordFilled: saving is disabled for:", formOrigin);
       return;
     }
 
     // Check if we already have a login saved for this site since we don't want to overwrite it in
     // case the user still needs their old password to succesffully complete a password change.
     // An empty formActionOrigin is used as a wildcard to not restrict to action matches.
@@ -594,17 +608,17 @@ this.LoginManagerParent = {
       ignoreActionAndRealm: false,
     });
 
     if (logins.length > 0) {
       log("_onGeneratedPasswordFilled: Login already saved for this site");
       return;
     }
 
-    let password = this.getGeneratedPassword(browsingContextId);
+    let password = generatedPW.value;
     let formLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
     formLogin.init(formOrigin, formActionOrigin, null, "", password);
     Services.logins.addLogin(formLogin);
   },
 
   /**
    * Maps all the <browser> elements for tabs in the parent process to the
    * current state used to display tab-specific UI.
--- a/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js
+++ b/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js
@@ -234,16 +234,27 @@ function promiseFormsProcessed(expectedC
         SpecialPowers.removeObserver(onProcessedForm, "passwordmgr-processed-form");
         resolve(SpecialPowers.Cu.waiveXrays(subject), data);
       }
     }
     SpecialPowers.addObserver(onProcessedForm, "passwordmgr-processed-form");
   });
 }
 
+function getTelemetryEvents(options) {
+  return new Promise(resolve => {
+    PWMGR_COMMON_PARENT.addMessageListener("getTelemetryEvents", function gotResult(events) {
+      info("CONTENT: getTelemetryEvents gotResult: " + JSON.stringify(events));
+      PWMGR_COMMON_PARENT.removeMessageListener("getTelemetryEvents", gotResult);
+      resolve(events);
+    });
+    PWMGR_COMMON_PARENT.sendAsyncMessage("getTelemetryEvents", options);
+  });
+}
+
 function loadRecipes(recipes) {
   info("Loading recipes");
   return new Promise(resolve => {
     PWMGR_COMMON_PARENT.addMessageListener("loadedRecipes", function loaded() {
       PWMGR_COMMON_PARENT.removeMessageListener("loadedRecipes", loaded);
       resolve(recipes);
     });
     PWMGR_COMMON_PARENT.sendAsyncMessage("loadRecipes", recipes);
--- a/toolkit/components/passwordmgr/test/mochitest/pwmgr_common_parent.js
+++ b/toolkit/components/passwordmgr/test/mochitest/pwmgr_common_parent.js
@@ -118,16 +118,39 @@ addMessageListener("loadRecipes", async 
 });
 
 addMessageListener("resetRecipes", async function() {
   let recipeParent = await LoginManagerParent.recipeParentPromise;
   await recipeParent.reset();
   sendAsyncMessage("recipesReset");
 });
 
+addMessageListener("getTelemetryEvents", options => {
+  options = Object.assign({
+    filterProps: {},
+    clear: false,
+  }, options);
+  let snapshots = Services.telemetry.snapshotEvents(Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS, options.clear);
+  let events = (options.process in snapshots) ? snapshots[options.process] : [];
+
+  // event is array of values like: [22476,"pwmgr","autocomplete_field","generatedpassword"]
+  let keys = ["id", "category", "method", "object", "value"];
+  events = events.filter(entry => {
+    for (let idx = 0; idx < keys.length; idx++) {
+      let key = keys[idx];
+      if ((key in options.filterProps) && options.filterProps[key] !== entry[idx]) {
+        return false;
+      }
+    }
+    return true;
+  });
+  sendAsyncMessage("getTelemetryEvents", events);
+});
+
+
 addMessageListener("proxyLoginManager", msg => {
   // Recreate nsILoginInfo objects from vanilla JS objects.
   let recreatedArgs = msg.args.map((arg, index) => {
     if (msg.loginInfoIndices.includes(index)) {
       return LoginHelper.vanillaObjectToLogin(arg);
     }
 
     return arg;
--- a/toolkit/components/passwordmgr/test/mochitest/test_autocomplete_new_password.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_autocomplete_new_password.html
@@ -14,19 +14,25 @@ Login Manager test: autofill with autoco
 
 <script>
 function initLogins() {
   const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
   Services.logins.removeAllLogins();
   let login1 = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
   login1.init("https://example.com", "https://autofill", null, "user1", "pass1");
 
+  const {LoginManagerParent} = ChromeUtils.import("resource://gre/modules/LoginManagerParent.jsm");
+  Services.telemetry.clearEvents();
+  if (LoginManagerParent._generatedPasswordsByPrincipalOrigin) {
+    LoginManagerParent._generatedPasswordsByPrincipalOrigin.clear();
+  }
   Services.logins.addLogin(login1);
 }
-let chromeScript = runInParent(initLogins);
+
+runInParent(initLogins);
 
 let readyPromise = registerRunTests();
 </script>
 <p id="display"></p>
 
 <!-- we presumably can't hide the content for this test. -->
 <div id="content">
 
@@ -44,24 +50,49 @@ let readyPromise = registerRunTests();
     <button type="submit">Submit</button>
   </form>
 </div>
 
 <pre id="test">
 <script class="testbody" type="text/javascript">
 const {ContentTaskUtils} =
   SpecialPowers.Cu.import("resource://testing-common/ContentTaskUtils.jsm", {});
+const { TestUtils } = SpecialPowers.Cu.import("resource://testing-common/TestUtils.jsm");
 
 let dateAndTimeFormatter = new SpecialPowers.Services.intl.DateTimeFormat(undefined, {
   dateStyle: "medium",
 });
 
+const TelemetryFilterProps = Object.freeze({
+  category: "pwmgr",
+  method: "autocomplete_field",
+  object: "generatedpassword",
+});
+
+async function waitForTelemetryEventsCondition(cond, options = {},
+                                               errorMsg = "waitForTelemetryEventsCondition timed out", maxTries = 200) {
+  return TestUtils.waitForCondition(async () => {
+    let events = await getTelemetryEvents(options);
+    let result;
+    try {
+      result = cond(events);
+      info("waitForTelemetryEventsCondition, result: " + result);
+    } catch (e) {
+      info("waitForTelemetryEventsCondition caught exception, got events: " + JSON.stringify(events));
+      ok(false, `${e}\n${e.stack}`);
+    }
+    return result ? events : null;
+  }, errorMsg, maxTries);
+}
+
 add_task(async function setup() {
   ok(readyPromise, "check promise is available");
   await readyPromise;
+  let events = await getTelemetryEvents({ process: "parent", filterProps: TelemetryFilterProps, clear: true });
+  is(events.length, 0, "Expect 0 events");
 });
 
 add_task(async function test_autofillAutocompleteUsername_noGeneration() {
   // reference form was filled as expected?
   checkForm(1, "user1", "pass1");
 
   // 2nd form should not be filled
   checkForm(2, "", "");
@@ -168,20 +199,28 @@ add_task(async function test_autofillAut
   results = await shownPromise;
   expectedACLabels = [
     "Use Generated Password",
   ];
   checkAutoCompleteResults(results, expectedACLabels, "example.com", "Check all rows are correct");
   synthesizeKey("KEY_ArrowDown");
   let storageAddPromise = promiseStorageChanged(["addLogin"]);
   synthesizeKey("KEY_Enter");
+
+  info("waiting for the password field to be filled with the generatedpassword");
   await SimpleTest.promiseWaitForCondition(() => !!pword.value, "Check generated pw filled");
   info("Wait for generated password to be added to storage");
   await storageAddPromise;
 
+  let expectedCount = 1;
+  info(`filled generated password, check there are now ${expectedCount} generatedpassword telemetry events`);
+  await waitForTelemetryEventsCondition(events => {
+    return events.length == expectedCount;
+  }, { process: "parent", filterProps: TelemetryFilterProps }, `Wait for there to be ${expectedCount} telemetry events`);
+
   let logins = LoginManager.getAllLogins();
   let timePasswordChanged = logins[logins.length - 1].timePasswordChanged;
   let time = dateAndTimeFormatter.format(new Date(timePasswordChanged));
   const LABEL_NO_USERNAME = "No username (" + time + ")";
 
   let generatedPW = pword.value;
   is(generatedPW.length, GENERATED_PASSWORD_LENGTH, "Check generated password length");
   ok(generatedPW.match(GENERATED_PASSWORD_REGEX), "Check generated password format");
@@ -200,16 +239,26 @@ add_task(async function test_autofillAut
   checkAutoCompleteResults(results, expectedACLabels, "example.com", "Check all rows are correct");
   synthesizeKey("KEY_ArrowDown");
   synthesizeKey("KEY_ArrowDown");
   synthesizeKey("KEY_Enter");
   await SimpleTest.promiseWaitForCondition(() => !!pword.value, "Check generated pw filled");
   // Same generated password should be used.
   checkForm(2, "", generatedPW);
 
+  info("filled generated password again, ensure we don't record another generatedpassword autocomplete telemetry event");
+  let telemetryEvents;
+  expectedCount = 2;
+  try {
+    telemetryEvents = await waitForTelemetryEventsCondition(events => events.length == expectedCount,
+                                                            { process: "parent", filterProps: TelemetryFilterProps },
+                                                            `Wait for there to be ${expectedCount} events`, 50);
+  } catch (ex) {}
+  ok(!telemetryEvents, "Expected to timeout waiting for there to be 2 events");
+
   logins = LoginManager.getAllLogins();
   is(logins.length, 1, "Still 1 login after filling the generated password a 2nd time");
   is(logins[0].timePasswordChanged, timePasswordChanged, "Saved login wasn't changed");
   is(logins[0].password, generatedPW, "Password is the same");
 
   info("reset initial login state with login1");
   runInParent(initLogins);
   info("invalidate the autocomplete cache after updating storage above");
--- a/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_getGeneratedPassword.js
+++ b/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_getGeneratedPassword.js
@@ -28,17 +28,17 @@ add_task(async function test_getGenerate
     };
   });
   ok(LMP._browsingContextGlobal.get(99), "Checking BrowsingContext.get(99) stub");
 
   let password1 = LMP.getGeneratedPassword(99);
   notEqual(password1, null, "Check password was returned");
   equal(password1.length, 15, "Check password length");
   equal(LMP._generatedPasswordsByPrincipalOrigin.size, 1, "1 added to cache");
-  equal(LMP._generatedPasswordsByPrincipalOrigin.get("https://www.example.com^userContextId=6"),
+  equal(LMP._generatedPasswordsByPrincipalOrigin.get("https://www.example.com^userContextId=6").value,
         password1, "Cache key and value");
   let password2 = LMP.getGeneratedPassword(99);
   equal(password1, password2, "Same password should be returned for the same origin");
 
   info("Changing the documentPrincipal to simulate a navigation in the frame");
   LMP._browsingContextGlobal.get.restore();
   sinon.stub(LMP._browsingContextGlobal, "get").withArgs(99).callsFake(() => {
     return {
--- a/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_onGeneratedPasswordFilled.js
+++ b/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_onGeneratedPasswordFilled.js
@@ -34,17 +34,17 @@ add_task(async function test_onGenerated
     };
   });
   ok(LMP._browsingContextGlobal.get(99), "Checking BrowsingContext.get(99) stub");
 
   let password1 = LMP.getGeneratedPassword(99);
   notEqual(password1, null, "Check password was returned");
   equal(password1.length, 15, "Check password length");
   equal(LMP._generatedPasswordsByPrincipalOrigin.size, 1, "1 added to cache");
-  equal(LMP._generatedPasswordsByPrincipalOrigin.get("https://www.example.com^userContextId=6"),
+  equal(LMP._generatedPasswordsByPrincipalOrigin.get("https://www.example.com^userContextId=6").value,
         password1, "Cache key and value");
 
   let storageChangedPromised = TestUtils.topicObserved("passwordmgr-storage-changed",
                                                        (_, data) => data == "addLogin");
 
   LMP._onGeneratedPasswordFilled({
     browsingContextId: 99,
     formActionOrigin: "https://www.mozilla.org",
--- a/toolkit/components/telemetry/Events.yaml
+++ b/toolkit/components/telemetry/Events.yaml
@@ -430,16 +430,27 @@ pwmgr:
     bug_numbers:
       - 1548463
     description: An action was taken on the about:logins page
     expiry_version: "74"
     notification_emails: ["loines@mozilla.com", "passwords-dev@mozilla.org", "jaws@mozilla.com"]
     release_channel_collection: opt-out
     record_in_processes: [content]
 
+  autocomplete_field:
+    objects: ["generatedpassword"]
+    methods: ["autocomplete_field"]
+    description: >
+      Sent the first time each unique generated password is used to fill a login field - i.e. the user selects it from from the autocomplete dropdown on a password input
+    bug_numbers: [1548878]
+    notification_emails: ["loines@mozilla.com", "passwords-dev@mozilla.org", "sfoster@mozilla.com"]
+    record_in_processes: ["main"]
+    release_channel_collection: opt-out
+    expiry_version: never
+
 fxa_avatar_menu:
   click:
     objects: [
       "account_settings",
       "cad",
       "login",
       "send_tab",
       "sync_now",