PasswordManager: Add rendered forms to pending_login_managers_

components/password_manager/core/browser/password_manager.cc

Index: components/password_manager/core/browser/password_manager.cc
diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc
index 4c0a4c3bcd07ce80ac680f2213bce96ca25bd886..1c5c32f3f71dc76c9096abd8e870dab6433884ad 100644
--- a/components/password_manager/core/browser/password_manager.cc
+++ b/components/password_manager/core/browser/password_manager.cc
@@ -310,10 +310,19 @@ void PasswordManager::OnPasswordFormSubmitted(
 
 void PasswordManager::OnPasswordFormsParsed(
     const std::vector<PasswordForm>& forms) {
+  CreatePendingLoginManagers(forms);
+}
+
+void PasswordManager::CreatePendingLoginManagers(
+    const std::vector<PasswordForm>& forms) {
   // Don't try to autofill or save passwords in the presence of SSL errors.
   if (driver_->DidLastPageLoadEncounterSSLErrors())
     return;
 
+  // Copy the weak pointers to the currently known login managers for comparison
+  // against the newly added.
+  std::vector<PasswordFormManager*> old_login_managers(
+      pending_login_managers_.get());
   for (std::vector<PasswordForm>::const_iterator iter = forms.begin();
        iter != forms.end();
        ++iter) {
@@ -321,6 +330,16 @@ void PasswordManager::OnPasswordFormsParsed(
     // SpdyProxy authentication, as indicated by the realm.
     if (EndsWith(iter->signon_realm, kSpdyProxyRealm, true))
       continue;
+    std::vector<PasswordFormManager*>::const_iterator old_manager;
+    for (old_manager = old_login_managers.begin();
+         old_manager != old_login_managers.end();
+         ++old_manager) {
+      if ((*old_manager)
+              ->DoesManage(*iter, PasswordFormManager::ACTION_MATCH_REQUIRED))

[Garrett Casto, 2014/06/19 00:05:40]

Nit: Is it possible to line break like

if ((*old_manager)->DoesManage(*iter,
       ...

I personally don't like breaking at "->" but I realize other people have
different opinions.

[vabr (Chromium), 2014/06/20 07:55:07]

Unfortunately, the second argument is 1 character too long for any indentation
started as in your suggestion to be possible. :)
Also, I prefer to keep what clang-format indented, to make future refactorings
easier.

But, I managed to address your comment by slightly changing the code, where I
don't keep the old_manager iterator outside the loop, and instead keep just a
bool signifying whether an old manager was found or not.

That makes the code more readable IMO, and as a side-effect leads to a different
indentation of the DoesManager line. :)

Please let me know if you have further objections, I'm happy to address them in
a follow-up CL.

+        break;
+    }
+    if (old_manager != old_login_managers.end())
+      continue;  // The current form is already managed.
 
     bool ssl_valid = iter->origin.SchemeIsSecure();
     PasswordFormManager* manager =
@@ -346,6 +365,8 @@ bool PasswordManager::ShouldPromptUserToSavePassword() const {
 
 void PasswordManager::OnPasswordFormsRendered(
     const std::vector<PasswordForm>& visible_forms) {
+  CreatePendingLoginManagers(visible_forms);
+
   scoped_ptr<BrowserSavePasswordProgressLogger> logger;
   if (client_->IsLoggingActive()) {
     logger.reset(new BrowserSavePasswordProgressLogger(client_));