[PasswordManager] Improve detection of ignorable change password forms.

components/password_manager/core/browser/password_form_manager.cc

Index: components/password_manager/core/browser/password_form_manager.cc
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc
index 096257b8d1a3114a819949087cb7350fd5c5c466..e8d78b0cf7b29ee6171492cfa176da07bb4fdd74 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <set>
 
+#include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/user_metrics.h"
 #include "base/strings/string_split.h"
@@ -15,12 +16,14 @@
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/validation.h"
+#include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/browser_save_password_progress_logger.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/password_manager/core/browser/password_store.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 
 using autofill::FormStructure;
 using autofill::PasswordForm;
@@ -68,6 +71,11 @@ PasswordForm CopyAndModifySSLValidity(const PasswordForm& orig,
   return result;
 }
 
+bool IsChangePasswordForm(const PasswordForm& candidate) {
+  return !candidate.new_password_element.empty() &&
+         !candidate.password_element.empty();
+}
+
 }  // namespace
 
 PasswordFormManager::PasswordFormManager(
@@ -314,24 +322,37 @@ void PasswordFormManager::ProvisionallySave(
     selected_username_ = credentials.username_value;
     is_new_login_ = false;
   } else {
-    // User typed in a new, unknown username.
-    user_action_ = kUserActionOverrideUsernameAndPassword;
-    pending_credentials_ = observed_form_;
-    pending_credentials_.username_value = credentials.username_value;
-    pending_credentials_.other_possible_usernames =
-        credentials.other_possible_usernames;
-
-    // The password value will be filled in later, remove any garbage for now.
-    pending_credentials_.password_value.clear();
-    pending_credentials_.new_password_value.clear();
-
-    // If this was a sign-up or change password form, the names of the elements
-    // are likely different than those on a login form, so do not bother saving
-    // them. We will fill them with meaningful values in UpdateLogin() when the
-    // user goes onto a real login form for the first time.
-    if (!credentials.new_password_element.empty()) {
-      pending_credentials_.password_element.clear();
-      pending_credentials_.new_password_element.clear();
+    // Check whether its a change-password form with missing username or its
+    // obscured. E.g. numeric indicator of the password strength please refer
+    // http://crbug.com/448351.
+    PasswordForm matching_form;
+    if (IsChangePasswordForm(credentials) &&
+        HasMatchingCredentialsInStore(credentials, &matching_form)) {
+      is_new_login_ = false;
+      pending_credentials_ = matching_form;
+      selected_username_ = matching_form.username_value;
+      user_action_ = kUserActionOverridePassword;
+    } else {
+      // User typed in a new, unknown username.
+      user_action_ = kUserActionOverrideUsernameAndPassword;
+      pending_credentials_ = observed_form_;
+      pending_credentials_.username_value = credentials.username_value;
+      pending_credentials_.other_possible_usernames =
+          credentials.other_possible_usernames;
+
+      // The password value will be filled in later, remove any garbage for now.
+      pending_credentials_.password_value.clear();
+      pending_credentials_.new_password_value.clear();
+
+      // If this was a sign-up or change password form, the names of the
+      // elements are likely different than those on a login form, so do not
+      // bother saving them. We will fill them with meaningful values in
+      // UpdateLogin() when the user goes onto a real login form for the first
+      // time.
+      if (!credentials.new_password_element.empty()) {
+        pending_credentials_.password_element.clear();
+        pending_credentials_.new_password_element.clear();
+      }
     }
   }
 
@@ -378,7 +399,9 @@ void PasswordFormManager::FetchMatchingLoginsFromPasswordStore(
 
   // Do not autofill on sign-up or change password forms (until we have some
   // working change password functionality).
-  if (!observed_form_.new_password_element.empty()) {
+  if (!observed_form_.new_password_element.empty() &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          autofill::switches::kEnablePasswordSaveOnInPageNavigation)) {
     if (logger)
       logger->LogMessage(Logger::STRING_FORM_NOT_AUTOFILLED);
     client_->AutofillResultsComputed();
@@ -404,10 +427,10 @@ bool PasswordFormManager::HasCompletedMatching() const {
 }
 
 bool PasswordFormManager::IsIgnorableChangePasswordForm() const {
-  bool is_change_password_form = !observed_form_.new_password_element.empty() &&
-                                 !observed_form_.password_element.empty();
-  bool is_username_certainly_correct = observed_form_.username_marked_by_site;
-  return is_change_password_form && !is_username_certainly_correct;
+  bool is_username_certainly_correct =
+      observed_form_.username_marked_by_site ||
+      HasMatchingCredentialsInStore(observed_form_, nullptr);
+  return IsChangePasswordForm(observed_form_) && !is_username_certainly_correct;
 }
 
 void PasswordFormManager::OnRequestDone(
@@ -868,4 +891,24 @@ void PasswordFormManager::SubmitFailed() {
     LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMISSION_FAILED);
 }
 
+bool PasswordFormManager::HasMatchingCredentialsInStore(
+    const autofill::PasswordForm& candidate,
+    autofill::PasswordForm* matching_form) const {
+  bool match_found = false;
+  for (PasswordFormMap::const_iterator iter = best_matches_.begin();

[vabr (Chromium), 2015/01/28 13:25:32]

nit: You can use
for (const auto& match : best_matches_) {...}

[Pritam Nikam, 2015/01/29 14:14:35]

Done.

+       iter != best_matches_.end(); ++iter) {
+    if (net::registry_controlled_domains::SameDomainOrHost(

[vabr (Chromium), 2015/01/28 13:25:32]

This is too permissive, we cannot just update passwords across domains. We do
not do that for PSL matched permissions either.

[Pritam Nikam, 2015/01/29 14:14:35]

Done.

+            iter->second->origin, candidate.origin,
+            net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES) &&
+        (iter->second->password_value == candidate.password_value)) {
+      if (matching_form != nullptr)

[vabr (Chromium), 2015/01/28 13:25:32]

nit: Just write:
if (matching_form)

[Pritam Nikam, 2015/01/29 14:14:35]

Done.

+        *matching_form = *(iter->second);
+      match_found = true;
+      break;
+    }
+  }
+
+  return match_found;
+}
+
 }  // namespace password_manager