Save password functionality added to the save password bubble

chrome/browser/password_manager/password_form_manager.cc

Index: chrome/browser/password_manager/password_form_manager.cc
diff --git a/chrome/browser/password_manager/password_form_manager.cc b/chrome/browser/password_manager/password_form_manager.cc
index a75b3c8a987633e072a9da020d04b797cd936ed7..63724b020ef897f9440af1204a351a16360bf16f 100644
--- a/chrome/browser/password_manager/password_form_manager.cc
+++ b/chrome/browser/password_manager/password_form_manager.cc
@@ -40,8 +40,7 @@ PasswordFormManager::PasswordFormManager(Profile* profile,
       manager_action_(kManagerActionNone),
       user_action_(kUserActionNone),
       submit_result_(kSubmitResultNotSubmitted),
-      should_save_password_(false),
-      should_blacklist_password_(false) {
+      password_action_(DO_NOTHING) {
   DCHECK(profile_);
   if (observed_form_.origin.is_valid())
     base::SplitString(observed_form_.origin.path(), '/', &form_path_tokens_);
@@ -54,7 +53,7 @@ PasswordFormManager::~PasswordFormManager() {
                             kMaxNumActionsTaken);
   // In case the tab is closed before the next navigation occurs this will
   // apply outstanding changes.
-  if (should_save_password_ || should_blacklist_password_)
+  if (!DO_NOTHING)

[Peter Kasting, 2013/09/10 23:04:41]

This isn't what you intended to write.

Plus, this check isn't necessary.  ApplyChange() checks internally, so you can
call it unconditionally.

[npentrel, 2013/09/11 09:10:42]

Done.

     ApplyChange();
 }
 
@@ -113,23 +112,11 @@ bool PasswordFormManager::DoesManage(const PasswordForm& form,
 }
 
 void PasswordFormManager::ApplyChange() {
-  DCHECK(!should_blacklist_password_ || !should_save_password_);
-  if (should_save_password_)
+  if (password_action_ == SAVE)
     Save();
-  else if (should_blacklist_password_)
+  else if (password_action_ == BLACKLIST)
     PermanentlyBlacklist();
-  should_blacklist_password_ = false;
-  should_save_password_ = false;
-}
-
-void PasswordFormManager::SavePassword() {
-  should_blacklist_password_ = false;
-  should_save_password_ = true;
-}
-
-void PasswordFormManager::BlacklistPassword() {
-  should_save_password_ = false;
-  should_blacklist_password_ = true;
+  password_action_ = DO_NOTHING;
 }
 
 bool PasswordFormManager::IsBlacklisted() {
@@ -560,7 +547,7 @@ bool PasswordFormManager::UpdatePendingCredentialsIfOtherPossibleUsername(
 int PasswordFormManager::ScoreResult(const PasswordForm& candidate) const {
   DCHECK_EQ(state_, MATCHING_PHASE);
   // For scoring of candidate login data:
-  // The most important element that should match is the origin, followed by

[Peter Kasting, 2013/09/10 23:04:41]

Why did you change this comment?

[npentrel, 2013/09/11 09:10:42]

Done.

+  // The most important element that match is the origin, followed by
   // the action, the password name, the submit button name, and finally the
   // username input field name.
   // Exact origin match gives an addition of 64 (1 << 6) + # of matching url