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..d99fe6d61ced64efa4269b750ef35d038f3d7632 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,6 +16,7 @@ |
#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" |
@@ -68,6 +70,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( |
@@ -246,13 +253,15 @@ void PasswordFormManager::ProvisionallySave( |
base::string16 password_to_save(credentials.new_password_element.empty() ? |
credentials.password_value : credentials.new_password_value); |
+ bool is_username_certainly_correct = credentials.username_marked_by_site; |
vabr (Chromium)
2015/02/02 14:18:49
nit: Because credentials.username_marked_by_site i
Pritam Nikam
2015/02/05 06:12:07
Done.
|
+ |
// Make sure the important fields stay the same as the initially observed or |
// autofilled ones, as they may have changed if the user experienced a login |
// failure. |
// Look for these credentials in the list containing auto-fill entries. |
PasswordFormMap::const_iterator it = |
best_matches_.find(credentials.username_value); |
- if (it != best_matches_.end()) { |
+ if (!is_username_certainly_correct && it != best_matches_.end()) { |
vabr (Chromium)
2015/02/02 14:18:49
Don't narrow the condition this way -- observe tha
Pritam Nikam
2015/02/05 06:12:06
Done.
|
// The user signed in with a login we autofilled. |
pending_credentials_ = *it->second; |
bool password_changed = |
@@ -303,6 +312,16 @@ void PasswordFormManager::ProvisionallySave( |
is_new_login_ = false; |
if (password_changed) |
user_action_ = kUserActionOverridePassword; |
+ |
+ // Check whether its a change-password form with matching credentials in |
+ // stored form. |
+ PasswordForm matching_form; |
+ if (IsChangePasswordForm(credentials) && |
+ HasMatchingCredentialsInStore(credentials, &matching_form)) { |
vabr (Chromium)
2015/02/02 14:18:49
This is actually unnecessary -- you already know t
Pritam Nikam
2015/02/05 06:12:07
Done.
|
+ pending_credentials_ = matching_form; |
vabr (Chromium)
2015/02/02 14:18:50
Save the copy -- just pass pending_credentials_ to
vabr (Chromium)
2015/02/02 14:18:50
Given the other comments, the modification needed
Pritam Nikam
2015/02/05 06:12:06
Done.
Pritam Nikam
2015/02/05 06:12:06
Done.
|
+ selected_username_ = matching_form.username_value; |
vabr (Chromium)
2015/02/02 14:18:50
Why do you set this? According to password_form_ma
Pritam Nikam
2015/02/05 06:12:06
Done.
|
+ user_action_ = kUserActionOverridePassword; |
vabr (Chromium)
2015/02/02 14:18:49
How do you know the password is overridden? Should
Pritam Nikam
2015/02/05 06:12:07
Done.
|
+ } |
} |
} else if (action == ALLOW_OTHER_POSSIBLE_USERNAMES && |
UpdatePendingCredentialsIfOtherPossibleUsername( |
@@ -325,10 +344,11 @@ void PasswordFormManager::ProvisionallySave( |
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 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 |
vabr (Chromium)
2015/02/02 14:18:49
Please do not just reformat the comments, if you d
Pritam Nikam
2015/02/05 06:12:06
Done.
|
+ // 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 +398,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( |
vabr (Chromium)
2015/02/02 14:18:49
This change looks like a bad merge, and nothing to
Pritam Nikam
2015/02/05 06:12:06
Done.
|
+ autofill::switches::kEnablePasswordSaveOnInPageNavigation)) { |
if (logger) |
logger->LogMessage(Logger::STRING_FORM_NOT_AUTOFILLED); |
client_->AutofillResultsComputed(); |
@@ -404,10 +426,8 @@ 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; |
+ return IsChangePasswordForm(observed_form_) && !is_username_certainly_correct; |
} |
void PasswordFormManager::OnRequestDone( |
@@ -868,4 +888,21 @@ void PasswordFormManager::SubmitFailed() { |
LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMISSION_FAILED); |
} |
+bool PasswordFormManager::HasMatchingCredentialsInStore( |
+ const autofill::PasswordForm& candidate, |
+ autofill::PasswordForm* matching_form) const { |
+ bool match_found = false; |
+ for (const auto& match : best_matches_) { |
+ if (match.second->username_value == candidate.username_value && |
+ match.second->password_value == candidate.password_value) { |
+ if (matching_form) |
vabr (Chromium)
2015/02/02 14:18:49
There should also be a check that the candidate ha
Pritam Nikam
2015/02/05 06:12:06
Done. Removed this function, instead performed in-
|
+ *matching_form = *(match.second); |
+ match_found = true; |
+ break; |
+ } |
+ } |
+ |
+ return match_found; |
+} |
+ |
} // namespace password_manager |