Index: components/password_manager/core/browser/password_store.cc |
diff --git a/components/password_manager/core/browser/password_store.cc b/components/password_manager/core/browser/password_store.cc |
index 276090ab12f0b3fac862c57be77c9bde9dc49031..9a9ce344433d0b1f0d4a7f4242e29cc9c5e8dc8d 100644 |
--- a/components/password_manager/core/browser/password_store.cc |
+++ b/components/password_manager/core/browser/password_store.cc |
@@ -29,11 +29,12 @@ namespace { |
// Takes ownership of the elements in |result|, passing ownership to |consumer| |
// if it is still alive. |
void MaybeCallConsumerCallback(base::WeakPtr<PasswordStoreConsumer> consumer, |
- scoped_ptr<std::vector<PasswordForm*>> result) { |
- if (consumer.get()) |
- consumer->OnGetPasswordStoreResults(*result); |
- else |
- STLDeleteElements(result.get()); |
+ ScopedVector<autofill::PasswordForm> result) { |
+ if (consumer) { |
+ consumer->results()->clear(); |
vasilii
2015/02/03 19:22:16
Do you need it?
vabr (Chromium)
2015/02/04 16:13:44
I don't, but I left it to make it clear that the c
|
+ consumer->results()->swap(result); |
+ consumer->OnGetPasswordStoreResults(); |
+ } |
} |
// http://crbug.com/404012. Let's see where the empty fields come from. |
@@ -48,8 +49,12 @@ void CheckForEmptyUsernameAndPassword(const PasswordForm& form) { |
PasswordStore::GetLoginsRequest::GetLoginsRequest( |
PasswordStoreConsumer* consumer) |
- : consumer_weak_(consumer->GetWeakPtr()), |
- result_(new std::vector<PasswordForm*>()) { |
+ : consumer_weak_(consumer->GetWeakPtr()) { |
+ // TODO(vabr): As long as |thread_checker_| gets constructed without failures, |
+ // this DCHECK is always trivially true. It looks like we could get rid of the |
+ // |thread_checker_| unless it was meant to make sure that threads are |
+ // actually present in relevant tests. But in that case, we should do better |
+ // than to hog production code because of tests. |
DCHECK(thread_checker_.CalledOnValidThread()); |
vasilii
2015/02/03 19:22:17
Looks like this is the only place where it's used.
vabr (Chromium)
2015/02/04 16:13:44
It is explained in the TODO I added above. We migh
|
origin_loop_ = base::MessageLoopProxy::current(); |
} |
@@ -59,23 +64,21 @@ PasswordStore::GetLoginsRequest::~GetLoginsRequest() { |
void PasswordStore::GetLoginsRequest::ApplyIgnoreLoginsCutoff() { |
if (!ignore_logins_cutoff_.is_null()) { |
- // Count down rather than up since we may be deleting elements. |
- // Note that in principle it could be more efficient to copy the whole array |
- // since that's worst-case linear time, but we expect that elements will be |
- // deleted rarely and lists will be small, so this avoids the copies. |
- for (size_t i = result_->size(); i > 0; --i) { |
- if ((*result_)[i - 1]->date_created < ignore_logins_cutoff_) { |
- delete (*result_)[i - 1]; |
- result_->erase(result_->begin() + (i - 1)); |
+ ScopedVector<autofill::PasswordForm> remaining_logins; |
+ remaining_logins.reserve(result_.size()); |
+ for (auto& login : result_) { |
+ if (login->date_created >= ignore_logins_cutoff_) { |
+ remaining_logins.push_back(login); |
+ login = nullptr; |
} |
} |
+ remaining_logins.swap(result_); |
} |
} |
void PasswordStore::GetLoginsRequest::ForwardResult() { |
origin_loop_->PostTask(FROM_HERE, |
- base::Bind(&MaybeCallConsumerCallback, |
- consumer_weak_, |
+ base::Bind(&MaybeCallConsumerCallback, consumer_weak_, |
base::Passed(result_.Pass()))); |
vasilii
2015/02/03 19:22:16
base::Passed(&result_)?
vabr (Chromium)
2015/02/04 16:13:44
Done.
|
} |
@@ -141,11 +144,11 @@ void PasswordStore::GetLogins(const PasswordForm& form, |
{ 2012, 1, 0, 1, 0, 0, 0, 0 }; // 00:00 Jan 1 2012 |
ignore_logins_cutoff = base::Time::FromUTCExploded(exploded_cutoff); |
} |
- GetLoginsRequest* request = new GetLoginsRequest(consumer); |
+ scoped_ptr<GetLoginsRequest> request(new GetLoginsRequest(consumer)); |
request->set_ignore_logins_cutoff(ignore_logins_cutoff); |
- ConsumerCallbackRunner callback_runner = base::Bind( |
- &PasswordStore::CopyAndForwardLoginsResult, base::Owned(request)); |
+ ConsumerCallbackRunner callback_runner = |
+ base::Bind(&CopyAndForwardLoginsResult, base::Passed(request.Pass())); |
vasilii
2015/02/03 19:22:16
base::Passed(&request)?
vabr (Chromium)
2015/02/04 16:13:44
Done.
|
ScheduleTask(base::Bind(&PasswordStore::GetLoginsImpl, this, form, |
prompt_policy, callback_runner)); |
} |
@@ -208,18 +211,18 @@ PasswordStore::GetBackgroundTaskRunner() { |
} |
// static |
-void PasswordStore::ForwardLoginsResult(GetLoginsRequest* request) { |
+void PasswordStore::ForwardLoginsResult(scoped_ptr<GetLoginsRequest> request) { |
request->ApplyIgnoreLoginsCutoff(); |
request->ForwardResult(); |
} |
// static |
void PasswordStore::CopyAndForwardLoginsResult( |
- PasswordStore::GetLoginsRequest* request, |
+ scoped_ptr<PasswordStore::GetLoginsRequest> request, |
ScopedVector<autofill::PasswordForm> matched_forms) { |
- // Move the contents of |matched_forms| into the request. |
- request->result()->swap(matched_forms.get()); |
- ForwardLoginsResult(request); |
+ DCHECK(request->result()->empty()); |
+ request->result()->swap(matched_forms); |
+ ForwardLoginsResult(request.Pass()); |
} |
void PasswordStore::LogStatsForBulkDeletion(int num_deletions) { |
@@ -247,11 +250,10 @@ void PasswordStore::NotifyLoginsChanged( |
template <typename BackendFunc> |
void PasswordStore::Schedule(BackendFunc func, |
vasilii
2015/02/03 19:22:17
Optional: you can get rid of the template because
vabr (Chromium)
2015/02/04 16:13:44
Sounds reasonable, the function signature was rest
|
PasswordStoreConsumer* consumer) { |
- GetLoginsRequest* request = new GetLoginsRequest(consumer); |
+ scoped_ptr<GetLoginsRequest> request(new GetLoginsRequest(consumer)); |
consumer->cancelable_task_tracker()->PostTask( |
- GetBackgroundTaskRunner().get(), |
- FROM_HERE, |
- base::Bind(func, this, base::Owned(request))); |
+ GetBackgroundTaskRunner().get(), FROM_HERE, |
+ base::Bind(func, this, base::Passed(request.Pass()))); |
vasilii
2015/02/03 19:22:17
base::Passed(&request);
vabr (Chromium)
2015/02/04 16:13:44
Done.
|
} |
void PasswordStore::WrapModificationTask(ModificationTask task) { |