PasswordStore: Use ScopedVector to express ownership of forms

chrome/browser/password_manager/password_store_win.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_store_win.h"
 
 #include <map>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 36 matching lines @@
     PasswordForm* form;
     PasswordStoreWin::ConsumerCallbackRunner callback_runner;
   };
 
   // Holds info associated with in-flight GetIE7Login requests.
   typedef std::map<PasswordWebDataService::Handle, RequestInfo>
       PendingRequestMap;
 
   // Gets logins from IE7 if no others are found. Also copies them into
   // Chrome's WebDatabase so we don't need to look next time.
-  std::vector<autofill::PasswordForm*> GetIE7Results(
-      const WDTypedResult* result,
-      const PasswordForm& form);
+  void GetIE7Results(const WDTypedResult* result,
+                     const PasswordForm& form,
+                     ScopedVector<autofill::PasswordForm>* matched_forms);
 
   // WebDataServiceConsumer implementation.
   virtual void OnWebDataServiceRequestDone(
       PasswordWebDataService::Handle handle,
       const WDTypedResult* result) override;
 
   scoped_refptr<PasswordWebDataService> web_data_service_;
 
   // This creates a cycle between us and PasswordStore. The cycle is broken
   // from PasswordStoreWin::Shutdown, which deletes us.
@@ skipping 20 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   IE7PasswordInfo info;
   info.url_hash =
       ie7_password::GetUrlHash(base::UTF8ToWide(form.origin.spec()));
   PasswordWebDataService::Handle handle =
       web_data_service_->GetIE7Login(info, this);
   pending_requests_[handle] =
       RequestInfo(new PasswordForm(form), callback_runner);
 }
 
-std::vector<PasswordForm*> PasswordStoreWin::DBHandler::GetIE7Results(
-    const WDTypedResult *result,
-    const PasswordForm& form) {
+void PasswordStoreWin::DBHandler::GetIE7Results(

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

I think it's cleaner to return ScopedVector<autofill::PasswordForm>.

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

Done.

+    const WDTypedResult* result,
+    const PasswordForm& form,
+    ScopedVector<autofill::PasswordForm>* matched_forms) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-  std::vector<PasswordForm*> matching_forms;
-
   const WDResult<IE7PasswordInfo>* r =
       static_cast<const WDResult<IE7PasswordInfo>*>(result);
   IE7PasswordInfo info = r->GetValue();
 
   if (!info.encrypted_data.empty()) {
     // We got a result.
     // Delete the entry. If it's good we will add it to the real saved password
     // table.
     web_data_service_->RemoveIE7Login(info);
     std::vector<ie7_password::DecryptedCredentials> credentials;
     base::string16 url = base::ASCIIToUTF16(form.origin.spec());
     if (ie7_password::DecryptPasswords(url,
                                        info.encrypted_data,
                                        &credentials)) {
       for (size_t i = 0; i < credentials.size(); ++i) {
         PasswordForm* autofill = new PasswordForm();
         autofill->username_value = credentials[i].username;
         autofill->password_value = credentials[i].password;
         autofill->signon_realm = form.signon_realm;
         autofill->origin = form.origin;
         autofill->preferred = true;
         autofill->ssl_valid = form.origin.SchemeIsSecure();
         autofill->date_created = info.date_created;
 
-        matching_forms.push_back(autofill);
+        matched_forms->push_back(autofill);
         // Add this PasswordForm to the saved password table. We're on the DB
         // thread already, so we use AddLoginImpl.
         password_store_->AddLoginImpl(*autofill);
       }
     }
   }
-  return matching_forms;
 }
 
 void PasswordStoreWin::DBHandler::OnWebDataServiceRequestDone(
     PasswordWebDataService::Handle handle,
     const WDTypedResult* result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
 
   PendingRequestMap::iterator i = pending_requests_.find(handle);
   DCHECK(i != pending_requests_.end());
 
   scoped_ptr<PasswordForm> form(i->second.form);
   PasswordStoreWin::ConsumerCallbackRunner callback_runner(
       i->second.callback_runner);
   pending_requests_.erase(i);
 
   if (!result) {
     // The WDS returns NULL if it is shutting down. Run callback with empty
     // result.
-    callback_runner.Run(std::vector<autofill::PasswordForm*>());
+    ScopedVector<autofill::PasswordForm> dummy;
+    callback_runner.Run(&dummy);
     return;
   }
 
   DCHECK_EQ(PASSWORD_IE7_RESULT, result->GetType());
-  std::vector<autofill::PasswordForm*> matched_forms =
-      GetIE7Results(result, *form);
+  ScopedVector<autofill::PasswordForm> matched_forms;
+  GetIE7Results(result, *form, &matched_forms);
 
-  callback_runner.Run(matched_forms);
+  callback_runner.Run(&matched_forms);
 }
 
 PasswordStoreWin::PasswordStoreWin(
     scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner,
     scoped_refptr<base::SingleThreadTaskRunner> db_thread_runner,
     scoped_ptr<password_manager::LoginDatabase> login_db,
     const scoped_refptr<PasswordWebDataService>& web_data_service)
     : PasswordStoreDefault(main_thread_runner,
                            db_thread_runner,
                            login_db.Pass()) {
@@ skipping 11 matching lines @@
 void PasswordStoreWin::Shutdown() {
   BrowserThread::PostTask(
       BrowserThread::DB, FROM_HERE,
       base::Bind(&PasswordStoreWin::ShutdownOnDBThread, this));
   PasswordStoreDefault::Shutdown();
 }
 
 void PasswordStoreWin::GetIE7LoginIfNecessary(
     const PasswordForm& form,
     const ConsumerCallbackRunner& callback_runner,
-    const std::vector<autofill::PasswordForm*>& matched_forms) {
+    ScopedVector<autofill::PasswordForm>* matched_forms) {

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

If you change the type of ConsumerCallbackRunner then it'll be much cleaner.
GetLoginImpl transfers the ownership to GetIE7LoginIfNecessary which transfers
it to callback_runner.

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

Done.

   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-  if (matched_forms.empty() && db_handler_.get()) {
+  if (matched_forms->empty() && db_handler_) {
     db_handler_->GetIE7Login(form, callback_runner);
   } else {
     // No need to get IE7 login.
     callback_runner.Run(matched_forms);
   }
 }
 
 void PasswordStoreWin::GetLoginsImpl(
     const PasswordForm& form,
     AuthorizationPromptPolicy prompt_policy,
     const ConsumerCallbackRunner& callback_runner) {
   ConsumerCallbackRunner get_ie7_login =
       base::Bind(&PasswordStoreWin::GetIE7LoginIfNecessary,
                  this, form, callback_runner);
   PasswordStoreDefault::GetLoginsImpl(form, prompt_policy, get_ie7_login);
 }