Generate passwords only for forms that autofill server marks as account creation forms.

components/autofill/content/renderer/password_generation_manager.cc

Index: components/autofill/content/renderer/password_generation_manager.cc
diff --git a/components/autofill/content/renderer/password_generation_manager.cc b/components/autofill/content/renderer/password_generation_manager.cc
index aa50cdc0b4dd69e531aa5e5a360c12e730482687..d459d75738271935e4bf91f7d4439c2ef2734f24 100644
--- a/components/autofill/content/renderer/password_generation_manager.cc
+++ b/components/autofill/content/renderer/password_generation_manager.cc
@@ -5,7 +5,9 @@
 #include "components/autofill/content/renderer/password_generation_manager.h"
 
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "components/autofill/core/common/autofill_messages.h"
+#include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/password_generation_util.h"
 #include "content/public/renderer/password_form_conversion_utils.h"
 #include "content/public/renderer/render_view.h"
@@ -64,6 +66,28 @@ bool GetAccountCreationPasswordFields(
   return false;
 }
 
+bool ContainsURL(const std::vector<GURL>& urls, const GURL& url) {
+  return std::find(urls.begin(), urls.end(), url) != urls.end();
+}
+
+// Returns true if the |form1| is essentially equal to |form2|.
+bool FormEquals(const autofill::FormData& form1,
+                const content::PasswordForm& form2) {
+  // TODO: use more signals than just origin to compare.

[Ilya Sherman, 2013/09/03 23:58:01]

nit: TODO(zysxqn):

[zysxqn, 2013/09/04 17:26:20]

Done.

+  return form1.origin == form2.origin;
+}
+
+bool ContainsForm(const std::vector<autofill::FormData>& forms,
+                  const content::PasswordForm& form) {
+  for (std::vector<autofill::FormData>::const_iterator it =
+           forms.begin(); it != forms.end(); ++it) {
+    if (FormEquals(*it, form)) {
+      return true;
+    }

[Ilya Sherman, 2013/09/03 23:58:01]

nit: No need for curly braces.

[zysxqn, 2013/09/04 17:26:20]

Done.

+  }
+  return false;
+}
+
 }  // namespace
 
 PasswordGenerationManager::PasswordGenerationManager(
@@ -85,8 +109,8 @@ void PasswordGenerationManager::DidFinishDocumentLoad(WebKit::WebFrame* frame) {
   // form, but there could be multiple password forms in each frame.
   if (!frame->parent()) {
     not_blacklisted_password_form_origins_.clear();
-    // Initialize to an empty and invalid GURL.
-    account_creation_form_origin_ = GURL();
+    account_creation_forms_.clear();
+    possible_account_creation_form_.reset(new content::PasswordForm());
     passwords_.clear();
   }
 }
@@ -127,7 +151,7 @@ void PasswordGenerationManager::DidFinishLoad(WebKit::WebFrame* frame) {
       password_generation::LogPasswordGenerationEvent(
           password_generation::SIGN_UP_DETECTED);
       passwords_ = passwords;
-      account_creation_form_origin_ = password_form->origin;
+      possible_account_creation_form_.swap(password_form);
       MaybeShowIcon();
       // We assume that there is only one account creation field per URL.
       return;
@@ -176,6 +200,8 @@ bool PasswordGenerationManager::OnMessageReceived(const IPC::Message& message) {
                         OnPasswordAccepted)
     IPC_MESSAGE_HANDLER(AutofillMsg_PasswordGenerationEnabled,
                         OnPasswordGenerationEnabled)
+    IPC_MESSAGE_HANDLER(AutofillMsg_AccountCreationFormsDetected,
+                        OnAccountCreationFormsDetected)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -203,27 +229,39 @@ void PasswordGenerationManager::OnPasswordGenerationEnabled(bool enabled) {
   enabled_ = enabled;
 }
 
+void PasswordGenerationManager::OnAccountCreationFormsDetected(
+    const std::vector<autofill::FormData>& forms) {
+  account_creation_forms_.insert(
+      account_creation_forms_.begin(), forms.begin(), forms.end());

[Ilya Sherman, 2013/09/03 23:58:01]

nit: Shouldn't be a big deal in practice, but it's more efficient to append to
the end of a vector than to insert at the front.

[zysxqn, 2013/09/04 17:26:20]

Done.

+  MaybeShowIcon();
+}
+
 void PasswordGenerationManager::MaybeShowIcon() {
   // We should show the password generation icon only when we have detected
-  // account creation form and we have confirmed from browser that this form
-  // is not blacklisted by the users.
-  if (!account_creation_form_origin_.is_valid() ||
+  // account creation form, we have confirmed from browser that this form
+  // is not blacklisted by the users, and the Autofill server has marked one
+  // of its field as ACCOUNT_CREATION_PASSWORD.
+  if (!possible_account_creation_form_.get() ||
       passwords_.empty() ||
-      not_blacklisted_password_form_origins_.empty()) {
+      not_blacklisted_password_form_origins_.empty() ||
+      account_creation_forms_.empty()) {
     return;
   }
 
-  for (std::vector<GURL>::iterator it =
-           not_blacklisted_password_form_origins_.begin();
-       it != not_blacklisted_password_form_origins_.end(); ++it) {
-    if (*it == account_creation_form_origin_) {
-      passwords_[0].passwordGeneratorButtonElement().setAttribute("style",
-                                                            "display:block");
-      password_generation::LogPasswordGenerationEvent(
-          password_generation::ICON_SHOWN);
-      return;
-    }
+  if (!ContainsURL(not_blacklisted_password_form_origins_,
+                   possible_account_creation_form_->origin)) {
+    return;
   }
+
+  if (!ContainsForm(account_creation_forms_,
+                    *possible_account_creation_form_)) {
+    return;
+  }
+
+  passwords_[0].passwordGeneratorButtonElement().setAttribute("style",
+                                                              "display:block");
+  password_generation::LogPasswordGenerationEvent(
+      password_generation::ICON_SHOWN);
 }
 
 }  // namespace autofill