Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1116)

Unified Diff: components/password_manager/core/browser/password_form_manager.cc

Issue 1014683006: [Password manager] Recognise squashed login+sign-up forms (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix failing PasswordManagerTest Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 dc4bf9ac1965a9ce0989811744b23cffc140d326..89d9633c365413a2dadec942e79285b2e838ef89 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -119,6 +119,13 @@ int PasswordFormManager::GetActionsTaken() const {
(manager_action_ + kManagerActionMax * submit_result_);
}
+// static
+base::string16 PasswordFormManager::PasswordToSave(const PasswordForm& form) {
+ if (form.new_password_element.empty() || form.new_password_value.empty())
+ return form.password_value;
+ return form.new_password_value;
+}
+
// TODO(timsteele): use a hash of some sort in the future?
PasswordFormManager::MatchResultMask PasswordFormManager::DoesManage(
const PasswordForm& form) const {
@@ -256,13 +263,7 @@ void PasswordFormManager::ProvisionallySave(
DCHECK_EQ(state_, POST_MATCHING_PHASE);
DCHECK_NE(RESULT_NO_MATCH, DoesManage(credentials));
- // If this was a sign-up or change password form, we want to persist the new
- // password; if this was a login form, then the current password (which might
- // still be "new" in the sense that we see these credentials for the first
- // time, or that the user manually entered his actual password to overwrite an
- // obsolete password we had in the store).
- base::string16 password_to_save(credentials.new_password_element.empty() ?
- credentials.password_value : credentials.new_password_value);
+ base::string16 password_to_save(PasswordToSave(credentials));
// 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
@@ -411,7 +412,7 @@ bool PasswordFormManager::HasCompletedMatching() const {
bool PasswordFormManager::IsIgnorableChangePasswordForm(
const PasswordForm& form) const {
bool is_change_password_form =
- !form.new_password_element.empty() && !form.password_element.empty();
+ !form.new_password_value.empty() && !form.password_value.empty();
return is_change_password_form && !form.username_marked_by_site &&
!DoesUsenameAndPasswordMatchCredentials(
form.username_value, form.password_value, best_matches_);
@@ -544,8 +545,10 @@ void PasswordFormManager::ProcessFrame(
return;
// 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()) {
+ // working change password functionality). The combined login-sign-up forms
Garrett Casto 2015/03/17 22:49:37 "The" is unnecessary here.
vabr (Chromium) 2015/03/18 16:51:03 Done.
+ // are OK to autofill in the login part.
+ if (observed_form_.layout != PasswordForm::Layout::LAYOUT_LOGIN_AND_SIGNUP &&
+ !observed_form_.new_password_element.empty()) {
if (client_->IsLoggingActive()) {
BrowserSavePasswordProgressLogger logger(client_);
logger.LogMessage(Logger::PROCESS_FRAME_METHOD);

Powered by Google App Engine
This is Rietveld 408576698