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

components/autofill/content/renderer/password_generation_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/content/renderer/password_generation_manager.h"
 
 #include "base/logging.h"
 #include "components/autofill/core/common/autofill_messages.h"
 #include "components/autofill/core/common/password_generation_util.h"
 #include "content/public/renderer/password_form_conversion_utils.h"
@@ skipping 67 matching lines @@
 void PasswordGenerationManager::DidFinishDocumentLoad(WebKit::WebFrame* frame) {
   // In every navigation, the IPC message sent by the password autofill manager
   // to query whether the current form is blacklisted or not happens when the
   // document load finishes, so we need to clear previous states here before we
   // hear back from the browser. We only clear this state on main frame load
   // as we don't want subframe loads to clear state that we have recieved from
   // the main frame. Note that we assume there is only one account creation
   // 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();
   }
 }
 
 void PasswordGenerationManager::DidFinishLoad(WebKit::WebFrame* frame) {
   // We don't want to generate passwords if the browser won't store or sync
   // them.
   if (!enabled_)
@@ skipping 71 matching lines @@
 
 bool PasswordGenerationManager::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(PasswordGenerationManager, message)
     IPC_MESSAGE_HANDLER(AutofillMsg_FormNotBlacklisted,
                         OnFormNotBlacklisted)
     IPC_MESSAGE_HANDLER(AutofillMsg_GeneratedPasswordAccepted,
                         OnPasswordAccepted)
     IPC_MESSAGE_HANDLER(AutofillMsg_PasswordGenerationEnabled,
                         OnPasswordGenerationEnabled)
+    IPC_MESSAGE_HANDLER(AutofillMsg_AccountCreationFormsDetected,
+                        OnAccountCreationFormsDetected)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 void PasswordGenerationManager::OnFormNotBlacklisted(
     const content::PasswordForm& form) {
   not_blacklisted_password_form_origins_.push_back(form.origin);
   MaybeShowIcon();
 }
 
 void PasswordGenerationManager::OnPasswordAccepted(
     const base::string16& password) {
   for (std::vector<WebKit::WebInputElement>::iterator it = passwords_.begin();
        it != passwords_.end(); ++it) {
     it->setValue(password);
     it->setAutofilled(true);
     // Advance focus to the next input field. We assume password fields in
     // an account creation form are always adjacent.
     render_view_->GetWebView()->advanceFocus(false);
   }
 }
 
 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