[EoS] Add backoff support EoS url fetcher
Also make EoS url fetcher support immediate or background fetch with
different backoff policy.
Bug: 867488
Change-Id: If41abed1f85781cf84b9038f600e0a94c6c1524c
Reviewed-on: https://chromium-review.googlesource.com/c/1263053
Commit-Queue: Jian Li <jianli@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597231}diff --git a/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTask.java b/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTask.java
index c4c2b87..2fefb64 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTask.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTask.java
@@ -65,8 +65,8 @@
}
mTaskFinishedCallback = callback;
- ExploreSitesBridge.updateCatalogFromNetwork(
- getProfile(), (ignored) -> mTaskFinishedCallback.taskFinished(false));
+ ExploreSitesBridge.updateCatalogFromNetwork(getProfile(), false /*isImmediateFetch*/,
+ (ignored) -> mTaskFinishedCallback.taskFinished(false));
}
@Override
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBridge.java
index 97b4f2d..35a00985 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBridge.java
@@ -48,8 +48,8 @@
* Causes a network request for updating the catalog.
*/
public static void updateCatalogFromNetwork(
- Profile profile, Callback<Boolean> finishedCallback) {
- nativeUpdateCatalogFromNetwork(profile, finishedCallback);
+ Profile profile, boolean isImmediateFetch, Callback<Boolean> finishedCallback) {
+ nativeUpdateCatalogFromNetwork(profile, isImmediateFetch, finishedCallback);
}
/**
@@ -73,5 +73,5 @@
Profile profile, int siteID, Callback<Bitmap> callback);
private static native void nativeUpdateCatalogFromNetwork(
- Profile profile, Callback<Boolean> callback);
+ Profile profile, boolean isImmediateFetch, Callback<Boolean> callback);
}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java b/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
index b573acc..d5799a5 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
@@ -121,8 +121,8 @@
*/
private void gotEspCatalog(List<ExploreSitesCategory> categoryList) {
if (categoryList == null || categoryList.size() == 0) {
- ExploreSitesBridge.updateCatalogFromNetwork(
- mProfile, (Boolean success) -> { updateCategoryIcons(); });
+ ExploreSitesBridge.updateCatalogFromNetwork(mProfile, true /*isImmediateFetch*/,
+ (Boolean success) -> { updateCategoryIcons(); });
}
// Initialize with defaults right away.
initializeCategoryTiles(categoryList);
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTaskUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTaskUnitTest.java
index 274cf44..a3b70ed 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTaskUnitTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTaskUnitTest.java
@@ -68,7 +68,7 @@
@Implementation
public static void updateCatalogFromNetwork(
- Profile profile, Callback<Void> finishedCallback) {
+ Profile profile, boolean isImmediateFetch, Callback<Void> finishedCallback) {
mUpdateCatalogFinishedCallback = finishedCallback;
}
}
diff --git a/chrome/browser/android/explore_sites/explore_sites_bridge.cc b/chrome/browser/android/explore_sites/explore_sites_bridge.cc
index 7da3aab..e2dfde387 100644
--- a/chrome/browser/android/explore_sites/explore_sites_bridge.cc
+++ b/chrome/browser/android/explore_sites/explore_sites_bridge.cc
@@ -142,6 +142,7 @@
JNIEnv* env,
const JavaParamRef<jclass>& j_caller,
const JavaParamRef<jobject>& j_profile,
+ jboolean is_immediate_fetch,
const JavaParamRef<jobject>& j_callback_obj) {
Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
DCHECK(profile);
@@ -163,7 +164,7 @@
}
service->UpdateCatalogFromNetwork(
- accept_languages,
+ static_cast<bool>(is_immediate_fetch), accept_languages,
base::BindOnce(&UpdateCatalogDone,
ScopedJavaGlobalRef<jobject>(j_callback_obj)));
}
diff --git a/chrome/browser/android/explore_sites/explore_sites_fetcher.cc b/chrome/browser/android/explore_sites/explore_sites_fetcher.cc
index 87c5094..7adf18d6 100644
--- a/chrome/browser/android/explore_sites/explore_sites_fetcher.cc
+++ b/chrome/browser/android/explore_sites/explore_sites_fetcher.cc
@@ -91,66 +91,101 @@
} // namespace
+const net::BackoffEntry::Policy
+ ExploreSitesFetcher::kImmediateFetchBackoffPolicy = {
+ 0, // Number of initial errors to ignore without backoff.
+ 1 * 1000, // Initial delay for backoff in ms: 1 second.
+ 2, // Factor to multiply for exponential backoff.
+ 0, // Fuzzing percentage.
+ 4 * 1000, // Maximum time to delay requests in ms: 4 seconds.
+ -1, // Don't discard entry even if unused.
+ false // Don't use initial delay unless the last was an error.
+};
+const int ExploreSitesFetcher::kMaxFailureCountForImmediateFetch = 3;
+
+const net::BackoffEntry::Policy
+ ExploreSitesFetcher::kBackgroundFetchBackoffPolicy = {
+ 0, // Number of initial errors to ignore without backoff.
+ 5 * 1000, // Initial delay for backoff in ms: 5 seconds.
+ 2, // Factor to multiply for exponential backoff.
+ 0, // Fuzzing percentage.
+ 320 * 1000, // Maximum time to delay requests in ms: 320 seconds.
+ -1, // Don't discard entry even if unused.
+ false // Don't use initial delay unless the last was an error.
+};
+const int ExploreSitesFetcher::kMaxFailureCountForBackgroundFetch = 7;
+
std::unique_ptr<ExploreSitesFetcher> ExploreSitesFetcher::CreateForGetCatalog(
- Callback callback,
- const std::string catalog_version,
- const std::string accept_languages,
- scoped_refptr<network::SharedURLLoaderFactory> loader_factory) {
+ bool is_immediate_fetch,
+ const std::string& catalog_version,
+ const std::string& accept_languages,
+ scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
+ Callback callback) {
GURL url = GetCatalogURL();
- return base::WrapUnique(
- new ExploreSitesFetcher(std::move(callback), url, catalog_version,
- accept_languages, loader_factory));
+ return base::WrapUnique(new ExploreSitesFetcher(
+ is_immediate_fetch, url, catalog_version, accept_languages,
+ loader_factory, std::move(callback)));
}
std::unique_ptr<ExploreSitesFetcher>
ExploreSitesFetcher::CreateForGetCategories(
- Callback callback,
- const std::string catalog_version,
- const std::string accept_languages,
- scoped_refptr<network::SharedURLLoaderFactory> loader_factory) {
+ bool is_immediate_fetch,
+ const std::string& catalog_version,
+ const std::string& accept_languages,
+ scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
+ Callback callback) {
GURL url = GetCategoriesURL();
- return base::WrapUnique(
- new ExploreSitesFetcher(std::move(callback), url, catalog_version,
- accept_languages, loader_factory));
+ return base::WrapUnique(new ExploreSitesFetcher(
+ is_immediate_fetch, url, catalog_version, accept_languages,
+ loader_factory, std::move(callback)));
}
ExploreSitesFetcher::ExploreSitesFetcher(
- Callback callback,
+ bool is_immediate_fetch,
const GURL& url,
- const std::string catalog_version,
- const std::string accept_languages,
- scoped_refptr<network::SharedURLLoaderFactory> loader_factory)
- : callback_(std::move(callback)),
+ const std::string& catalog_version,
+ const std::string& accept_languages,
+ scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
+ Callback callback)
+ : accept_languages_(accept_languages),
+ backoff_entry_(is_immediate_fetch ? &kImmediateFetchBackoffPolicy
+ : &kBackgroundFetchBackoffPolicy),
+ max_failure_count_(is_immediate_fetch
+ ? kMaxFailureCountForImmediateFetch
+ : kMaxFailureCountForBackgroundFetch),
+ callback_(std::move(callback)),
url_loader_factory_(loader_factory),
weak_factory_(this) {
base::Version version = version_info::GetVersion();
std::string channel_name = chrome::GetChannelName();
- std::string client_version =
- base::StringPrintf("%d.%d.%d.%s.chrome",
- version.components()[0], // Major
- version.components()[2], // Build
- version.components()[3], // Patch
- channel_name.c_str());
- GURL final_url =
+ client_version_ = base::StringPrintf("%d.%d.%d.%s.chrome",
+ version.components()[0], // Major
+ version.components()[2], // Build
+ version.components()[3], // Patch
+ channel_name.c_str());
+ request_url_ =
net::AppendOrReplaceQueryParameter(url, "country_code", GetCountry());
- final_url = net::AppendOrReplaceQueryParameter(final_url, "version_token",
- catalog_version);
- DVLOG(1) << "Final URL: " << final_url.spec();
+ request_url_ = net::AppendOrReplaceQueryParameter(
+ request_url_, "version_token", catalog_version);
+}
+ExploreSitesFetcher::~ExploreSitesFetcher() {}
+
+void ExploreSitesFetcher::Start() {
auto resource_request = std::make_unique<network::ResourceRequest>();
- resource_request->url = final_url;
+ resource_request->url = request_url_;
resource_request->method = kRequestMethod;
bool is_stable_channel =
chrome::GetChannel() == version_info::Channel::STABLE;
std::string api_key = is_stable_channel ? google_apis::GetAPIKey()
: google_apis::GetNonStableAPIKey();
resource_request->headers.SetHeader("x-goog-api-key", api_key);
- resource_request->headers.SetHeader("X-Client-Version", client_version);
+ resource_request->headers.SetHeader("X-Client-Version", client_version_);
resource_request->headers.SetHeader(net::HttpRequestHeaders::kContentType,
kRequestContentType);
- if (!accept_languages.empty()) {
+ if (!accept_languages_.empty()) {
resource_request->headers.SetHeader(
- net::HttpRequestHeaders::kAcceptLanguage, accept_languages);
+ net::HttpRequestHeaders::kAcceptLanguage, accept_languages_);
}
// Get field trial value, if any.
@@ -169,15 +204,19 @@
weak_factory_.GetWeakPtr()));
}
-ExploreSitesFetcher::~ExploreSitesFetcher() {}
-
void ExploreSitesFetcher::OnSimpleLoaderComplete(
std::unique_ptr<std::string> response_body) {
ExploreSitesRequestStatus status = HandleResponseCode();
if (response_body && response_body->empty()) {
DVLOG(1) << "Failed to get response or empty response";
- status = ExploreSitesRequestStatus::kShouldRetry;
+ status = ExploreSitesRequestStatus::kFailure;
+ }
+
+ if (status == ExploreSitesRequestStatus::kFailure &&
+ !disable_retry_for_testing_) {
+ RetryWithBackoff();
+ return;
}
std::move(callback_).Run(status, std::move(response_body));
@@ -193,18 +232,33 @@
DVLOG(1) << "Net error: " << net_error;
return (net_error == net::ERR_BLOCKED_BY_ADMINISTRATOR)
? ExploreSitesRequestStatus::kShouldSuspendBlockedByAdministrator
- : ExploreSitesRequestStatus::kShouldRetry;
+ : ExploreSitesRequestStatus::kFailure;
} else if (response_code < 200 || response_code > 299) {
DVLOG(1) << "HTTP status: " << response_code;
switch (response_code) {
case net::HTTP_BAD_REQUEST:
return ExploreSitesRequestStatus::kShouldSuspendBadRequest;
default:
- return ExploreSitesRequestStatus::kShouldRetry;
+ return ExploreSitesRequestStatus::kFailure;
}
}
return ExploreSitesRequestStatus::kSuccess;
}
+void ExploreSitesFetcher::RetryWithBackoff() {
+ backoff_entry_.InformOfRequest(false);
+
+ if (backoff_entry_.failure_count() >= max_failure_count_) {
+ std::move(callback_).Run(ExploreSitesRequestStatus::kFailure,
+ std::unique_ptr<std::string>());
+ return;
+ }
+
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(&ExploreSitesFetcher::Start, weak_factory_.GetWeakPtr()),
+ backoff_entry_.GetTimeUntilRelease());
+}
+
} // namespace explore_sites
diff --git a/chrome/browser/android/explore_sites/explore_sites_fetcher.h b/chrome/browser/android/explore_sites/explore_sites_fetcher.h
index 0f319d20..8b331dd 100644
--- a/chrome/browser/android/explore_sites/explore_sites_fetcher.h
+++ b/chrome/browser/android/explore_sites/explore_sites_fetcher.h
@@ -12,6 +12,7 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/android/explore_sites/explore_sites_types.h"
+#include "net/base/backoff_entry.h"
namespace network {
class SimpleURLLoader;
@@ -29,38 +30,62 @@
base::OnceCallback<void(ExploreSitesRequestStatus status,
const std::unique_ptr<std::string> data)>;
+ static const net::BackoffEntry::Policy kImmediateFetchBackoffPolicy;
+ static const int kMaxFailureCountForImmediateFetch;
+ static const net::BackoffEntry::Policy kBackgroundFetchBackoffPolicy;
+ static const int kMaxFailureCountForBackgroundFetch;
+
// Creates a fetcher for the GetCatalog RPC.
static std::unique_ptr<ExploreSitesFetcher> CreateForGetCatalog(
- Callback callback,
- const std::string catalog_version,
- const std::string accept_languages,
- scoped_refptr<network::SharedURLLoaderFactory> loader_factory);
+ bool is_immediate_fetch,
+ const std::string& catalog_version,
+ const std::string& accept_languages,
+ scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
+ Callback callback);
// Creates a fetcher for the GetCategories RPC.
static std::unique_ptr<ExploreSitesFetcher> CreateForGetCategories(
- Callback callback,
- const std::string catalog_version,
- const std::string accept_languages,
- scoped_refptr<network::SharedURLLoaderFactory> loader_factory);
+ bool is_immediate_fetch,
+ const std::string& catalog_version,
+ const std::string& accept_languages,
+ scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
+ Callback callback);
~ExploreSitesFetcher();
+ void Start();
+
+ void disable_retry_for_testing() { disable_retry_for_testing_ = true; }
+
private:
explicit ExploreSitesFetcher(
- Callback callback,
+ bool is_immediate_fetch,
const GURL& url,
- const std::string catalog_version,
- const std::string accept_languages,
- scoped_refptr<network ::SharedURLLoaderFactory> loader_factory);
+ const std::string& catalog_version,
+ const std::string& accept_languages,
+ scoped_refptr<network ::SharedURLLoaderFactory> loader_factory,
+ Callback callback);
// Invoked from SimpleURLLoader after download is complete.
void OnSimpleLoaderComplete(std::unique_ptr<std::string> response_body);
ExploreSitesRequestStatus HandleResponseCode();
+ void RetryWithBackoff();
+
+ std::string accept_languages_;
+ std::string client_version_;
+ GURL request_url_;
+
+ net::BackoffEntry backoff_entry_;
+ int max_failure_count_;
+ bool disable_retry_for_testing_ = false;
+
Callback callback_;
+
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
std::unique_ptr<network::SimpleURLLoader> url_loader_;
+
base::WeakPtrFactory<ExploreSitesFetcher> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(ExploreSitesFetcher);
diff --git a/chrome/browser/android/explore_sites/explore_sites_fetcher_unittest.cc b/chrome/browser/android/explore_sites/explore_sites_fetcher_unittest.cc
index f110f12..d720b85 100644
--- a/chrome/browser/android/explore_sites/explore_sites_fetcher_unittest.cc
+++ b/chrome/browser/android/explore_sites/explore_sites_fetcher_unittest.cc
@@ -36,6 +36,7 @@
const char kAcceptLanguages[] = "en-US,en;q=0.5";
const char kCountryOverride[] = "country_override";
const char kExperimentData[] = "FooBar";
+const char kTestData[] = "Any data.";
} // namespace
namespace explore_sites {
@@ -52,6 +53,16 @@
net::HttpStatusCode http_error);
ExploreSitesRequestStatus RunFetcherWithData(const std::string& response_data,
std::string* data_received);
+ ExploreSitesRequestStatus RunFetcherWithBackoffs(
+ bool is_immediate_fetch,
+ size_t num_of_backoffs,
+ std::vector<base::TimeDelta> backoff_delays,
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks,
+ std::string* data_received);
+
+ void RespondWithData(const std::string& data);
+ void RespondWithNetError(int net_error);
+ void RespondWithHttpError(net::HttpStatusCode http_error);
void SetUpExperimentOption(std::string option, std::string data) {
const std::string kTrialName = "trial_name";
@@ -77,12 +88,11 @@
ExploreSitesFetcher::Callback StoreResult();
network::TestURLLoaderFactory::PendingRequest* GetPendingRequest(
size_t index);
+ std::unique_ptr<ExploreSitesFetcher> CreateFetcher(bool disable_retry,
+ bool is_immediate_fetch);
ExploreSitesRequestStatus RunFetcher(
base::OnceCallback<void(void)> respond_callback,
std::string* data_received);
- void RespondWithData(const std::string& data);
- void RespondWithNetError(int net_error);
- void RespondWithHttpError(net::HttpStatusCode http_error);
network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory>
@@ -192,9 +202,7 @@
base::OnceCallback<void(void)> respond_callback,
std::string* data_received) {
std::unique_ptr<ExploreSitesFetcher> fetcher =
- ExploreSitesFetcher::CreateForGetCatalog(StoreResult(), "123",
- kAcceptLanguages,
- test_shared_url_loader_factory_);
+ CreateFetcher(true /* disable_retry*/, true /*is_immediate_fetch*/);
std::move(respond_callback).Run();
task_runner_->RunUntilIdle();
@@ -204,19 +212,60 @@
return last_status_;
}
+ExploreSitesRequestStatus ExploreSitesFetcherTest::RunFetcherWithBackoffs(
+ bool is_immediate_fetch,
+ size_t num_of_backoffs,
+ std::vector<base::TimeDelta> backoff_delays,
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks,
+ std::string* data_received) {
+ DCHECK_EQ(num_of_backoffs, backoff_delays.size());
+ DCHECK(num_of_backoffs <= respond_callbacks.size() &&
+ respond_callbacks.size() <= num_of_backoffs + 1);
+
+ std::unique_ptr<ExploreSitesFetcher> fetcher =
+ CreateFetcher(false /* disable_retry*/, is_immediate_fetch);
+
+ std::move(respond_callbacks[0]).Run();
+ task_runner_->RunUntilIdle();
+
+ for (size_t i = 0; i < num_of_backoffs; ++i) {
+ task_runner_->FastForwardBy(backoff_delays[i]);
+ if (i + 1 <= respond_callbacks.size() - 1)
+ std::move(respond_callbacks[i + 1]).Run();
+ task_runner_->RunUntilIdle();
+ }
+
+ if (last_data_)
+ *data_received = *last_data_;
+ return last_status_;
+}
+
+std::unique_ptr<ExploreSitesFetcher> ExploreSitesFetcherTest::CreateFetcher(
+ bool disable_retry,
+ bool is_immediate_fetch) {
+ std::unique_ptr<ExploreSitesFetcher> fetcher =
+ ExploreSitesFetcher::CreateForGetCatalog(
+ is_immediate_fetch, "123", kAcceptLanguages,
+ test_shared_url_loader_factory_, StoreResult());
+ if (disable_retry)
+ fetcher->disable_retry_for_testing();
+ fetcher->Start();
+ return fetcher;
+}
+
TEST_F(ExploreSitesFetcherTest, NetErrors) {
EXPECT_EQ(ExploreSitesRequestStatus::kShouldSuspendBlockedByAdministrator,
RunFetcherWithNetError(net::ERR_BLOCKED_BY_ADMINISTRATOR));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithNetError(net::ERR_INTERNET_DISCONNECTED));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithNetError(net::ERR_NETWORK_CHANGED));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithNetError(net::ERR_CONNECTION_RESET));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithNetError(net::ERR_CONNECTION_CLOSED));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithNetError(net::ERR_CONNECTION_REFUSED));
}
@@ -224,37 +273,35 @@
EXPECT_EQ(ExploreSitesRequestStatus::kShouldSuspendBadRequest,
RunFetcherWithHttpError(net::HTTP_BAD_REQUEST));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_NOT_IMPLEMENTED));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_UNAUTHORIZED));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_NOT_FOUND));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_CONFLICT));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_INTERNAL_SERVER_ERROR));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_BAD_GATEWAY));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_SERVICE_UNAVAILABLE));
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
RunFetcherWithHttpError(net::HTTP_GATEWAY_TIMEOUT));
}
TEST_F(ExploreSitesFetcherTest, EmptyResponse) {
std::string data;
- EXPECT_EQ(ExploreSitesRequestStatus::kShouldRetry,
- RunFetcherWithData("", &data));
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure, RunFetcherWithData("", &data));
EXPECT_TRUE(data.empty());
}
TEST_F(ExploreSitesFetcherTest, Success) {
std::string data;
EXPECT_EQ(ExploreSitesRequestStatus::kSuccess,
- RunFetcherWithData("Any data.", &data));
- EXPECT_FALSE(data.empty());
- EXPECT_EQ(data, "Any data.");
+ RunFetcherWithData(kTestData, &data));
+ EXPECT_EQ(kTestData, data);
EXPECT_EQ(last_resource_request.url.spec(),
"https://exploresites-pa.googleapis.com/v1/"
@@ -265,9 +312,8 @@
SetUpExperimentOption(kCountryOverride, "KZ");
std::string data;
EXPECT_EQ(ExploreSitesRequestStatus::kSuccess,
- RunFetcherWithData("Any data.", &data));
- EXPECT_FALSE(data.empty());
- EXPECT_EQ(data, "Any data.");
+ RunFetcherWithData(kTestData, &data));
+ EXPECT_EQ(kTestData, data);
EXPECT_EQ(last_resource_request.url.spec(),
"https://exploresites-pa.googleapis.com/v1/"
@@ -277,7 +323,7 @@
TEST_F(ExploreSitesFetcherTest, TestHeaders) {
std::string data;
EXPECT_EQ(ExploreSitesRequestStatus::kSuccess,
- RunFetcherWithData("Any data.", &data));
+ RunFetcherWithData(kTestData, &data));
net::HttpRequestHeaders headers = last_resource_request.headers;
std::string content_type;
@@ -311,7 +357,7 @@
std::string data;
EXPECT_EQ(ExploreSitesRequestStatus::kSuccess,
- RunFetcherWithData("Any data.", &data));
+ RunFetcherWithData(kTestData, &data));
net::HttpRequestHeaders headers = last_resource_request.headers;
std::string header_text;
@@ -321,4 +367,136 @@
EXPECT_EQ(std::string(kExperimentData), header_text);
}
+TEST_F(ExploreSitesFetcherTest, OneBackoffForImmediateFetch) {
+ std::string data;
+ int initial_delay_ms =
+ ExploreSitesFetcher::kImmediateFetchBackoffPolicy.initial_delay_ms;
+ std::vector<base::TimeDelta> backoff_delays = {
+ base::TimeDelta::FromMilliseconds(initial_delay_ms)};
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks;
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithNetError,
+ base::Unretained(this), net::ERR_INTERNET_DISCONNECTED));
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithData,
+ base::Unretained(this), kTestData));
+ EXPECT_EQ(
+ ExploreSitesRequestStatus::kSuccess,
+ RunFetcherWithBackoffs(true /*is_immediate_fetch*/, 1u, backoff_delays,
+ std::move(respond_callbacks), &data));
+ EXPECT_EQ(kTestData, data);
+}
+
+TEST_F(ExploreSitesFetcherTest, OneBackoffForBackgroundFetch) {
+ std::string data;
+ int initial_delay_ms =
+ ExploreSitesFetcher::kBackgroundFetchBackoffPolicy.initial_delay_ms;
+ std::vector<base::TimeDelta> backoff_delays = {
+ base::TimeDelta::FromMilliseconds(initial_delay_ms)};
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks;
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithNetError,
+ base::Unretained(this), net::ERR_INTERNET_DISCONNECTED));
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithData,
+ base::Unretained(this), kTestData));
+ EXPECT_EQ(
+ ExploreSitesRequestStatus::kSuccess,
+ RunFetcherWithBackoffs(false /*is_immediate_fetch*/, 1u, backoff_delays,
+ std::move(respond_callbacks), &data));
+ EXPECT_EQ(kTestData, data);
+}
+
+TEST_F(ExploreSitesFetcherTest, TwoBackoffsForImmediateFetch) {
+ std::string data;
+ int initial_delay_ms =
+ ExploreSitesFetcher::kImmediateFetchBackoffPolicy.initial_delay_ms;
+ std::vector<base::TimeDelta> backoff_delays = {
+ base::TimeDelta::FromMilliseconds(initial_delay_ms),
+ base::TimeDelta::FromMilliseconds(initial_delay_ms * 2)};
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks;
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithNetError,
+ base::Unretained(this), net::ERR_INTERNET_DISCONNECTED));
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithHttpError,
+ base::Unretained(this), net::HTTP_INTERNAL_SERVER_ERROR));
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithData,
+ base::Unretained(this), kTestData));
+ EXPECT_EQ(
+ ExploreSitesRequestStatus::kSuccess,
+ RunFetcherWithBackoffs(true /*is_immediate_fetch*/, 2u, backoff_delays,
+ std::move(respond_callbacks), &data));
+ EXPECT_EQ(kTestData, data);
+}
+
+TEST_F(ExploreSitesFetcherTest, TwoBackoffsForBackgroundFetch) {
+ std::string data;
+ int initial_delay_ms =
+ ExploreSitesFetcher::kBackgroundFetchBackoffPolicy.initial_delay_ms;
+ std::vector<base::TimeDelta> backoff_delays = {
+ base::TimeDelta::FromMilliseconds(initial_delay_ms),
+ base::TimeDelta::FromMilliseconds(initial_delay_ms * 2)};
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks;
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithNetError,
+ base::Unretained(this), net::ERR_INTERNET_DISCONNECTED));
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithHttpError,
+ base::Unretained(this), net::HTTP_INTERNAL_SERVER_ERROR));
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithData,
+ base::Unretained(this), kTestData));
+ EXPECT_EQ(
+ ExploreSitesRequestStatus::kSuccess,
+ RunFetcherWithBackoffs(false /*is_immediate_fetch*/, 2u, backoff_delays,
+ std::move(respond_callbacks), &data));
+ EXPECT_EQ(kTestData, data);
+}
+
+TEST_F(ExploreSitesFetcherTest, ExceedMaxBackoffsForImmediateFetch) {
+ std::string data;
+ int delay_ms =
+ ExploreSitesFetcher::kImmediateFetchBackoffPolicy.initial_delay_ms;
+ std::vector<base::TimeDelta> backoff_delays;
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks;
+ for (int i = 0; i < ExploreSitesFetcher::kMaxFailureCountForImmediateFetch;
+ ++i) {
+ backoff_delays.push_back(base::TimeDelta::FromMilliseconds(delay_ms));
+ delay_ms *= 2;
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithNetError,
+ base::Unretained(this), net::ERR_INTERNET_DISCONNECTED));
+ }
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
+ RunFetcherWithBackoffs(
+ true /*is_immediate_fetch*/,
+ ExploreSitesFetcher::kMaxFailureCountForImmediateFetch,
+ backoff_delays, std::move(respond_callbacks), &data));
+ EXPECT_TRUE(data.empty());
+}
+
+TEST_F(ExploreSitesFetcherTest, ExceedMaxBackoffsForBackgroundFetch) {
+ std::string data;
+ int delay_ms =
+ ExploreSitesFetcher::kBackgroundFetchBackoffPolicy.initial_delay_ms;
+ std::vector<base::TimeDelta> backoff_delays;
+ std::vector<base::OnceCallback<void(void)>> respond_callbacks;
+ for (int i = 0; i < ExploreSitesFetcher::kMaxFailureCountForBackgroundFetch;
+ ++i) {
+ backoff_delays.push_back(base::TimeDelta::FromMilliseconds(delay_ms));
+ delay_ms *= 2;
+ respond_callbacks.push_back(
+ base::BindOnce(&ExploreSitesFetcherTest::RespondWithNetError,
+ base::Unretained(this), net::ERR_INTERNET_DISCONNECTED));
+ }
+ EXPECT_EQ(ExploreSitesRequestStatus::kFailure,
+ RunFetcherWithBackoffs(
+ false /*is_immediate_fetch*/,
+ ExploreSitesFetcher::kMaxFailureCountForBackgroundFetch,
+ backoff_delays, std::move(respond_callbacks), &data));
+ EXPECT_TRUE(data.empty());
+}
+
} // namespace explore_sites
diff --git a/chrome/browser/android/explore_sites/explore_sites_service.h b/chrome/browser/android/explore_sites/explore_sites_service.h
index 9bdd70a..c3558a3 100644
--- a/chrome/browser/android/explore_sites/explore_sites_service.h
+++ b/chrome/browser/android/explore_sites/explore_sites_service.h
@@ -30,9 +30,12 @@
virtual void GetSiteImage(int site_id, BitmapCallback callback) = 0;
// Fetch the latest catalog from the network and stores it locally. Returns
- // true in the callback for success. If the accept_languages string is empty,
- // no "Accept-Language" header is created for the network request.
- virtual void UpdateCatalogFromNetwork(std::string accept_languages,
+ // true in the callback for success. |is_immediate_fetch| denotes if the
+ // fetch should be done immediately or on background. If the accept_languages
+ // string is empty, no "Accept-Language" header is created for the network
+ // request.
+ virtual void UpdateCatalogFromNetwork(bool is_immediate_fetch,
+ const std::string& accept_languages,
BooleanCallback callback) = 0;
};
diff --git a/chrome/browser/android/explore_sites/explore_sites_service_impl.cc b/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
index cd46955..a47e0b0 100644
--- a/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
+++ b/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
@@ -79,7 +79,8 @@
}
void ExploreSitesServiceImpl::UpdateCatalogFromNetwork(
- std::string accept_languages,
+ bool is_immediate_fetch,
+ const std::string& accept_languages,
BooleanCallback callback) {
if (!IsExploreSitesEnabled())
return;
@@ -92,9 +93,11 @@
// Create a fetcher and start fetching the protobuf (async).
explore_sites_fetcher_ = ExploreSitesFetcher::CreateForGetCatalog(
+ is_immediate_fetch, catalog_version, accept_languages,
+ url_loader_factory_,
base::BindOnce(&ExploreSitesServiceImpl::OnCatalogFetched,
- weak_ptr_factory_.GetWeakPtr(), std::move(callback)),
- catalog_version, accept_languages, url_loader_factory_);
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+ explore_sites_fetcher_->Start();
}
void ExploreSitesServiceImpl::OnCatalogFetched(
diff --git a/chrome/browser/android/explore_sites/explore_sites_service_impl.h b/chrome/browser/android/explore_sites/explore_sites_service_impl.h
index 3b51b94..46eeaa4 100644
--- a/chrome/browser/android/explore_sites/explore_sites_service_impl.h
+++ b/chrome/browser/android/explore_sites/explore_sites_service_impl.h
@@ -36,7 +36,8 @@
void GetCatalog(CatalogCallback callback) override;
void GetCategoryImage(int category_id, BitmapCallback callback) override;
void GetSiteImage(int site_id, BitmapCallback callback) override;
- void UpdateCatalogFromNetwork(std::string accept_languages,
+ void UpdateCatalogFromNetwork(bool is_immediate_fetch,
+ const std::string& accept_languages,
BooleanCallback callback) override;
private:
diff --git a/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc b/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
index d035698..aa0f171 100644
--- a/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
+++ b/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
@@ -158,7 +158,7 @@
scoped_feature_list.InitAndEnableFeature(chrome::android::kExploreSites);
service()->UpdateCatalogFromNetwork(
- kAcceptLanguages,
+ true /*is_immediate_fetch*/, kAcceptLanguages,
base::BindOnce(&ExploreSitesServiceImplTest::UpdateCatalogDoneCallback,
base::Unretained(this)));
diff --git a/chrome/browser/android/explore_sites/explore_sites_types.h b/chrome/browser/android/explore_sites/explore_sites_types.h
index 1956db0..185906c 100644
--- a/chrome/browser/android/explore_sites/explore_sites_types.h
+++ b/chrome/browser/android/explore_sites/explore_sites_types.h
@@ -69,8 +69,8 @@
enum class ExploreSitesRequestStatus {
// Request completed successfully.
kSuccess = 0,
- // Request failed due to to local network problem, caller should retry.
- kShouldRetry = 1,
+ // Request failed even after all the retries.
+ kFailure = 1,
// Request failed with error indicating that the request can not be serviced
// by the server.
kShouldSuspendBadRequest = 2,
