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..41ff8a60de7736f23f720e0bef152436caf1c2b7 100644
--- a/components/autofill/content/renderer/password_generation_manager.cc
+++ b/components/autofill/content/renderer/password_generation_manager.cc
@@ -85,6 +85,7 @@ 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();
+    autofill_account_creation_form_origins_.clear();
     // Initialize to an empty and invalid GURL.
     account_creation_form_origin_ = GURL();
     passwords_.clear();
@@ -176,6 +177,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 +206,53 @@ void PasswordGenerationManager::OnPasswordGenerationEnabled(bool enabled) {
   enabled_ = enabled;
 }
 
+void PasswordGenerationManager::OnAccountCreationFormsDetected(
+    const std::vector<GURL>& origins) {
+  autofill_account_creation_form_origins_.insert(
+      autofill_account_creation_form_origins_.begin(),
+      origins.begin(),
+      origins.end());

[Ilya Sherman, 2013/08/29 23:08:13]

Is it expected to be possible that autofill_acount_creation_form_origins_ might
not be empty when this code is reached?  If so, why?

[zysxqn, 2013/09/03 23:00:20]

I'm not 100% sure about the flow but account_creation_forms_ is only cleared
once the main frame is loaded. So if there are account creation forms in other
frames (rare? I'm not sure) it might not be empty here. I think we use
"push_back" for the same reason in OnFormNotBlacklisted() above.

+  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.
+  // account creation form, we have confirmed from browser that this form
+  // is not blacklisted by the users, and the autofill server has marked one

[Ilya Sherman, 2013/08/29 23:08:13]

nit: "autofill" -> "Autofill"

[zysxqn, 2013/09/03 23:00:20]

Done.

+  // of its field as ACCOUNT_CREATION_PASSWORD.
   if (!account_creation_form_origin_.is_valid() ||
       passwords_.empty() ||
-      not_blacklisted_password_form_origins_.empty()) {
+      not_blacklisted_password_form_origins_.empty() ||
+      autofill_account_creation_form_origins_.empty()) {
     return;
   }
 
+  bool not_blacklisted = false;
   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;
+      not_blacklisted = true;
+      break;
+    }
+  }

[Ilya Sherman, 2013/08/29 23:08:13]

nit: I'd write this as an early return using a wrapper function around
std::find:

bool ContainsURL(const std::vector<GURL>& urls, const GURL& url) {
  return std::find(urls.begin(), urls.end(), url) != urls.end();
}

// Only show the icon for non-blacklisted forms:
if (!ContainsURL(not_blacklisted_password_form_origins_,
                 account_creation_form_origin_))
  return;

You can write something similar to replace the loop below as well.

[zysxqn, 2013/09/03 23:00:20]

Done.

+
+  bool is_autofill_account_creation_form = false;
+  for (std::vector<GURL>::iterator it =
+           autofill_account_creation_form_origins_.begin();
+       it != autofill_account_creation_form_origins_.end(); ++it) {
+    if (*it == account_creation_form_origin_) {
+      is_autofill_account_creation_form = true;
+      break;
     }
   }
+
+  if (not_blacklisted && is_autofill_account_creation_form) {
+    passwords_[0].passwordGeneratorButtonElement().setAttribute(
+        "style", "display:block");
+    password_generation::LogPasswordGenerationEvent(
+        password_generation::ICON_SHOWN);
+  }
 }
 
 }  // namespace autofill