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

Unified Diff: components/password_manager/core/browser/affiliation_backend.cc

Issue 1008373003: Integrate throttling logic into AffiliationBackend. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed comment. Created 5 years, 9 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/core/browser/affiliation_backend.cc
diff --git a/components/password_manager/core/browser/affiliation_backend.cc b/components/password_manager/core/browser/affiliation_backend.cc
index ec937608af23d26e17d6fdf7bce07e4672aea742..5b2c637beb046053f8284ef0a4b124068d858a6d 100644
--- a/components/password_manager/core/browser/affiliation_backend.cc
+++ b/components/password_manager/core/browser/affiliation_backend.cc
@@ -9,12 +9,12 @@
#include "base/bind.h"
#include "base/location.h"
#include "base/single_thread_task_runner.h"
-#include "base/task_runner.h"
-#include "base/thread_task_runner_handle.h"
#include "base/threading/thread_checker.h"
#include "base/time/clock.h"
+#include "base/time/tick_clock.h"
#include "base/time/time.h"
#include "components/password_manager/core/browser/affiliation_database.h"
+#include "components/password_manager/core/browser/affiliation_fetch_throttler.h"
#include "components/password_manager/core/browser/affiliation_fetcher.h"
#include "components/password_manager/core/browser/facet_manager.h"
#include "net/url_request/url_request_context_getter.h"
@@ -23,9 +23,13 @@ namespace password_manager {
AffiliationBackend::AffiliationBackend(
const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
- scoped_ptr<base::Clock> time_source)
+ const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+ scoped_ptr<base::Clock> time_source,
+ scoped_ptr<base::TickClock> time_tick_source)
: request_context_getter_(request_context_getter),
+ task_runner_(task_runner),
clock_(time_source.Pass()),
+ tick_clock_(time_tick_source.Pass()),
weak_ptr_factory_(this) {
DCHECK_LT(base::Time(), clock_->Now());
}
@@ -35,6 +39,11 @@ AffiliationBackend::~AffiliationBackend() {
void AffiliationBackend::Initialize(const base::FilePath& db_path) {
thread_checker_.reset(new base::ThreadChecker);
+
+ DCHECK(!throttler_);
+ throttler_.reset(
+ new AffiliationFetchThrottler(this, task_runner_, tick_clock_.get()));
+
cache_.reset(new AffiliationDatabase());
if (!cache_->Init(db_path)) {
// TODO(engedy): Implement this. crbug.com/437865.
@@ -99,20 +108,6 @@ FacetManager* AffiliationBackend::GetOrCreateFacetManager(
return facet_managers_.get(facet_uri);
}
-void AffiliationBackend::SendNetworkRequest() {
- DCHECK(!fetcher_);
-
- std::vector<FacetURI> requested_facet_uris;
- for (const auto& facet_manager_pair : facet_managers_) {
- if (facet_manager_pair.second->DoesRequireFetch())
- requested_facet_uris.push_back(facet_manager_pair.first);
- }
- DCHECK(!requested_facet_uris.empty());
- fetcher_.reset(AffiliationFetcher::Create(request_context_getter_.get(),
- requested_facet_uris, this));
- fetcher_->StartRequest();
-}
-
void AffiliationBackend::OnSendNotification(const FacetURI& facet_uri) {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
@@ -132,17 +127,14 @@ bool AffiliationBackend::ReadAffiliationsFromDatabase(
}
void AffiliationBackend::SignalNeedNetworkRequest() {
- // TODO(engedy): Add more sophisticated throttling logic. crbug.com/437865.
- if (fetcher_)
- return;
- SendNetworkRequest();
+ throttler_->SignalNetworkRequestNeeded();
}
void AffiliationBackend::RequestNotificationAtTime(const FacetURI& facet_uri,
base::Time time) {
// TODO(engedy): Avoid spamming the task runner; only ever schedule the first
// callback. crbug.com/437865.
- base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ task_runner_->PostDelayedTask(
FROM_HERE, base::Bind(&AffiliationBackend::OnSendNotification,
weak_ptr_factory_.GetWeakPtr(), facet_uri),
time - clock_->Now());
@@ -151,7 +143,9 @@ void AffiliationBackend::RequestNotificationAtTime(const FacetURI& facet_uri,
void AffiliationBackend::OnFetchSucceeded(
scoped_ptr<AffiliationFetcherDelegate::Result> result) {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
+
fetcher_.reset();
+ throttler_->InformOfNetworkRequestComplete(true);
for (const AffiliatedFacets& affiliated_facets : *result) {
AffiliatedFacetsWithUpdateTime affiliation;
@@ -182,7 +176,7 @@ void AffiliationBackend::OnFetchSucceeded(
// requests came in while the current fetch was in flight.
for (const auto& facet_manager_pair : facet_managers_) {
if (facet_manager_pair.second->DoesRequireFetch()) {
- SendNetworkRequest();
+ throttler_->SignalNetworkRequestNeeded();
return;
}
}
@@ -191,15 +185,46 @@ void AffiliationBackend::OnFetchSucceeded(
void AffiliationBackend::OnFetchFailed() {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
- // TODO(engedy): Implement this. crbug.com/437865.
- NOTIMPLEMENTED();
+ fetcher_.reset();
+ throttler_->InformOfNetworkRequestComplete(false);
+
+ // Trigger a retry if a fetch is still needed.
+ for (const auto& facet_manager_pair : facet_managers_) {
+ if (facet_manager_pair.second->DoesRequireFetch()) {
+ throttler_->SignalNetworkRequestNeeded();
+ return;
+ }
+ }
}
void AffiliationBackend::OnMalformedResponse() {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
- // TODO(engedy): Implement this. crbug.com/437865.
- NOTIMPLEMENTED();
+ // TODO(engedy): Potentially handle this case differently. crbug.com/437865.
+ OnFetchFailed();
+}
+
+bool AffiliationBackend::OnCanSendNetworkRequest() {
+ DCHECK(!fetcher_);
+ std::vector<FacetURI> requested_facet_uris;
+ for (const auto& facet_manager_pair : facet_managers_) {
+ if (facet_manager_pair.second->DoesRequireFetch())
+ requested_facet_uris.push_back(facet_manager_pair.first);
+ }
+
+ // In case a request is no longer needed, return false to indicate this.
+ if (requested_facet_uris.empty())
+ return false;
+
+ fetcher_.reset(AffiliationFetcher::Create(request_context_getter_.get(),
+ requested_facet_uris, this));
+ fetcher_->StartRequest();
+ return true;
+}
+
+void AffiliationBackend::SetThrottlerForTesting(
+ scoped_ptr<AffiliationFetchThrottler> throttler) {
+ throttler_ = throttler.Pass();
}
} // namespace password_manager

Powered by Google App Engine
This is Rietveld 408576698