Extend logging for password manager internals page

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index e85a4c27e0c41bfd444e5631ac90675f141c9915..18b78142854c8661ac5ef72716d28619c6dd5b6e 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -7,6 +7,7 @@
 #include <stddef.h>
 
 #include <memory>
+#include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ -433,7 +434,11 @@ bool FillUserNameAndPassword(
     bool set_selection,
     std::map<const blink::WebInputElement, blink::WebString>*
         nonscript_modified_values,
-    base::Callback<void(blink::WebInputElement*)> registration_callback) {
+    base::Callback<void(blink::WebInputElement*)> registration_callback,
+    RendererSavePasswordProgressLogger* logger) {
+  if (logger)
+    logger->LogMessage(Logger::STRING_FILL_USERNAME_AND_PASSWORD_METHOD);
+
   // Don't fill username if password can't be set.
   if (!IsElementAutocompletable(*password_element))
     return false;
@@ -452,6 +457,8 @@ bool FillUserNameAndPassword(
                        exact_username_match)) {
     username = fill_data.username_field.value;
     password = fill_data.password_field.value;
+    if (logger)
+      logger->LogMessage(Logger::STRING_USERNAMES_MATCH);
   } else {
     // Scan additional logins for a match.
     for (const auto& it : fill_data.additional_logins) {
@@ -461,6 +468,10 @@ bool FillUserNameAndPassword(
         break;
       }
     }
+    if (logger) {
+      logger->LogBoolean(Logger::STRING_MATCH_IN_ADDITIONAL,
+                         !(username.empty() && password.empty()));
+    }
 
     // Check possible usernames.
     if (username.empty() && password.empty()) {
@@ -487,10 +498,12 @@ bool FillUserNameAndPassword(
   // Input matches the username, fill in required values.
   if (!username_element->isNull() &&
       IsElementAutocompletable(*username_element)) {
-    // TODO(vabr): Why not setSuggestedValue? http://crbug.com/507714
+    // TODO(crbug.com/507714): Why not setSuggestedValue?
     username_element->setValue(username, true);
     (*nonscript_modified_values)[*username_element] = username;
     username_element->setAutofilled(true);
+    if (logger)
+      logger->LogMessage(Logger::STRING_USERNAME_FILLED);

[dvadym, 2016/04/26 14:47:34]

It seems that it would be helpful to log also nameForAutofill() of a username
field, then we will know from log, whether we fill incorrectly. WDYT?

[vabr (Chromium), 2016/04/26 16:55:41]

While I agree, this would need some more changes to the
SavePasswordProgressLogger. Currently, no free strings are allowed, for security
reasons. The only exception is the element names as captured in a PasswordForm,
there the code does some processing to avoid exploits. So what we'd need is to
add a method similar to SavePasswordProgressLogger::LogPasswordForm, but just
logging element names. It's not a lot of work, but I wanted to keep this CL
small.

My suggestion is to do a follow-up CL for the logging of element names.

     if (set_selection) {
       form_util::PreviewSuggestion(username, current_username,
                                    username_element);
@@ -509,6 +522,8 @@ bool FillUserNameAndPassword(
   registration_callback.Run(password_element);
 
   password_element->setAutofilled(true);
+  if (logger)
+    logger->LogMessage(Logger::STRING_PASSWORD_FILLED);

[dvadym, 2016/04/26 14:47:34]

It seems that it would be helpful to log also nameForAutofill() of a password
field, then we will know from log, whether we fill incorrectly. WDYT?

[vabr (Chromium), 2016/04/26 16:55:41]

Ditto.

   return true;
 }
 
@@ -526,7 +541,8 @@ bool FillFormOnPasswordReceived(
     blink::WebInputElement password_element,
     std::map<const blink::WebInputElement, blink::WebString>*
         nonscript_modified_values,
-    base::Callback<void(blink::WebInputElement*)> registration_callback) {
+    base::Callback<void(blink::WebInputElement*)> registration_callback,
+    RendererSavePasswordProgressLogger* logger) {
   // Do not fill if the password field is in a chain of iframes not having
   // identical origin.
   blink::WebFrame* cur_frame = password_element.document().frame();
@@ -594,13 +610,10 @@ bool FillFormOnPasswordReceived(
 
   // Fill if we have an exact match for the username. Note that this sets
   // username to autofilled.
-  return FillUserNameAndPassword(&username_element,
-                                 &password_element,
-                                 fill_data,
-                                 true /* exact_username_match */,
-                                 false /* set_selection */,
-                                 nonscript_modified_values,
-                                 registration_callback);
+  return FillUserNameAndPassword(
+      &username_element, &password_element, fill_data,
+      true /* exact_username_match */, false /* set_selection */,
+      nonscript_modified_values, registration_callback, logger);
 }
 
 // Takes a |map| with pointers as keys and linked_ptr as values, and returns
@@ -694,11 +707,11 @@ bool PasswordAutofillAgent::TextFieldDidEndEditing(
 
   // Do not set selection when ending an editing session, otherwise it can
   // mess with focus.
-  FillUserNameAndPassword(
-      &username, &password, fill_data, true, false,
-      &nonscript_modified_values_,
-      base::Bind(&PasswordValueGatekeeper::RegisterElement,
-                 base::Unretained(&gatekeeper_)));
+  FillUserNameAndPassword(&username, &password, fill_data, true, false,
+                          &nonscript_modified_values_,
+                          base::Bind(&PasswordValueGatekeeper::RegisterElement,
+                                     base::Unretained(&gatekeeper_)),
+                          nullptr);
   return true;
 }
 
@@ -1311,10 +1324,24 @@ void PasswordAutofillAgent::DidStartProvisionalLoad() {
 void PasswordAutofillAgent::OnFillPasswordForm(
     int key,
     const PasswordFormFillData& form_data) {
+  std::unique_ptr<RendererSavePasswordProgressLogger> logger;
+  if (logging_state_active_) {
+    logger.reset(new RendererSavePasswordProgressLogger(this, routing_id()));
+    logger->LogMessage(Logger::STRING_ON_FILL_PASSWORD_FORM_METHOD);
+  }
+
   bool ambiguous_or_empty_names =
       DoesFormContainAmbiguousOrEmptyNames(form_data);
   FormElementsList forms;
   FindFormElements(render_frame(), form_data, ambiguous_or_empty_names, &forms);
+  if (logger) {
+    logger->LogBoolean(Logger::STRING_AMBIGUOUS_OR_EMPTY_NAMES,
+                       ambiguous_or_empty_names);
+    logger->LogNumber(Logger::STRING_NUMBER_OF_POTENTIAL_FORMS_TO_FILL,
+                      forms.size());
+    logger->LogBoolean(Logger::STRING_FORM_DATA_WAIT,
+                       form_data.wait_for_username);
+  }
   for (const auto& form : forms) {
     base::string16 username_field_name;
     base::string16 password_field_name =
@@ -1325,6 +1352,14 @@ void PasswordAutofillAgent::OnFillPasswordForm(
       username_field_name =
           FieldName(form_data.username_field, ambiguous_or_empty_names);
     }
+    if (logger) {
+      logger->LogBoolean(Logger::STRING_CONTAINS_FILLABLE_USERNAME_FIELD,
+                         form_contains_fillable_username_field);
+      logger->LogBoolean(Logger::STRING_USERNAME_FIELD_NAME_EMPTY,
+                         username_field_name.empty());
+      logger->LogBoolean(Logger::STRING_PASSWORD_FIELD_NAME_EMPTY,
+                         password_field_name.empty());
+    }
 
     // Attach autocomplete listener to enable selecting alternate logins.
     blink::WebInputElement username_element;
@@ -1362,12 +1397,11 @@ void PasswordAutofillAgent::OnFillPasswordForm(
     // until the user types in a valid username.
     if (!form_data.wait_for_username) {
       FillFormOnPasswordReceived(
-          form_data,
-          username_element,
-          password_element,
+          form_data, username_element, password_element,
           &nonscript_modified_values_,
           base::Bind(&PasswordValueGatekeeper::RegisterElement,
-                     base::Unretained(&gatekeeper_)));
+                     base::Unretained(&gatekeeper_)),
+          logger.get());
     }
 
     PasswordInfo password_info;