Backed out changeset 678b0803aa1c (bug 1330111)
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Tue, 07 Feb 2017 14:03:17 +0100
changeset 341078 11e702637d4d72f1f9c20ae5988da42a7c821ac9
parent 341077 c5a1171cca7acd698e215862e27d1c3ce0787e5d
child 341079 22ec133c5af9b1449abfe2445fb20bc93379d74f
push id86633
push usercbook@mozilla.com
push dateTue, 07 Feb 2017 13:03:53 +0000
treeherdermozilla-inbound@73401a58d048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1330111
milestone54.0a1
backs out678b0803aa1c11802436cc8f12ff1c4bdb624572
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
Backed out changeset 678b0803aa1c (bug 1330111)
toolkit/components/passwordmgr/LoginManagerContent.jsm
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -917,40 +917,47 @@ var LoginManagerContent = {
    * @param {Set} recipes that could be used to affect how the form is filled
    * @param {Object} [options = {}] is a list of options for this method.
             - [inputElement] is an optional target input element we want to fill
    */
   _fillForm(form, autofillForm, clobberUsername, clobberPassword,
                         userTriggered, foundLogins, recipes, {inputElement} = {}) {
     log("_fillForm", form.elements);
     let ignoreAutocomplete = true;
-    // Will be set to one of AUTOFILL_RESULT in the `try` block.
-    let autofillResult = -1;
     const AUTOFILL_RESULT = {
       FILLED: 0,
       NO_PASSWORD_FIELD: 1,
       PASSWORD_DISABLED_READONLY: 2,
       NO_LOGINS_FIT: 3,
       NO_SAVED_LOGINS: 4,
       EXISTING_PASSWORD: 5,
       EXISTING_USERNAME: 6,
       MULTIPLE_LOGINS: 7,
       NO_AUTOFILL_FORMS: 8,
       AUTOCOMPLETE_OFF: 9,
       INSECURE: 10,
     };
 
+    function recordAutofillResult(result) {
+      if (userTriggered) {
+        // Ignore fills as a result of user action.
+        return;
+      }
+      const autofillResultHist = Services.telemetry.getHistogramById("PWMGR_FORM_AUTOFILL_RESULT");
+      autofillResultHist.add(result);
+    }
+
     try {
       // Nothing to do if we have no matching logins available,
       // and there isn't a need to show the insecure form warning.
       if (foundLogins.length == 0 &&
           (InsecurePasswordUtils.isFormSecure(form) ||
           !LoginHelper.showInsecureFieldWarning)) {
         // We don't log() here since this is a very common case.
-        autofillResult = AUTOFILL_RESULT.NO_SAVED_LOGINS;
+        recordAutofillResult(AUTOFILL_RESULT.NO_SAVED_LOGINS);
         return;
       }
 
       // Heuristically determine what the user/pass fields are
       // We do this before checking to see if logins are stored,
       // so that the user isn't prompted for a master password
       // without need.
       var [usernameField, passwordField, ignored] =
@@ -970,26 +977,26 @@ var LoginManagerContent = {
         } else {
           throw new Error("Unexpected input element type.");
         }
       }
 
       // Need a valid password field to do anything.
       if (passwordField == null) {
         log("not filling form, no password field found");
-        autofillResult = AUTOFILL_RESULT.NO_PASSWORD_FIELD;
+        recordAutofillResult(AUTOFILL_RESULT.NO_PASSWORD_FIELD);
         return;
       }
 
       this._formFillService.markAsLoginManagerField(passwordField);
 
       // If the password field is disabled or read-only, there's nothing to do.
       if (passwordField.disabled || passwordField.readOnly) {
         log("not filling form, password field disabled or read-only");
-        autofillResult = AUTOFILL_RESULT.PASSWORD_DISABLED_READONLY;
+        recordAutofillResult(AUTOFILL_RESULT.PASSWORD_DISABLED_READONLY);
         return;
       }
 
       // Attach autocomplete stuff to the username field, if we have
       // one. This is normally used to select from multiple accounts,
       // but even with one account we should refill if the user edits.
       // We would also need this attached to show the insecure login
       // warning, regardless of saved login.
@@ -997,25 +1004,25 @@ var LoginManagerContent = {
         this._formFillService.markAsLoginManagerField(usernameField);
       }
 
       // Nothing to do if we have no matching logins available.
       // Only insecure pages reach this block and logs the same
       // telemetry flag.
       if (foundLogins.length == 0) {
         // We don't log() here since this is a very common case.
-        autofillResult = AUTOFILL_RESULT.NO_SAVED_LOGINS;
+        recordAutofillResult(AUTOFILL_RESULT.NO_SAVED_LOGINS);
         return;
       }
 
       // Prevent autofilling insecure forms.
       if (!userTriggered && !LoginHelper.insecureAutofill &&
           !InsecurePasswordUtils.isFormSecure(form)) {
         log("not filling form since it's insecure");
-        autofillResult = AUTOFILL_RESULT.INSECURE;
+        recordAutofillResult(AUTOFILL_RESULT.INSECURE);
         return;
       }
 
       var isAutocompleteOff = false;
       if (this._isAutocompleteDisabled(form) ||
           this._isAutocompleteDisabled(usernameField) ||
           this._isAutocompleteDisabled(passwordField)) {
         isAutocompleteOff = true;
@@ -1040,41 +1047,41 @@ var LoginManagerContent = {
         if (!fit)
           log("Ignored", l.username, "login: won't fit");
 
         return fit;
       }, this);
 
       if (logins.length == 0) {
         log("form not filled, none of the logins fit in the field");
-        autofillResult = AUTOFILL_RESULT.NO_LOGINS_FIT;
+        recordAutofillResult(AUTOFILL_RESULT.NO_LOGINS_FIT);
         return;
       }
 
       // Don't clobber an existing password.
       if (passwordField.value && !clobberPassword) {
         log("form not filled, the password field was already filled");
-        autofillResult = AUTOFILL_RESULT.EXISTING_PASSWORD;
+        recordAutofillResult(AUTOFILL_RESULT.EXISTING_PASSWORD);
         return;
       }
 
       // Select a login to use for filling in the form.
       var selectedLogin;
       if (!clobberUsername && usernameField && (usernameField.value ||
                                                 usernameField.disabled ||
                                                 usernameField.readOnly)) {
         // If username was specified in the field, it's disabled or it's readOnly, only fill in the
         // password if we find a matching login.
         var username = usernameField.value.toLowerCase();
 
         let matchingLogins = logins.filter(l =>
                                            l.username.toLowerCase() == username);
         if (matchingLogins.length == 0) {
           log("Password not filled. None of the stored logins match the username already present.");
-          autofillResult = AUTOFILL_RESULT.EXISTING_USERNAME;
+          recordAutofillResult(AUTOFILL_RESULT.EXISTING_USERNAME);
           return;
         }
 
         // If there are multiple, and one matches case, use it
         for (let l of matchingLogins) {
           if (l.username == usernameField.value) {
             selectedLogin = l;
           }
@@ -1093,34 +1100,34 @@ var LoginManagerContent = {
         let matchingLogins;
         if (usernameField)
           matchingLogins = logins.filter(l => l.username);
         else
           matchingLogins = logins.filter(l => !l.username);
 
         if (matchingLogins.length != 1) {
           log("Multiple logins for form, so not filling any.");
-          autofillResult = AUTOFILL_RESULT.MULTIPLE_LOGINS;
+          recordAutofillResult(AUTOFILL_RESULT.MULTIPLE_LOGINS);
           return;
         }
 
         selectedLogin = matchingLogins[0];
       }
 
       // We will always have a selectedLogin at this point.
 
       if (!autofillForm) {
         log("autofillForms=false but form can be filled");
-        autofillResult = AUTOFILL_RESULT.NO_AUTOFILL_FORMS;
+        recordAutofillResult(AUTOFILL_RESULT.NO_AUTOFILL_FORMS);
         return;
       }
 
       if (isAutocompleteOff && !ignoreAutocomplete) {
         log("Not filling the login because we're respecting autocomplete=off");
-        autofillResult = AUTOFILL_RESULT.AUTOCOMPLETE_OFF;
+        recordAutofillResult(AUTOFILL_RESULT.AUTOCOMPLETE_OFF);
         return;
       }
 
       // Fill the form
 
       if (usernameField) {
       // Don't modify the username field if it's disabled or readOnly so we preserve its case.
         let disabledOrReadOnly = usernameField.disabled || usernameField.readOnly;
@@ -1136,32 +1143,22 @@ var LoginManagerContent = {
           usernameField.setUserInput(selectedLogin.username);
         }
       }
       if (passwordField.value != selectedLogin.password) {
         passwordField.setUserInput(selectedLogin.password);
       }
 
       log("_fillForm succeeded");
-      autofillResult = AUTOFILL_RESULT.FILLED;
+      recordAutofillResult(AUTOFILL_RESULT.FILLED);
       let doc = form.ownerDocument;
       let win = doc.defaultView;
       let messageManager = messageManagerFromWindow(win);
       messageManager.sendAsyncMessage("LoginStats:LoginFillSuccessful");
     } finally {
-      if (autofillResult == -1) {
-        // eslint-disable-next-line no-unsafe-finally
-        throw new Error("_fillForm: autofillResult must be specified");
-      }
-
-      if (!userTriggered) {
-        // Ignore fills as a result of user action for this probe.
-        Services.telemetry.getHistogramById("PWMGR_FORM_AUTOFILL_RESULT").add(autofillResult);
-      }
-
       Services.obs.notifyObservers(form.rootElement, "passwordmgr-processed-form", null);
     }
   },
 
   /**
    * Verify if a field is a valid login form field and
    * returns some information about it's FormLike.
    *