Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(578)

Unified Diff: components/password_manager/content/browser/credential_manager_dispatcher.cc

Issue 866983003: GetLoginsRequest: Use ScopedVector to express ownership of forms (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@324291_scopedvector
Patch Set: Fix Mac unittest Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 imp
vabr (Chromium) 2015/02/06 14:16:05 I checked with Mike, and corrected the comment bas
+ // 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)

Powered by Google App Engine
This is Rietveld 408576698