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..e0e7d98be6561b999005bb56205e5f1ef20850b7 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,27 @@ 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(zysxqn): use more signals than just origin to compare.
+  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;
+  }
+  return false;
+}
+
 }  // namespace
 
 PasswordGenerationManager::PasswordGenerationManager(
@@ -85,8 +108,8 @@ void PasswordGenerationManager::DidFinishDocumentLoad(WebKit::WebFrame* frame) {
   // form, but there could be multiple password forms in each frame.
   if (!frame->parent()) {

[Garrett Casto, 2013/09/04 22:03:30]

One thing that would be really nice would be some stats to track cases where the
local heuristics fire but we don't show the icon. You don't need to add them in
this CL, but a TODO would be nice.

[zysxqn, 2013/09/04 22:17:50]

Done.

     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 +150,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 +199,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 +228,39 @@ void PasswordGenerationManager::OnPasswordGenerationEnabled(bool enabled) {
   enabled_ = enabled;
 }
 
+void PasswordGenerationManager::OnAccountCreationFormsDetected(
+    const std::vector<autofill::FormData>& forms) {
+  account_creation_forms_.insert(
+      account_creation_forms_.end(), forms.begin(), forms.end());
+  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