From e8a43d9e6c52f2b80d3f7726f1484d5ab7dbf7ac Mon Sep 17 00:00:00 2001
From: "gcasto@chromium.org"
 <gcasto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Thu, 13 Sep 2012 03:00:19 +0000
Subject: [PATCH] Loosen up heuristics for detecting account creation forms.

BUG=120779


Review URL: https://chromiumcodereview.appspot.com/10837324

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@156482 0039d316-1c4b-4281-b951-d872f2087c98
---
 .../autofill/password_generation_manager.cc        | 41 ++++++++++++++--------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/chrome/renderer/autofill/password_generation_manager.cc b/chrome/renderer/autofill/password_generation_manager.cc
index 11bd0b56d018..2ac682eeaa4f 100644
--- a/chrome/renderer/autofill/password_generation_manager.cc
+++ b/chrome/renderer/autofill/password_generation_manager.cc
@@ -33,22 +33,32 @@ bool GetAccountCreationPasswordFields(
   WebKit::WebVector<WebKit::WebFormControlElement> control_elements;
   form.getFormControlElements(control_elements);
 
+  size_t num_input_elements = 0;
   for (size_t i = 0; i < control_elements.size(); i++) {
     WebKit::WebInputElement* input_element =
         toWebInputElement(&control_elements[i]);
     // Only pay attention to visible password fields.
     if (input_element &&
-        input_element->isPasswordField() &&
+        input_element->isTextField() &&
         input_element->hasNonEmptyBoundingBox()) {
-      passwords->push_back(*input_element);
+      num_input_elements++;
+      if (input_element->isPasswordField())
+        passwords->push_back(*input_element);
     }
   }
 
-  // For now, just assume that if there are two password fields in the
-  // form that this is meant for account creation.
-  // TODO(gcasto): Determine better heauristics for this.
-  if (passwords->size() == 2)
+  // This may be too lenient, but we assume that any form with at least three
+  // input elements where at least one of them is a password is an account
+  // creation form.
+  if (!passwords->empty() && num_input_elements >= 3) {
+    // We trim |passwords| because occasionally there are forms where the
+    // security question answers are put in password fields and we don't want
+    // to fill those.
+    if (passwords->size() > 2)
+      passwords->resize(2);
+
     return true;
+  }
 
   return false;
 }
@@ -103,7 +113,7 @@ void PasswordGenerationManager::DidFinishLoad(WebKit::WebFrame* frame) {
     scoped_ptr<webkit::forms::PasswordForm> password_form(
         webkit::forms::PasswordFormDomManager::CreatePasswordForm(forms[i]));
     if (!password_form.get()) {
-      DVLOG(2) << "Invalid action on form";
+      DVLOG(2) << "Skipping form as it would not be saved";
       continue;
     }
 
@@ -168,14 +178,15 @@ void PasswordGenerationManager::handleClick(WebKit::WebInputElement& element) {
   scoped_ptr<webkit::forms::PasswordForm> password_form(
       webkit::forms::PasswordFormDomManager::CreatePasswordForm(
           element.form()));
-  if (password_form.get()) {
-    Send(new AutofillHostMsg_ShowPasswordGenerationPopup(routing_id(),
-                                                         rect,
-                                                         element.maxLength(),
-                                                         *password_form));
-    password_generation::LogPasswordGenerationEvent(
-        password_generation::BUBBLE_SHOWN);
-  }
+  // We should not have shown the icon we can't create a valid PasswordForm.
+  DCHECK(password_form.get());
+
+  Send(new AutofillHostMsg_ShowPasswordGenerationPopup(routing_id(),
+                                                       rect,
+                                                       element.maxLength(),
+                                                       *password_form));
+  password_generation::LogPasswordGenerationEvent(
+      password_generation::BUBBLE_SHOWN);
 }
 
 void PasswordGenerationManager::willDetach(
-- 
2.11.4.GIT