Commit 20b13dfd authored by Justin DeWitt's avatar Justin DeWitt Committed by Commit Bot

[EoS] Add country code to the request for the catalog.

The country code comes from the first place in the following list with a
non-empty country code:
* Field Trial parameter "country_override", configurable by Finch server
* Finch permanent country, which is updated when the Finch configs are
  updated (by default once per Chrome version). Permanent country may
  not be available in the first run of Chrome.
* Finch latest country, which is available on every run of Chrome.

Bug: 867488
Change-Id: I0e407a50a4020882152dda85935ecf843a000cb0
Reviewed-on: https://chromium-review.googlesource.com/c/1256110
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: default avatarPeter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595978}
parent ef7c3e67
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/browser/android/explore_sites/explore_sites_fetcher.h" #include "chrome/browser/android/explore_sites/explore_sites_fetcher.h"
#include <string>
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
...@@ -17,8 +18,10 @@ ...@@ -17,8 +18,10 @@
#include "chrome/browser/android/explore_sites/catalog.pb.h" #include "chrome/browser/android/explore_sites/catalog.pb.h"
#include "chrome/browser/android/explore_sites/explore_sites_types.h" #include "chrome/browser/android/explore_sites/explore_sites_types.h"
#include "chrome/browser/android/explore_sites/url_util.h" #include "chrome/browser/android/explore_sites/url_util.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/net/chrome_accept_language_settings.h" #include "chrome/browser/net/chrome_accept_language_settings.h"
#include "chrome/common/channel_info.h" #include "chrome/common/channel_info.h"
#include "components/variations/service/variations_service.h"
#include "components/version_info/version_info.h" #include "components/version_info/version_info.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
...@@ -39,6 +42,27 @@ namespace explore_sites { ...@@ -39,6 +42,27 @@ namespace explore_sites {
namespace { namespace {
std::string GetCountry() {
std::string manually_set_variation_country =
base::GetFieldTrialParamValueByFeature(chrome::android::kExploreSites,
"country_override");
if (!manually_set_variation_country.empty())
return manually_set_variation_country;
variations::VariationsService* variations_service =
g_browser_process->variations_service();
if (variations_service) {
std::string country = variations_service->GetStoredPermanentCountry();
if (!country.empty())
return country;
country = variations_service->GetLatestCountry();
if (!country.empty())
return country;
}
return "DEFAULT";
}
// Content type needed in order to communicate with the server in binary // Content type needed in order to communicate with the server in binary
// proto format. // proto format.
const char kRequestContentType[] = "application/x-protobuf"; const char kRequestContentType[] = "application/x-protobuf";
...@@ -68,33 +92,30 @@ constexpr net::NetworkTrafficAnnotationTag traffic_annotation = ...@@ -68,33 +92,30 @@ constexpr net::NetworkTrafficAnnotationTag traffic_annotation =
std::unique_ptr<ExploreSitesFetcher> ExploreSitesFetcher::CreateForGetCatalog( std::unique_ptr<ExploreSitesFetcher> ExploreSitesFetcher::CreateForGetCatalog(
Callback callback, Callback callback,
const std::string catalog_version, const std::string catalog_version,
const std::string country_code,
const std::string accept_languages, const std::string accept_languages,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory) { scoped_refptr<network::SharedURLLoaderFactory> loader_factory) {
GURL url = GetCatalogURL(); GURL url = GetCatalogURL();
return base::WrapUnique( return base::WrapUnique(
new ExploreSitesFetcher(std::move(callback), url, catalog_version, new ExploreSitesFetcher(std::move(callback), url, catalog_version,
country_code, accept_languages, loader_factory)); accept_languages, loader_factory));
} }
std::unique_ptr<ExploreSitesFetcher> std::unique_ptr<ExploreSitesFetcher>
ExploreSitesFetcher::CreateForGetCategories( ExploreSitesFetcher::CreateForGetCategories(
Callback callback, Callback callback,
const std::string catalog_version, const std::string catalog_version,
const std::string country_code,
const std::string accept_languages, const std::string accept_languages,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory) { scoped_refptr<network::SharedURLLoaderFactory> loader_factory) {
GURL url = GetCategoriesURL(); GURL url = GetCategoriesURL();
return base::WrapUnique( return base::WrapUnique(
new ExploreSitesFetcher(std::move(callback), url, catalog_version, new ExploreSitesFetcher(std::move(callback), url, catalog_version,
country_code, accept_languages, loader_factory)); accept_languages, loader_factory));
} }
ExploreSitesFetcher::ExploreSitesFetcher( ExploreSitesFetcher::ExploreSitesFetcher(
Callback callback, Callback callback,
const GURL& url, const GURL& url,
const std::string catalog_version, const std::string catalog_version,
const std::string country_code,
const std::string accept_languages, const std::string accept_languages,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory) scoped_refptr<network::SharedURLLoaderFactory> loader_factory)
: callback_(std::move(callback)), : callback_(std::move(callback)),
...@@ -109,9 +130,10 @@ ExploreSitesFetcher::ExploreSitesFetcher( ...@@ -109,9 +130,10 @@ ExploreSitesFetcher::ExploreSitesFetcher(
version.components()[3], // Patch version.components()[3], // Patch
channel_name.c_str()); channel_name.c_str());
GURL final_url = GURL final_url =
net::AppendOrReplaceQueryParameter(url, "country_code", country_code); net::AppendOrReplaceQueryParameter(url, "country_code", GetCountry());
final_url = net::AppendOrReplaceQueryParameter(final_url, "version_token", final_url = net::AppendOrReplaceQueryParameter(final_url, "version_token",
catalog_version); catalog_version);
DVLOG(1) << "Final URL: " << final_url.spec();
auto resource_request = std::make_unique<network::ResourceRequest>(); auto resource_request = std::make_unique<network::ResourceRequest>();
resource_request->url = final_url; resource_request->url = final_url;
......
...@@ -33,7 +33,6 @@ class ExploreSitesFetcher { ...@@ -33,7 +33,6 @@ class ExploreSitesFetcher {
static std::unique_ptr<ExploreSitesFetcher> CreateForGetCatalog( static std::unique_ptr<ExploreSitesFetcher> CreateForGetCatalog(
Callback callback, Callback callback,
const std::string catalog_version, const std::string catalog_version,
const std::string country_code,
const std::string accept_languages, const std::string accept_languages,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory); scoped_refptr<network::SharedURLLoaderFactory> loader_factory);
...@@ -41,7 +40,6 @@ class ExploreSitesFetcher { ...@@ -41,7 +40,6 @@ class ExploreSitesFetcher {
static std::unique_ptr<ExploreSitesFetcher> CreateForGetCategories( static std::unique_ptr<ExploreSitesFetcher> CreateForGetCategories(
Callback callback, Callback callback,
const std::string catalog_version, const std::string catalog_version,
const std::string country_code,
const std::string accept_languages, const std::string accept_languages,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory); scoped_refptr<network::SharedURLLoaderFactory> loader_factory);
...@@ -52,7 +50,6 @@ class ExploreSitesFetcher { ...@@ -52,7 +50,6 @@ class ExploreSitesFetcher {
Callback callback, Callback callback,
const GURL& url, const GURL& url,
const std::string catalog_version, const std::string catalog_version,
const std::string country_code,
const std::string accept_languages, const std::string accept_languages,
scoped_refptr<network ::SharedURLLoaderFactory> loader_factory); scoped_refptr<network ::SharedURLLoaderFactory> loader_factory);
......
...@@ -4,11 +4,19 @@ ...@@ -4,11 +4,19 @@
#include "chrome/browser/android/explore_sites/explore_sites_fetcher.h" #include "chrome/browser/android/explore_sites/explore_sites_fetcher.h"
#include <map>
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/metrics/field_trial.h"
#include "base/metrics/field_trial_params.h"
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "base/test/mock_callback.h" #include "base/test/mock_callback.h"
#include "base/test/mock_entropy_provider.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_mock_time_task_runner.h" #include "base/test/test_mock_time_task_runner.h"
#include "chrome/browser/android/chrome_feature_list.h"
#include "chrome/browser/android/explore_sites/catalog.pb.h" #include "chrome/browser/android/explore_sites/catalog.pb.h"
#include "chrome/browser/android/explore_sites/explore_sites_feature.h"
#include "chrome/browser/android/explore_sites/explore_sites_types.h" #include "chrome/browser/android/explore_sites/explore_sites_types.h"
#include "net/http/http_request_headers.h" #include "net/http/http_request_headers.h"
#include "net/http/http_status_code.h" #include "net/http/http_status_code.h"
...@@ -43,6 +51,25 @@ class ExploreSitesFetcherTest : public testing::Test { ...@@ -43,6 +51,25 @@ class ExploreSitesFetcherTest : public testing::Test {
ExploreSitesRequestStatus RunFetcherWithData(const std::string& response_data, ExploreSitesRequestStatus RunFetcherWithData(const std::string& response_data,
std::string* data_received); std::string* data_received);
void SetUpExperimentOption(std::string country_code) {
const std::string kTrialName = "trial_name";
const std::string kGroupName = "group_name";
scoped_refptr<base::FieldTrial> trial =
base::FieldTrialList::CreateFieldTrial(kTrialName, kGroupName);
std::map<std::string, std::string> params = {
{"country_override", country_code}};
base::AssociateFieldTrialParams(kTrialName, kGroupName, params);
std::unique_ptr<base::FeatureList> feature_list =
std::make_unique<base::FeatureList>();
feature_list->RegisterFieldTrialOverride(
chrome::android::kExploreSites.name,
base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial.get());
scoped_feature_list_.InitWithFeatureList(std::move(feature_list));
}
network::ResourceRequest last_resource_request; network::ResourceRequest last_resource_request;
private: private:
...@@ -64,6 +91,8 @@ class ExploreSitesFetcherTest : public testing::Test { ...@@ -64,6 +91,8 @@ class ExploreSitesFetcherTest : public testing::Test {
std::unique_ptr<std::string> last_data_; std::unique_ptr<std::string> last_data_;
base::MessageLoopForIO message_loop_; base::MessageLoopForIO message_loop_;
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
std::unique_ptr<base::FieldTrialList> field_trial_list_;
base::test::ScopedFeatureList scoped_feature_list_;
}; };
ExploreSitesFetcher::Callback ExploreSitesFetcherTest::StoreResult() { ExploreSitesFetcher::Callback ExploreSitesFetcherTest::StoreResult() {
...@@ -88,6 +117,8 @@ void ExploreSitesFetcherTest::SetUp() { ...@@ -88,6 +117,8 @@ void ExploreSitesFetcherTest::SetUp() {
EXPECT_TRUE(request.url.is_valid() && !request.url.is_empty()); EXPECT_TRUE(request.url.is_valid() && !request.url.is_empty());
last_resource_request = request; last_resource_request = request;
})); }));
field_trial_list_ = std::make_unique<base::FieldTrialList>(
std::make_unique<base::MockEntropyProvider>());
} }
ExploreSitesRequestStatus ExploreSitesFetcherTest::RunFetcherWithNetError( ExploreSitesRequestStatus ExploreSitesFetcherTest::RunFetcherWithNetError(
...@@ -160,7 +191,7 @@ ExploreSitesRequestStatus ExploreSitesFetcherTest::RunFetcher( ...@@ -160,7 +191,7 @@ ExploreSitesRequestStatus ExploreSitesFetcherTest::RunFetcher(
base::OnceCallback<void(void)> respond_callback, base::OnceCallback<void(void)> respond_callback,
std::string* data_received) { std::string* data_received) {
std::unique_ptr<ExploreSitesFetcher> fetcher = std::unique_ptr<ExploreSitesFetcher> fetcher =
ExploreSitesFetcher::CreateForGetCatalog(StoreResult(), "123", "KE", ExploreSitesFetcher::CreateForGetCatalog(StoreResult(), "123",
kAcceptLanguages, kAcceptLanguages,
test_shared_url_loader_factory_); test_shared_url_loader_factory_);
...@@ -226,7 +257,20 @@ TEST_F(ExploreSitesFetcherTest, Success) { ...@@ -226,7 +257,20 @@ TEST_F(ExploreSitesFetcherTest, Success) {
EXPECT_EQ(last_resource_request.url.spec(), EXPECT_EQ(last_resource_request.url.spec(),
"https://exploresites-pa.googleapis.com/v1/" "https://exploresites-pa.googleapis.com/v1/"
"getcatalog?country_code=KE&version_token=123"); "getcatalog?country_code=DEFAULT&version_token=123");
}
TEST_F(ExploreSitesFetcherTest, DefaultCountry) {
SetUpExperimentOption("KZ");
std::string data;
EXPECT_EQ(ExploreSitesRequestStatus::kSuccess,
RunFetcherWithData("Any data.", &data));
EXPECT_FALSE(data.empty());
EXPECT_EQ(data, "Any data.");
EXPECT_EQ(last_resource_request.url.spec(),
"https://exploresites-pa.googleapis.com/v1/"
"getcatalog?country_code=KZ&version_token=123");
} }
TEST_F(ExploreSitesFetcherTest, TestHeaders) { TEST_F(ExploreSitesFetcherTest, TestHeaders) {
......
...@@ -79,14 +79,12 @@ void ExploreSitesServiceImpl::UpdateCatalogFromNetwork( ...@@ -79,14 +79,12 @@ void ExploreSitesServiceImpl::UpdateCatalogFromNetwork(
// TODO(petewil): Eventually get the catalog version from DB. // TODO(petewil): Eventually get the catalog version from DB.
std::string catalog_version = ""; std::string catalog_version = "";
// TODO(petewil): Eventually get the country code from somewhere.
std::string country_code = "KE";
// Create a fetcher and start fetching the protobuf (async). // Create a fetcher and start fetching the protobuf (async).
explore_sites_fetcher_ = ExploreSitesFetcher::CreateForGetCatalog( explore_sites_fetcher_ = ExploreSitesFetcher::CreateForGetCatalog(
base::BindOnce(&ExploreSitesServiceImpl::OnCatalogFetched, base::BindOnce(&ExploreSitesServiceImpl::OnCatalogFetched,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)), weak_ptr_factory_.GetWeakPtr(), std::move(callback)),
catalog_version, country_code, accept_languages, url_loader_factory_); catalog_version, accept_languages, url_loader_factory_);
} }
void ExploreSitesServiceImpl::OnCatalogFetched( void ExploreSitesServiceImpl::OnCatalogFetched(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment