GetLoginsRequest: Use ScopedVector to express ownership of forms

components/password_manager/content/browser/credential_manager_dispatcher.cc

Index: components/password_manager/content/browser/credential_manager_dispatcher.cc
diff --git a/components/password_manager/content/browser/credential_manager_dispatcher.cc b/components/password_manager/content/browser/credential_manager_dispatcher.cc
index d459cfa4eaac65724b90dcd874cfc65133b1eb3b..bce9c2d46b8ee8e7127e4b47276cf073c755d80a 100644
--- a/components/password_manager/content/browser/credential_manager_dispatcher.cc
+++ b/components/password_manager/content/browser/credential_manager_dispatcher.cc
@@ -26,6 +26,7 @@ namespace password_manager {
 class CredentialManagerDispatcher::PendingRequestTask
     : public PasswordStoreConsumer {
  public:
+  // TODO(mkwst): De-inline the methods in this class. http://goo.gl/RmFwKd
   PendingRequestTask(CredentialManagerDispatcher* const dispatcher,
                      int request_id,
                      bool request_zero_click_only,
@@ -43,52 +44,7 @@ class CredentialManagerDispatcher::PendingRequestTask
 
   // PasswordStoreConsumer implementation.
   void OnGetPasswordStoreResults(
-      const std::vector<autofill::PasswordForm*>& results) override {
-    // We own the PasswordForm instances, so we're responsible for cleaning
-    // up the instances we don't add to |local_results| or |federated_results|.
-    //
-    // TODO(mkwst): Switch this and PromptUserToChooseCredentials() to use
-    // ScopedVector.
-    std::vector<autofill::PasswordForm*> local_results;
-    std::vector<autofill::PasswordForm*> federated_results;
-    for (autofill::PasswordForm* form : results) {
-      if (form->origin == origin_)
-        local_results.push_back(form);
-      else if (federations_.count(form->origin.spec()))
-        federated_results.push_back(form);
-      else
-        delete form;
-    }
-
-    if ((local_results.empty() && federated_results.empty()) ||
-        dispatcher_->web_contents()->GetLastCommittedURL().GetOrigin() !=
-            origin_) {
-      dispatcher_->SendCredential(id_, CredentialInfo());
-      return;
-    }
-    if (local_results.size() == 1 && dispatcher_->IsZeroClickAllowed()) {
-      // TODO(mkwst): Use the `one_time_disable_zero_click` flag on the result
-      // to prevent auto-sign-in, once that flag is implemented.
-      CredentialInfo info(*local_results[0],
-                          local_results[0]->federation_url.is_empty()
-                              ? CredentialType::CREDENTIAL_TYPE_LOCAL
-                              : CredentialType::CREDENTIAL_TYPE_FEDERATED);
-      STLDeleteElements(&local_results);
-      STLDeleteElements(&federated_results);
-      dispatcher_->SendCredential(id_, info);
-      return;
-    }
-
-    if (zero_click_only_ ||
-        !dispatcher_->client()->PromptUserToChooseCredentials(
-            local_results, federated_results,
-            base::Bind(&CredentialManagerDispatcher::SendCredential,
-                       base::Unretained(dispatcher_), id_))) {
-      STLDeleteElements(&local_results);
-      STLDeleteElements(&federated_results);
-      dispatcher_->SendCredential(id_, CredentialInfo());
-    }
-  }
+      ScopedVector<autofill::PasswordForm> results) override;
 
  private:
   // Backlink to the CredentialManagerDispatcher that owns this object.
@@ -102,6 +58,46 @@ class CredentialManagerDispatcher::PendingRequestTask
   DISALLOW_COPY_AND_ASSIGN(PendingRequestTask);
 };
 
+void CredentialManagerDispatcher::PendingRequestTask::OnGetPasswordStoreResults(
+    ScopedVector<autofill::PasswordForm> results) {
+  ScopedVector<autofill::PasswordForm> local_results;
+  ScopedVector<autofill::PasswordForm> federated_results;
+  for (auto& form : results) {
+    if (form->origin == origin_) {
+      local_results.push_back(form);
+      form = nullptr;
+    } else if (federations_.count(form->origin.spec())) {
+      federated_results.push_back(form);
+      form = nullptr;
+    }
+  }
+
+  if ((local_results.empty() && federated_results.empty()) ||
+      dispatcher_->web_contents()->GetLastCommittedURL().GetOrigin() !=
+          origin_) {
+    dispatcher_->SendCredential(id_, CredentialInfo());
+    return;
+  }
+  if (local_results.size() == 1 && dispatcher_->IsZeroClickAllowed()) {
+    // TODO(mkwst): Use the `one_time_disable_zero_click` flag on the result

[vasilii, 2015/02/05 19:23:27]

The flag is 'skip_zero_click' and it's already implemented.

[vabr (Chromium), 2015/02/06 14:16:05]

I checked with Mike, and corrected the comment based on that.
Done.

+    // to prevent auto-sign-in, once that flag is implemented.
+    CredentialInfo info(*local_results[0],
+                        local_results[0]->federation_url.is_empty()
+                            ? CredentialType::CREDENTIAL_TYPE_LOCAL
+                            : CredentialType::CREDENTIAL_TYPE_FEDERATED);
+    dispatcher_->SendCredential(id_, info);
+    return;
+  }
+
+  if (zero_click_only_ ||
+      !dispatcher_->client()->PromptUserToChooseCredentials(
+          local_results.Pass(), federated_results.Pass(),
+          base::Bind(&CredentialManagerDispatcher::SendCredential,
+                     base::Unretained(dispatcher_), id_))) {
+    dispatcher_->SendCredential(id_, CredentialInfo());
+  }
+}
+
 CredentialManagerDispatcher::CredentialManagerDispatcher(
     content::WebContents* web_contents,
     PasswordManagerClient* client)