Save password functionality added to the save password bubble

chrome/browser/password_manager/password_form_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/password_manager/password_form_manager.h"
 
 #include <algorithm>
 
 #include "base/metrics/histogram.h"
 #include "base/strings/string_split.h"
@@ skipping 22 matching lines @@
       is_new_login_(true),
       has_generated_password_(false),
       password_manager_(password_manager),
       preferred_match_(NULL),
       state_(PRE_MATCHING_PHASE),
       profile_(profile),
       web_contents_(web_contents),
       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_);
   observed_form_.ssl_valid = ssl_valid;
 }
 
 PasswordFormManager::~PasswordFormManager() {
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.ActionsTaken",
                             GetActionsTaken(),
                             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();
 }
 
 int PasswordFormManager::GetActionsTaken() {
   return user_action_ + kUserActionMax * (manager_action_ +
          kManagerActionMax * submit_result_);
 };
 
 // TODO(timsteele): use a hash of some sort in the future?
 bool PasswordFormManager::DoesManage(const PasswordForm& form,
@@ skipping 38 matching lines @@
                          after_scheme2,
                          observed_form_.origin.spec().end())
                          != form.origin.spec().end();
     }
     return false;
   }
   return true;
 }
 
 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() {
   DCHECK_EQ(state_, POST_MATCHING_PHASE);
   if (preferred_match_ && preferred_match_->blacklisted_by_user)
     return true;
   return false;
 }
 
 void PasswordFormManager::PermanentlyBlacklist() {
@@ skipping 410 matching lines @@
         return true;
       }
     }
   }
   return false;
 }
 
 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
   // dirs.
   // Partial match gives an addition of 32 (1 << 5) + # matching url dirs
   // That way, a partial match cannot trump an exact match even if
   // the partial one matches all other attributes (action, elements) (and
   // regardless of the matching depth in the URL path).
   // If public suffix origin match was not used, it gives an addition of
   // 16 (1 << 4).
@@ skipping 42 matching lines @@
 
 void PasswordFormManager::SubmitFailed() {
   submit_result_ = kSubmitResultFailed;
 }
 
 void PasswordFormManager::SendNotBlacklistedToRenderer() {
   content::RenderViewHost* host = web_contents_->GetRenderViewHost();
   host->Send(new AutofillMsg_FormNotBlacklisted(host->GetRoutingID(),
                                                  observed_form_));
 }