Bug 1548878 - Add telemetry for when a generated password is first filled. r=MattN
☠☠ backed out by df727b0ee168 ☠ ☠
authorSam Foster <sfoster@mozilla.com>
Thu, 13 Jun 2019 23:57:53 +0000
changeset 478961 323de6d7d42282f98ca8ab1d5be9788d83fa7581
parent 478960 79b12fe97edec0c4c27556ef615c8bf4c21c6fa7
child 478962 5ea8582b48c0a48440aeeea454e0ae1b2176aade
push id88003
push usersfoster@mozilla.com
push dateFri, 14 Jun 2019 22:32:07 +0000
treeherderautoland@323de6d7d422 [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/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
@@ -16,17 +16,18 @@ Login Manager test: autofill with autoco
 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");
 
   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 +45,50 @@ 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 = 50) {
+  return TestUtils.waitForCondition(async () => {
+    let events = await getTelemetryEvents(options);
+    info("waitForTelemetryEventsCondition, got events: " + JSON.stringify(events));
+    let result;
+    try {
+      result = cond(events);
+    } catch (e) {
+      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;
+  info("setup: waiting for 0 telemetry events");
+  await waitForTelemetryEventsCondition(events => events && events.length == 0,
+                                        { process: "parent", filterProps: TelemetryFilterProps },
+                                        "Wait for 0 telemetry 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 +195,27 @@ 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;
 
+  info("filled generated password, check 1 generatedpassword telemetry events");
+  await waitForTelemetryEventsCondition(events => events.length == 1,
+                                        { process: "parent", filterProps: TelemetryFilterProps },
+                                        "Wait for there to be 1 telemetry event");
+
   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 +234,25 @@ 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;
+  try {
+    telemetryEvents = await waitForTelemetryEventsCondition(events => events.length == 2,
+                                                            { process: "parent", filterProps: TelemetryFilterProps },
+                                                            "Wait to see if we get 2 telemetry events", 20);
+  } 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/telemetry/Events.yaml
+++ b/toolkit/components/telemetry/Events.yaml
@@ -418,16 +418,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",