PasswordStore: Use ScopedVector to express ownership of forms

chrome/browser/password_manager/password_store_x.cc

Index: chrome/browser/password_manager/password_store_x.cc
diff --git a/chrome/browser/password_manager/password_store_x.cc b/chrome/browser/password_manager/password_store_x.cc
index ab4df822608cfacf6c4fac497d4d9312d5432f8b..e0e756249b923cab27beeee88755dc5aeb07ccf8 100644
--- a/chrome/browser/password_manager/password_store_x.cc
+++ b/chrome/browser/password_manager/password_store_x.cc
@@ -24,7 +24,6 @@ using content::BrowserThread;
 using password_manager::PasswordStoreChange;
 using password_manager::PasswordStoreChangeList;
 using password_manager::PasswordStoreDefault;
-using std::vector;
 
 namespace {
 
@@ -130,7 +129,8 @@ struct LoginLessThan {
 };
 }  // anonymous namespace
 
-void PasswordStoreX::SortLoginsByOrigin(NativeBackend::PasswordFormList* list) {
+void PasswordStoreX::SortLoginsByOrigin(
+    std::vector<autofill::PasswordForm*>* list) {
   // In login_database.cc, the query has ORDER BY origin_url. Simulate that.
   std::sort(list->begin(), list->end(), LoginLessThan());
 }
@@ -140,9 +140,9 @@ void PasswordStoreX::GetLoginsImpl(
     AuthorizationPromptPolicy prompt_policy,
     const ConsumerCallbackRunner& callback_runner) {
   CheckMigration();
-  std::vector<autofill::PasswordForm*> matched_forms;
+  ScopedVector<autofill::PasswordForm> matched_forms;
   if (use_native_backend() && backend_->GetLogins(form, &matched_forms)) {
-    SortLoginsByOrigin(&matched_forms);
+    SortLoginsByOrigin(&matched_forms.get());
     // The native backend may succeed and return no data even while locked, if
     // the query did not match anything stored. So we continue to allow fallback
     // until we perform a write operation, or until a read returns actual data.
@@ -154,13 +154,17 @@ void PasswordStoreX::GetLoginsImpl(
     return;
   }
   // The consumer will be left hanging unless we reply.
-  callback_runner.Run(matched_forms);
+  callback_runner.Run(&matched_forms);
 }
 
 void PasswordStoreX::GetAutofillableLoginsImpl(GetLoginsRequest* request) {
   CheckMigration();
+  ScopedVector<autofill::PasswordForm> obtained_forms;
   if (use_native_backend() &&
-      backend_->GetAutofillableLogins(request->result())) {
+      backend_->GetAutofillableLogins(&obtained_forms)) {
+    request->result()->swap(obtained_forms.get());
+    obtained_forms.weak_clear();  // TODO(vabr): Change request to have a

[vasilii, 2015/01/27 20:45:51]

Why? request->result() was probably empty. However, if not then we leak here.

[vabr (Chromium), 2015/01/28 13:27:36]

You are correct.
Done.

+                                  // ScopedVector instead.
     SortLoginsByOrigin(request->result());
     // See GetLoginsImpl() for why we disallow fallback conditionally here.
     if (request->result()->size() > 0)
@@ -175,8 +179,11 @@ void PasswordStoreX::GetAutofillableLoginsImpl(GetLoginsRequest* request) {
 
 void PasswordStoreX::GetBlacklistLoginsImpl(GetLoginsRequest* request) {
   CheckMigration();
-  if (use_native_backend() &&
-      backend_->GetBlacklistLogins(request->result())) {
+  ScopedVector<autofill::PasswordForm> obtained_forms;
+  if (use_native_backend() && backend_->GetBlacklistLogins(&obtained_forms)) {
+    request->result()->swap(obtained_forms.get());
+    obtained_forms.weak_clear();  // TODO(vabr): Change request to have a

[vasilii, 2015/01/27 20:45:51]

Same as above.

[vabr (Chromium), 2015/01/28 13:27:36]

Done.

+                                  // ScopedVector instead.
     SortLoginsByOrigin(request->result());
     // See GetLoginsImpl() for why we disallow fallback conditionally here.
     if (request->result()->size() > 0)
@@ -189,7 +196,8 @@ void PasswordStoreX::GetBlacklistLoginsImpl(GetLoginsRequest* request) {
   ForwardLoginsResult(request);
 }
 
-bool PasswordStoreX::FillAutofillableLogins(vector<PasswordForm*>* forms) {
+bool PasswordStoreX::FillAutofillableLogins(
+    ScopedVector<autofill::PasswordForm>* forms) {
   CheckMigration();
   if (use_native_backend() && backend_->GetAutofillableLogins(forms)) {
     // See GetLoginsImpl() for why we disallow fallback conditionally here.
@@ -202,7 +210,8 @@ bool PasswordStoreX::FillAutofillableLogins(vector<PasswordForm*>* forms) {
   return false;
 }
 
-bool PasswordStoreX::FillBlacklistLogins(vector<PasswordForm*>* forms) {
+bool PasswordStoreX::FillBlacklistLogins(
+    ScopedVector<autofill::PasswordForm>* forms) {
   CheckMigration();
   if (use_native_backend() && backend_->GetBlacklistLogins(forms)) {
     // See GetLoginsImpl() for why we disallow fallback conditionally here.
@@ -250,7 +259,7 @@ bool PasswordStoreX::allow_default_store() {
 
 ssize_t PasswordStoreX::MigrateLogins() {
   DCHECK(backend_.get());
-  vector<PasswordForm*> forms;
+  ScopedVector<autofill::PasswordForm> forms;
   bool ok = PasswordStoreDefault::FillAutofillableLogins(&forms) &&
       PasswordStoreDefault::FillBlacklistLogins(&forms);
   if (ok) {
@@ -283,6 +292,5 @@ ssize_t PasswordStoreX::MigrateLogins() {
     }
   }
   ssize_t result = ok ? forms.size() : -1;
-  STLDeleteElements(&forms);
   return result;
 }