Allow deleting autofill password suggestions on Shift+Delete

chrome/browser/password_manager/password_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_manager.h"
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/platform_thread.h"
 #include "chrome/browser/password_manager/password_form_manager.h"
 #include "chrome/browser/password_manager/password_manager_delegate.h"
 #include "chrome/browser/password_manager/password_manager_metrics_util.h"
+#include "chrome/browser/password_manager/password_store.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "components/autofill/core/common/autofill_messages.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
@@ skipping 229 matching lines @@
 
 bool PasswordManager::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(PasswordManager, message)
     IPC_MESSAGE_HANDLER(AutofillHostMsg_PasswordFormsParsed,
                         OnPasswordFormsParsed)
     IPC_MESSAGE_HANDLER(AutofillHostMsg_PasswordFormsRendered,
                         OnPasswordFormsRendered)
     IPC_MESSAGE_HANDLER(AutofillHostMsg_PasswordFormSubmitted,
                         OnPasswordFormSubmitted)
+    IPC_MESSAGE_HANDLER(AutofillHostMsg_RemovePasswordSuggestion,
+                        OnRemovePasswordSuggestion)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 void PasswordManager::OnPasswordFormSubmitted(
     const PasswordForm& password_form) {
   ProvisionallySavePassword(password_form);
   for (size_t i = 0; i < submission_callbacks_.size(); ++i) {
     submission_callbacks_[i].Run(password_form);
@@ skipping 19 matching lines @@
         new PasswordFormManager(delegate_->GetProfile(),
                                 this,
                                 web_contents(),
                                 *iter,
                                 ssl_valid);
     pending_login_managers_.push_back(manager);
     manager->FetchMatchingLoginsFromPasswordStore();
   }
 }
 
+void PasswordManager::OnRemovePasswordSuggestion(
+    const PasswordForm& password_form,
+    std::vector<PasswordForm> updated_password_forms) {
+  scoped_refptr<PasswordStore> password_store =
+      PasswordStoreFactory::GetForProfile(delegate_->GetProfile(),
+                                          Profile::EXPLICIT_ACCESS);
+
+  ManagePasswordsBubbleUIController* manage_passwords_bubble_ui_controller =
+      ManagePasswordsBubbleUIController::FromWebContents(web_contents());
+  if (manage_passwords_bubble_ui_controller) {
+    manage_passwords_bubble_ui_controller->RemoveFromBestMatches(password_form);
+  }
+
+  // Deleting the PasswordFormManager for the current |password_form| to avoid
+  // that we have two PasswordFormManager for it:
+  // AutofillHostMsg_PasswordFormsParsed is going to be recived in the near

[vabr (Chromium), 2014/03/07 23:43:36]

typos: recived, genrate

[riadh.chtara, 2014/03/14 17:16:08]

Done.

+  // future to update the current form and this is going to genrate a new
+  // PasswordFormManager

[vabr (Chromium), 2014/03/07 23:43:36]

Please end sentences with a full stop.

[riadh.chtara, 2014/03/14 17:16:08]

Done.

+
+  std::vector<PasswordFormManager*> not_yet_updated;
+  bool removed = false;
+  for (ScopedVector<PasswordFormManager>::iterator iter =
+           pending_login_managers_.begin();
+       iter != pending_login_managers_.end();
+       iter++) {

[vabr (Chromium), 2014/03/07 23:43:36]

nit: prefer prefix increment ++iter to postfix iter++
Prefix is consistently used in this file. Also, the value of the expression
iter++ (which is iter before increment) is not used, so it does not make sense
to produce it.

Also below.

[riadh.chtara, 2014/03/14 17:16:08]

Done.

+
+    if ((*iter)->DoesManage(password_form,
+                            PasswordFormManager::ACTION_MATCH_REQUIRED)) {
+      if (!removed) {
+        (*iter)->RemoveAndUpdate(password_form.username_value);
+        removed = true;
+      } else {
+        (*iter)->Update(password_form.username_value);
+      }
+    } else {
+      not_yet_updated.push_back(*iter);
+    }
+  }
+
+  if (removed) {
+    for (std::vector<PasswordFormManager*>::iterator pfm =

[vabr (Chromium), 2014/03/07 23:43:36]

Please do not use abbreviations like 'pfm' or 'pf'. That looks cryptic (also,
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Genera...),
and once it's decrypted, brings no interesting information. Use |iter| for
general iterators, as that looks consistent with the file. Ideally, use more
descriptive name, like |form_manager_update_candidate|.

[riadh.chtara, 2014/03/14 17:16:08]

Done.

+             not_yet_updated.begin();
+         pfm != not_yet_updated.end();
+         pfm++) {
+      for (std::vector<PasswordForm>::iterator pf =
+               updated_password_forms.begin();
+           pf != updated_password_forms.end();) {
+        if ((*pfm)
+                ->DoesManage(*pf, PasswordFormManager::ACTION_MATCH_REQUIRED)) {
+
+          (*pfm)->Update(password_form.username_value);
+          pf = updated_password_forms.erase(pf);

[vabr (Chromium), 2014/03/07 23:43:36]

Single element deleting in vector is most of the time wasteful: every deletion
requires moving the element in the tail of the vector. Unless we know the vector
being very small (like <4 elements) for the very majority of cases, I would
suggest a different approach.

In this case, I don't really understand, why you delete some of the elements in
the for loop, while the rest will get deleted just a couple of lines below, at
the function end.

[riadh.chtara, 2014/03/14 17:16:08]

Done.

+

[vabr (Chromium), 2014/03/07 23:43:36]

nit: remove blank line

[riadh.chtara, 2014/03/14 17:16:08]

Done.

+        } else {
+          pf++;
+
+        }
+
+      }
+    }
+
+  }
+}
+
+
 bool PasswordManager::ShouldShowSavePasswordInfoBar() const {
   return provisional_save_manager_->IsNewLogin() &&
          !provisional_save_manager_->HasGeneratedPassword() &&
          !provisional_save_manager_->IsPendingCredentialsPublicSuffixMatch();
 }
 
 void PasswordManager::OnPasswordFormsRendered(
     const std::vector<PasswordForm>& visible_forms) {
   if (!provisional_save_manager_.get())
     return;
@@ skipping 109 matching lines @@
   }
 
   ManagePasswordsBubbleUIController* manage_passwords_bubble_ui_controller =
       ManagePasswordsBubbleUIController::FromWebContents(web_contents());
   if (manage_passwords_bubble_ui_controller &&
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableSavePasswordBubble)) {
     manage_passwords_bubble_ui_controller->OnPasswordAutofilled(best_matches);
   }
 }