ManagePasswordsUIController should make a deep copy of values which it doesn't own.

chrome/browser/ui/passwords/manage_passwords_ui_controller.cc

Index: chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
index 890bf6798b5fe0c909098457484fba7b5b89651d..f7e4ed5dbdda2e866b86ef827ce0bcac5c55bb2a 100644
--- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
@@ -37,7 +37,20 @@ autofill::ConstPasswordFormMap ConstifyMap(
   return ret;
 }
 
-} // namespace
+void DeepCopyMap(const autofill::PasswordFormMap& map,

[vabr (Chromium), 2014/08/04 15:36:56]

Please comment on the use and expectations. Things I suggest to mention:
* ret and deleter being output params
* ret keeping the mapping of |map|, but with weak pointers to copies of forms
stored in |map|
* deleter being the owner of the copies (has to outlive |ret|)

[vasilii, 2014/08/04 16:20:59]

Done.

+                 autofill::ConstPasswordFormMap* ret,
+                 ScopedVector<autofill::PasswordForm>* deleter) {
+  ConstifyMap(map).swap(*ret);
+  deleter->clear();
+  for (autofill::ConstPasswordFormMap::iterator i = ret->begin();
+       i != ret->end(); ++i) {
+    autofill::PasswordForm* form(new autofill::PasswordForm(*i->second));

[vabr (Chromium), 2014/08/04 15:36:56]

Please don't let naked pointers contain new-allocated objects; if someone adds
code between this line and the following, they can introduce leaks.

Instead call new directly inside push_back and use back() to retrieve it for
i->second, or use a scoped_ptr and fill first i->second, and then release the
scoped_ptr into deleter.

[vasilii, 2014/08/04 16:20:59]

Done.

+    deleter->push_back(form);
+    i->second = form;
+  }
+}
+
+}  // namespace
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ManagePasswordsUIController);
 
@@ -93,7 +106,7 @@ void ManagePasswordsUIController::OnAutomaticPasswordSave(
 
 void ManagePasswordsUIController::OnPasswordAutofilled(
     const PasswordFormMap& password_form_map) {
-  password_form_map_ = ConstifyMap(password_form_map);
+  DeepCopyMap(password_form_map, &password_form_map_, &new_password_forms_);
   origin_ = password_form_map_.begin()->second->origin;
   state_ = password_manager::ui::MANAGE_STATE;
   UpdateBubbleAndIconVisibility();
@@ -101,7 +114,7 @@ void ManagePasswordsUIController::OnPasswordAutofilled(
 
 void ManagePasswordsUIController::OnBlacklistBlockedAutofill(
     const PasswordFormMap& password_form_map) {
-  password_form_map_ = ConstifyMap(password_form_map);
+  DeepCopyMap(password_form_map, &password_form_map_, &new_password_forms_);
   origin_ = password_form_map_.begin()->second->origin;
   state_ = password_manager::ui::BLACKLIST_STATE;
   UpdateBubbleAndIconVisibility();