Skip to content

GitLab

T
tangled
Commit 042810ea authored by Marc Treib's avatar Marc Treib Committed by Commit Bot
Browse files
Options

OneGoogleBar on local NTP: Use cookie auth instead of OAuth2 

This is achieved by talking to a different endpoint for fetching the
OGB sources, which accepts cookies. As a consequence, the cached data
is now cleared when the Gaia cookies change, rather than when the
SigninManager state changes.

Bug: 740462
Change-Id: Id230a118507e32e6f1a17c103c93c8eb5ccb0ba6
Reviewed-on: https://chromium-review.googlesource.com/595990
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarChris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492008}
parent 70fc047e
Hide whitespace changes
Inline Side-by-side
Showing with 149 additions and 364 deletions
chrome/browser/search/one_google_bar/one_google_bar_fetcher_impl.cc
View file @ 042810ea
......@@ -19,12 +19,9 @@
#include "chrome/common/chrome_content_client.h"
#include "chrome/common/chrome_features.h"
#include "components/google/core/browser/google_url_tracker.h"
#include "components/google/core/browser/google_util.h"
#include "components/safe_json/safe_json_parser.h"
#include "components/signin/core/browser/access_token_fetcher.h"
#include "components/signin/core/browser/signin_manager.h"
#include "components/variations/net/variations_http_headers.h"
#include "google_apis/gaia/oauth2_token_service.h"
#include "google_apis/google_api_keys.h"
#include "net/base/load_flags.h"
#include "net/http/http_status_code.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
......@@ -33,12 +30,7 @@
namespace {
const char kApiUrl[] = "https://onegoogle-pa.googleapis.com/v1/getbar";
const char kApiKeyFormat[] = "?key=%s";
const char kApiScope[] = "https://www.googleapis.com/auth/onegoogle.api";
const char kAuthorizationRequestHeaderFormat[] = "Bearer %s";
const char kApiPath[] = "/async/newtab_ogb";
const char kResponsePreamble[] = ")]}'";
......@@ -69,14 +61,15 @@ bool GetImpl(const base::DictionaryValue& dict,
bool GetHtml(const base::DictionaryValue& dict,
const std::string& name,
std::string* out) {
return GetImpl(dict, name, "privateDoNotAccessOrElseSafeHtmlWrappedValue",
out);
return GetImpl(dict, name,
"private_do_not_access_or_else_safe_html_wrapped_value", out);
}
bool GetScript(const base::DictionaryValue& dict,
const std::string& name,
std::string* out) {
return GetImpl(dict, name, "privateDoNotAccessOrElseSafeScriptWrappedValue",
return GetImpl(dict, name,
"private_do_not_access_or_else_safe_script_wrapped_value",
out);
}
......@@ -84,7 +77,8 @@ bool GetStyleSheet(const base::DictionaryValue& dict,
const std::string& name,
std::string* out) {
return GetImpl(dict, name,
"privateDoNotAccessOrElseSafeStyleSheetWrappedValue", out);
"private_do_not_access_or_else_safe_style_sheet_wrapped_value",
out);
}
} // namespace safe_html
......@@ -96,9 +90,15 @@ base::Optional<OneGoogleBarData> JsonToOGBData(const base::Value& value) {
return base::nullopt;
}
const base::DictionaryValue* update = nullptr;
if (!dict->GetDictionary("update", &update)) {
DLOG(WARNING) << "Parse error: no update";
return base::nullopt;
}
const base::DictionaryValue* one_google_bar = nullptr;
if (!dict->GetDictionary("oneGoogleBar", &one_google_bar)) {
DLOG(WARNING) << "Parse error: no oneGoogleBar";
if (!update->GetDictionary("ogb", &one_google_bar)) {
DLOG(WARNING) << "Parse error: no ogb";
return base::nullopt;
}
......@@ -110,16 +110,17 @@ base::Optional<OneGoogleBarData> JsonToOGBData(const base::Value& value) {
}
const base::DictionaryValue* page_hooks = nullptr;
if (!one_google_bar->GetDictionary("pageHooks", &page_hooks)) {
DLOG(WARNING) << "Parse error: no pageHooks";
if (!one_google_bar->GetDictionary("page_hooks", &page_hooks)) {
DLOG(WARNING) << "Parse error: no page_hooks";
return base::nullopt;
}
safe_html::GetScript(*page_hooks, "inHeadScript", &result.in_head_script);
safe_html::GetStyleSheet(*page_hooks, "inHeadStyle", &result.in_head_style);
safe_html::GetScript(*page_hooks, "afterBarScript", &result.after_bar_script);
safe_html::GetHtml(*page_hooks, "endOfBodyHtml", &result.end_of_body_html);
safe_html::GetScript(*page_hooks, "endOfBodyScript",
safe_html::GetScript(*page_hooks, "in_head_script", &result.in_head_script);
safe_html::GetStyleSheet(*page_hooks, "in_head_style", &result.in_head_style);
safe_html::GetScript(*page_hooks, "after_bar_script",
&result.after_bar_script);
safe_html::GetHtml(*page_hooks, "end_of_body_html", &result.end_of_body_html);
safe_html::GetScript(*page_hooks, "end_of_body_script",
&result.end_of_body_script);
return result;
......@@ -132,9 +133,7 @@ class OneGoogleBarFetcherImpl::AuthenticatedURLFetcher
public:
using FetchDoneCallback = base::OnceCallback<void(const net::URLFetcher*)>;
AuthenticatedURLFetcher(SigninManagerBase* signin_manager,
OAuth2TokenService* token_service,
net::URLRequestContextGetter* request_context,
AuthenticatedURLFetcher(net::URLRequestContextGetter* request_context,
const GURL& google_base_url,
FetchDoneCallback callback);
~AuthenticatedURLFetcher() override = default;
......@@ -142,117 +141,44 @@ class OneGoogleBarFetcherImpl::AuthenticatedURLFetcher
void Start();
private:
GURL GetApiUrl(bool use_oauth) const;
std::string GetRequestBody() const;
std::string GetExtraRequestHeaders(const GURL& url,
const std::string& access_token) const;
void GotAccessToken(const GoogleServiceAuthError& error,
const std::string& access_token);
GURL GetApiUrl() const;
std::string GetExtraRequestHeaders(const GURL& url) const;
// URLFetcherDelegate implementation.
void OnURLFetchComplete(const net::URLFetcher* source) override;
SigninManagerBase* const signin_manager_;
OAuth2TokenService* const token_service_;
net::URLRequestContextGetter* const request_context_;
const GURL google_base_url_;
FetchDoneCallback callback_;
std::unique_ptr<AccessTokenFetcher> token_fetcher_;
// The underlying URLFetcher which does the actual fetch.
std::unique_ptr<net::URLFetcher> url_fetcher_;
};
OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::AuthenticatedURLFetcher(
SigninManagerBase* signin_manager,
OAuth2TokenService* token_service,
net::URLRequestContextGetter* request_context,
const GURL& google_base_url,
FetchDoneCallback callback)
: signin_manager_(signin_manager),
token_service_(token_service),
request_context_(request_context),
: request_context_(request_context),
google_base_url_(google_base_url),
callback_(std::move(callback)) {}
void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::Start() {
if (!signin_manager_->IsAuthenticated()) {
GotAccessToken(GoogleServiceAuthError::AuthErrorNone(), std::string());
return;
}
OAuth2TokenService::ScopeSet scopes;
scopes.insert(kApiScope);
token_fetcher_ = base::MakeUnique<AccessTokenFetcher>(
"one_google", signin_manager_, token_service_, scopes,
base::BindOnce(&AuthenticatedURLFetcher::GotAccessToken,
base::Unretained(this)));
}
GURL OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetApiUrl(
bool use_oauth) const {
GURL OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetApiUrl() const {
std::string api_url = base::GetFieldTrialParamValueByFeature(
features::kOneGoogleBarOnLocalNtp, "one-google-api-url");
if (api_url.empty()) {
api_url = kApiUrl;
}
// Append the API key only for unauthenticated requests.
if (!use_oauth) {
api_url +=
base::StringPrintf(kApiKeyFormat, google_apis::GetAPIKey().c_str());
}
return GURL(api_url);
}
std::string OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetRequestBody()
const {
std::string override_options = base::GetFieldTrialParamValueByFeature(
features::kOneGoogleBarOnLocalNtp, "one-google-bar-options");
if (!override_options.empty()) {
return override_options;
if (!api_url.empty()) {
return GURL(api_url);
}
base::DictionaryValue dict;
dict.SetInteger("subproduct", 243);
dict.SetBoolean("enable_multilogin", true);
dict.SetString("user_agent", GetUserAgent());
dict.SetString("accept_language", g_browser_process->GetApplicationLocale());
dict.SetString("original_request_url", google_base_url_.spec());
auto material_options_dict = base::MakeUnique<base::DictionaryValue>();
material_options_dict->SetString("page_title", " ");
material_options_dict->SetBoolean("position_fixed", true);
material_options_dict->SetBoolean("disable_moving_userpanel_to_menu", true);
auto styling_options_dict = base::MakeUnique<base::DictionaryValue>();
auto background_color_dict = base::MakeUnique<base::DictionaryValue>();
auto alpha_dict = base::MakeUnique<base::DictionaryValue>();
alpha_dict->SetInteger("value", 0);
background_color_dict->Set("alpha", std::move(alpha_dict));
styling_options_dict->Set("background_color",
std::move(background_color_dict));
material_options_dict->Set("styling_options",
std::move(styling_options_dict));
dict.Set("material_options", std::move(material_options_dict));
std::string result;
base::JSONWriter::Write(dict, &result);
return result;
return google_base_url_.Resolve(kApiPath);
}
std::string
OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetExtraRequestHeaders(
const GURL& url,
const std::string& access_token) const {
const GURL& url) const {
net::HttpRequestHeaders headers;
headers.SetHeader("Content-Type", "application/json; charset=UTF-8");
if (!access_token.empty()) {
headers.SetHeader("Authorization",
base::StringPrintf(kAuthorizationRequestHeaderFormat,
access_token.c_str()));
}
// Note: It's OK to pass |is_signed_in| false if it's unknown, as it does
// not affect transmission of experiments coming from the variations server.
variations::AppendVariationHeaders(url,
......@@ -261,14 +187,8 @@ OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GetExtraRequestHeaders(
return headers.ToString();
}
void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GotAccessToken(
const GoogleServiceAuthError& error,
const std::string& access_token) {
// Delete the token fetcher after we leave this method.
std::unique_ptr<AccessTokenFetcher> deleter(std::move(token_fetcher_));
bool use_oauth = !access_token.empty();
GURL url = GetApiUrl(use_oauth);
void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::Start() {
GURL url = GetApiUrl();
net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("one_google_bar_service", R"(
semantics {
......@@ -281,7 +201,8 @@ void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GotAccessToken(
destination: GOOGLE_OWNED_SERVICE
}
policy {
cookies_allowed: NO
cookies_allowed: YES
cookie_store: "user"
setting:
"Users can control this feature via selecting a non-Google default "
"search engine in Chrome settings under 'Search Engine'."
......@@ -292,16 +213,12 @@ void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::GotAccessToken(
}
}
})");
url_fetcher_ = net::URLFetcher::Create(0, url, net::URLFetcher::POST, this,
url_fetcher_ = net::URLFetcher::Create(0, url, net::URLFetcher::GET, this,
traffic_annotation);
url_fetcher_->SetRequestContext(request_context_);
url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_AUTH_DATA |
net::LOAD_DO_NOT_SEND_COOKIES |
net::LOAD_DO_NOT_SAVE_COOKIES);
url_fetcher_->SetUploadData("application/json", GetRequestBody());
url_fetcher_->SetExtraRequestHeaders(
GetExtraRequestHeaders(url, access_token));
url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_AUTH_DATA);
url_fetcher_->SetExtraRequestHeaders(GetExtraRequestHeaders(url));
url_fetcher_->Start();
}
......@@ -312,13 +229,9 @@ void OneGoogleBarFetcherImpl::AuthenticatedURLFetcher::OnURLFetchComplete(
}
OneGoogleBarFetcherImpl::OneGoogleBarFetcherImpl(
SigninManagerBase* signin_manager,
OAuth2TokenService* token_service,
net::URLRequestContextGetter* request_context,
GoogleURLTracker* google_url_tracker)
: signin_manager_(signin_manager),
token_service_(token_service),
request_context_(request_context),
: request_context_(request_context),
google_url_tracker_(google_url_tracker),
weak_ptr_factory_(this) {}
......@@ -335,9 +248,12 @@ void OneGoogleBarFetcherImpl::IssueRequestIfNoneOngoing() {
return;
}
GURL google_base_url = google_util::CommandLineGoogleBaseURL();
if (!google_base_url.is_valid()) {
google_base_url = google_url_tracker_->google_url();
}
pending_request_ = base::MakeUnique<AuthenticatedURLFetcher>(
signin_manager_, token_service_, request_context_,
google_url_tracker_->google_url(),
request_context_, google_base_url,
base::BindOnce(&OneGoogleBarFetcherImpl::FetchDone,
base::Unretained(this)));
pending_request_->Start();
......
chrome/browser/search/one_google_bar/one_google_bar_fetcher_impl.h
View file @ 042810ea
......@@ -15,8 +15,6 @@
#include "chrome/browser/search/one_google_bar/one_google_bar_fetcher.h"
class GoogleURLTracker;
class OAuth2TokenService;
class SigninManagerBase;
namespace base {
class Value;
......@@ -29,11 +27,12 @@ class URLRequestContextGetter;
struct OneGoogleBarData;
// TODO(treib): This class uses cookies for authentication. After "Dice" account
// consistency launches, we should switch to using OAuth2 instead.
// See crbug.com/751534.
class OneGoogleBarFetcherImpl : public OneGoogleBarFetcher {
public:
OneGoogleBarFetcherImpl(SigninManagerBase* signin_manager,
OAuth2TokenService* token_service,
net::URLRequestContextGetter* request_context,
OneGoogleBarFetcherImpl(net::URLRequestContextGetter* request_context,
GoogleURLTracker* google_url_tracker);
~OneGoogleBarFetcherImpl() override;
......@@ -51,8 +50,6 @@ class OneGoogleBarFetcherImpl : public OneGoogleBarFetcher {
void Respond(Status status, const base::Optional<OneGoogleBarData>& data);
SigninManagerBase* signin_manager_;
OAuth2TokenService* token_service_;
net::URLRequestContextGetter* request_context_;
GoogleURLTracker* google_url_tracker_;
......
chrome/browser/search/one_google_bar/one_google_bar_fetcher_impl_unittest.cc
View file @ 042810ea
......@@ -15,12 +15,6 @@
#include "chrome/browser/search/one_google_bar/one_google_bar_data.h"
#include "components/google/core/browser/google_url_tracker.h"
#include "components/safe_json/testing_json_parser.h"
#include "components/signin/core/browser/account_tracker_service.h"
#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
#include "components/signin/core/browser/fake_signin_manager.h"
#include "components/signin/core/browser/test_signin_client.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "google_apis/gaia/fake_oauth2_token_service_delegate.h"
#include "net/http/http_request_headers.h"
#include "net/http/http_status_code.h"
#include "net/url_request/test_url_fetcher_factory.h"
......@@ -37,10 +31,10 @@ using testing::StartsWith;
namespace {
const char kMinimalValidResponse[] = R"json({"oneGoogleBar": {
"html": { "privateDoNotAccessOrElseSafeHtmlWrappedValue": "" },
"pageHooks": {}
}})json";
const char kMinimalValidResponse[] = R"json({"update": { "ogb": {
"html": { "private_do_not_access_or_else_safe_html_wrapped_value": "" },
"page_hooks": {}
}}})json";
// Required to instantiate a GoogleUrlTracker in UNIT_TEST_MODE.
class GoogleURLTrackerClientStub : public GoogleURLTrackerClient {
......@@ -61,39 +55,13 @@ class GoogleURLTrackerClientStub : public GoogleURLTrackerClient {
class OneGoogleBarFetcherImplTest : public testing::Test {
public:
OneGoogleBarFetcherImplTest()
: signin_client_(&pref_service_),
signin_manager_(&signin_client_, &account_tracker_),
task_runner_(new base::TestSimpleTaskRunner()),
: task_runner_(new base::TestSimpleTaskRunner()),
request_context_getter_(
new net::TestURLRequestContextGetter(task_runner_)),
token_service_(base::MakeUnique<FakeOAuth2TokenServiceDelegate>(
request_context_getter_.get())),
google_url_tracker_(base::MakeUnique<GoogleURLTrackerClientStub>(),
GoogleURLTracker::UNIT_TEST_MODE),
one_google_bar_fetcher_(&signin_manager_,
&token_service_,
request_context_getter_.get(),
&google_url_tracker_) {
SigninManagerBase::RegisterProfilePrefs(pref_service_.registry());
SigninManagerBase::RegisterPrefs(pref_service_.registry());
}
void SignIn() {
signin_manager_.SignIn("account");
token_service_.GetDelegate()->UpdateCredentials("account", "refresh_token");
}
void IssueAccessToken() {
token_service_.IssueAllTokensForAccount(
"account", "access_token",
base::Time::Now() + base::TimeDelta::FromHours(1));
}
void IssueAccessTokenError() {
token_service_.IssueErrorForAllPendingRequestsForAccount(
"account",
GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_UNAVAILABLE));
}
one_google_bar_fetcher_(request_context_getter_.get(),
&google_url_tracker_) {}
net::TestURLFetcher* GetRunningURLFetcher() {
// All created URLFetchers have ID 0 by default.
......@@ -137,40 +105,18 @@ class OneGoogleBarFetcherImplTest : public testing::Test {
safe_json::TestingJsonParser::ScopedFactoryOverride factory_override_;
net::TestURLFetcherFactory url_fetcher_factory_;
sync_preferences::TestingPrefServiceSyncable pref_service_;
TestSigninClient signin_client_;
AccountTrackerService account_tracker_;
FakeSigninManagerBase signin_manager_;
scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
FakeProfileOAuth2TokenService token_service_;
GoogleURLTracker google_url_tracker_;
OneGoogleBarFetcherImpl one_google_bar_fetcher_;
};
TEST_F(OneGoogleBarFetcherImplTest, UnauthenticatedRequestReturns) {
base::MockCallback<OneGoogleBarFetcher::OneGoogleCallback> callback;
one_google_bar_fetcher()->Fetch(callback.Get());
base::Optional<OneGoogleBarData> data;
EXPECT_CALL(callback, Run(OneGoogleBarFetcher::Status::OK, _))
.WillOnce(SaveArg<1>(&data));
RespondWithData(kMinimalValidResponse);
EXPECT_TRUE(data.has_value());
}
TEST_F(OneGoogleBarFetcherImplTest, AuthenticatedRequestReturns) {
SignIn();
TEST_F(OneGoogleBarFetcherImplTest, RequestReturns) {
base::MockCallback<OneGoogleBarFetcher::OneGoogleCallback> callback;
one_google_bar_fetcher()->Fetch(callback.Get());
IssueAccessToken();
base::Optional<OneGoogleBarData> data;
EXPECT_CALL(callback, Run(OneGoogleBarFetcher::Status::OK, _))
.WillOnce(SaveArg<1>(&data));
......@@ -179,57 +125,6 @@ TEST_F(OneGoogleBarFetcherImplTest, AuthenticatedRequestReturns) {
EXPECT_TRUE(data.has_value());
}
TEST_F(OneGoogleBarFetcherImplTest, UnauthenticatedRequestHasApiKey) {
base::MockCallback<OneGoogleBarFetcher::OneGoogleCallback> callback;
one_google_bar_fetcher()->Fetch(callback.Get());
// The request should have an API key (as a query param).
EXPECT_THAT(GetRunningURLFetcher()->GetOriginalURL().query(),
StartsWith("key="));
// But no "Authorization" header.
net::HttpRequestHeaders headers;
GetRunningURLFetcher()->GetExtraRequestHeaders(&headers);
EXPECT_FALSE(headers.HasHeader("Authorization"));
}
TEST_F(OneGoogleBarFetcherImplTest, AuthenticatedRequestHasAuthHeader) {
SignIn();
base::MockCallback<OneGoogleBarFetcher::OneGoogleCallback> callback;
one_google_bar_fetcher()->Fetch(callback.Get());
IssueAccessToken();
// The request should *not* have an API key (as a query param).
EXPECT_THAT(GetRunningURLFetcher()->GetOriginalURL().query(), IsEmpty());
// It should have an "Authorization" header.
net::HttpRequestHeaders headers;
GetRunningURLFetcher()->GetExtraRequestHeaders(&headers);
EXPECT_TRUE(headers.HasHeader("Authorization"));
}
TEST_F(OneGoogleBarFetcherImplTest,
AuthenticatedRequestFallsBackToUnauthenticated) {
SignIn();
base::MockCallback<OneGoogleBarFetcher::OneGoogleCallback> callback;
one_google_bar_fetcher()->Fetch(callback.Get());
IssueAccessTokenError();
// The request should have fallen back to unauthenticated mode with an API key
// (as a query param).
EXPECT_THAT(GetRunningURLFetcher()->GetOriginalURL().query(),
StartsWith("key="));
// But no "Authorization" header.
net::HttpRequestHeaders headers;
GetRunningURLFetcher()->GetExtraRequestHeaders(&headers);
EXPECT_FALSE(headers.HasHeader("Authorization"));
}
TEST_F(OneGoogleBarFetcherImplTest, HandlesResponsePreamble) {
base::MockCallback<OneGoogleBarFetcher::OneGoogleCallback> callback;
one_google_bar_fetcher()->Fetch(callback.Get());
......@@ -251,34 +146,41 @@ TEST_F(OneGoogleBarFetcherImplTest, ParsesFullResponse) {
base::Optional<OneGoogleBarData> data;
EXPECT_CALL(callback, Run(OneGoogleBarFetcher::Status::OK, _))
.WillOnce(SaveArg<1>(&data));
RespondWithData(R"json({"oneGoogleBar": {
"html": { "privateDoNotAccessOrElseSafeHtmlWrappedValue": "bar_html" },
"pageHooks": {
"inHeadScript": {
"privateDoNotAccessOrElseSafeScriptWrappedValue": "in_head_script"
RespondWithData(R"json({"update": { "ogb": {
"html": {
"private_do_not_access_or_else_safe_html_wrapped_value": "bar_html_value"
},
"page_hooks": {
"in_head_script": {
"private_do_not_access_or_else_safe_script_wrapped_value":
"in_head_script_value"
},
"inHeadStyle": {
"privateDoNotAccessOrElseSafeStyleSheetWrappedValue": "in_head_style"
"in_head_style": {
"private_do_not_access_or_else_safe_style_sheet_wrapped_value":
"in_head_style_value"
},
"afterBarScript": {
"privateDoNotAccessOrElseSafeScriptWrappedValue": "after_bar_script"
"after_bar_script": {
"private_do_not_access_or_else_safe_script_wrapped_value":
"after_bar_script_value"
},
"endOfBodyHtml": {
"privateDoNotAccessOrElseSafeHtmlWrappedValue": "end_of_body_html"
"end_of_body_html": {
"private_do_not_access_or_else_safe_html_wrapped_value":
"end_of_body_html_value"
},
"endOfBodyScript": {
"privateDoNotAccessOrElseSafeScriptWrappedValue": "end_of_body_script"
"end_of_body_script": {
"private_do_not_access_or_else_safe_script_wrapped_value":
"end_of_body_script_value"
}
}
}})json");
}}})json");
ASSERT_TRUE(data.has_value());
EXPECT_THAT(data->bar_html, Eq("bar_html"));
EXPECT_THAT(data->in_head_script, Eq("in_head_script"));
EXPECT_THAT(data->in_head_style, Eq("in_head_style"));
EXPECT_THAT(data->after_bar_script, Eq("after_bar_script"));
EXPECT_THAT(data->end_of_body_html, Eq("end_of_body_html"));
EXPECT_THAT(data->end_of_body_script, Eq("end_of_body_script"));
EXPECT_THAT(data->bar_html, Eq("bar_html_value"));
EXPECT_THAT(data->in_head_script, Eq("in_head_script_value"));
EXPECT_THAT(data->in_head_style, Eq("in_head_style_value"));
EXPECT_THAT(data->after_bar_script, Eq("after_bar_script_value"));
EXPECT_THAT(data->end_of_body_html, Eq("end_of_body_html_value"));
EXPECT_THAT(data->end_of_body_script, Eq("end_of_body_script_value"));
}
TEST_F(OneGoogleBarFetcherImplTest, CoalescesMultipleRequests) {
......@@ -342,8 +244,8 @@ TEST_F(OneGoogleBarFetcherImplTest, IncompleteJsonErrorIsFatal) {
EXPECT_CALL(callback,
Run(OneGoogleBarFetcher::Status::FATAL_ERROR, Eq(base::nullopt)));
RespondWithData(R"json({"oneGoogleBar": {
RespondWithData(R"json({"update": { "ogb": {
"html": {},
"pageHooks": {}
}})json");
"page_hooks": {}
}}})json");
}
chrome/browser/search/one_google_bar/one_google_bar_service.cc
View file @ 042810ea
......@@ -10,42 +10,39 @@
#include "base/callback.h"
#include "base/memory/ptr_util.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_fetcher.h"
#include "components/signin/core/browser/signin_manager_base.h"
#include "components/signin/core/browser/gaia_cookie_manager_service.h"
class OneGoogleBarService::SigninObserver : public SigninManagerBase::Observer {
class OneGoogleBarService::SigninObserver
: public GaiaCookieManagerService::Observer {
public:
using SigninStatusChangedCallback = base::Closure;
SigninObserver(SigninManagerBase* signin_manager,
SigninObserver(GaiaCookieManagerService* cookie_service,
const SigninStatusChangedCallback& callback)
: signin_manager_(signin_manager), callback_(callback) {
signin_manager_->AddObserver(this);
: cookie_service_(cookie_service), callback_(callback) {
cookie_service_->AddObserver(this);
}
~SigninObserver() override { signin_manager_->RemoveObserver(this); }
~SigninObserver() override { cookie_service_->RemoveObserver(this); }
private:
// SigninManagerBase::Observer implementation.
void GoogleSigninSucceeded(const std::string& account_id,
const std::string& username) override {
// GaiaCookieManagerService::Observer implementation.
void OnGaiaAccountsInCookieUpdated(const std::vector<gaia::ListedAccount>&,
const std::vector<gaia::ListedAccount>&,
const GoogleServiceAuthError&) override {
callback_.Run();
}
void GoogleSignedOut(const std::string& account_id,
const std::string& username) override {
callback_.Run();
}
SigninManagerBase* const signin_manager_;
GaiaCookieManagerService* const cookie_service_;
SigninStatusChangedCallback callback_;
};
OneGoogleBarService::OneGoogleBarService(
SigninManagerBase* signin_manager,
GaiaCookieManagerService* cookie_service,
std::unique_ptr<OneGoogleBarFetcher> fetcher)
: fetcher_(std::move(fetcher)),
signin_observer_(base::MakeUnique<SigninObserver>(
signin_manager,
cookie_service,
base::Bind(&OneGoogleBarService::SigninStatusChanged,
base::Unretained(this)))) {}
......
chrome/browser/search/one_google_bar/one_google_bar_service.h
View file @ 042810ea
......@@ -14,14 +14,14 @@
#include "chrome/browser/search/one_google_bar/one_google_bar_service_observer.h"
#include "components/keyed_service/core/keyed_service.h"
class SigninManagerBase;
class GaiaCookieManagerService;
// A service that downloads, caches, and hands out OneGoogleBarData. It never
// initiates a download automatically, only when Refresh is called. When the
// user signs in or out, the cached value is cleared.
class OneGoogleBarService : public KeyedService {
public:
OneGoogleBarService(SigninManagerBase* signin_manager,
OneGoogleBarService(GaiaCookieManagerService* cookie_service,
std::unique_ptr<OneGoogleBarFetcher> fetcher);
~OneGoogleBarService() override;
......
chrome/browser/search/one_google_bar/one_google_bar_service_factory.cc
View file @ 042810ea
......@@ -9,11 +9,8 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_fetcher_impl.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_service.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/signin/core/browser/profile_oauth2_token_service.h"
#include "components/signin/core/browser/signin_manager.h"
#include "content/public/browser/browser_context.h"
// static
......@@ -32,8 +29,7 @@ OneGoogleBarServiceFactory::OneGoogleBarServiceFactory()
: BrowserContextKeyedServiceFactory(
"OneGoogleBarService",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(SigninManagerFactory::GetInstance());
DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
DependsOn(GaiaCookieManagerServiceFactory::GetInstance());
DependsOn(GoogleURLTrackerFactory::GetInstance());
}
......@@ -42,14 +38,11 @@ OneGoogleBarServiceFactory::~OneGoogleBarServiceFactory() = default;
KeyedService* OneGoogleBarServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
Profile* profile = Profile::FromBrowserContext(context);
SigninManagerBase* signin_manager =
SigninManagerFactory::GetForProfile(profile);
OAuth2TokenService* token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
GaiaCookieManagerService* cookie_service =
GaiaCookieManagerServiceFactory::GetForProfile(profile);
GoogleURLTracker* google_url_tracker =
GoogleURLTrackerFactory::GetForProfile(profile);
return new OneGoogleBarService(
signin_manager, base::MakeUnique<OneGoogleBarFetcherImpl>(
signin_manager, token_service,
cookie_service, base::MakeUnique<OneGoogleBarFetcherImpl>(
profile->GetRequestContext(), google_url_tracker));
}
chrome/browser/search/one_google_bar/one_google_bar_service_unittest.cc
View file @ 042810ea
......@@ -11,13 +11,16 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/optional.h"
#include "base/test/scoped_task_environment.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_data.h"
#include "chrome/browser/search/one_google_bar/one_google_bar_fetcher.h"
#include "components/signin/core/browser/account_tracker_service.h"
#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
#include "components/signin/core/browser/fake_signin_manager.h"
#include "components/signin/core/browser/fake_gaia_cookie_manager_service.h"
#include "components/signin/core/browser/test_signin_client.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "google_apis/gaia/fake_oauth2_token_service.h"
#include "google_apis/gaia/gaia_constants.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -54,24 +57,46 @@ class OneGoogleBarServiceTest : public testing::Test {
public:
OneGoogleBarServiceTest()
: signin_client_(&pref_service_),
signin_manager_(&signin_client_, &account_tracker_) {
SigninManagerBase::RegisterProfilePrefs(pref_service_.registry());
SigninManagerBase::RegisterPrefs(pref_service_.registry());
fetcher_factory_(/*default_factory=*/nullptr),
cookie_service_(&token_service_,
GaiaConstants::kChromeSource,
&signin_client_) {
// GaiaCookieManagerService calls static methods of AccountTrackerService
// which access prefs.
AccountTrackerService::RegisterPrefs(pref_service_.registry());
cookie_service_.Init(&fetcher_factory_);
auto fetcher = base::MakeUnique<FakeOneGoogleBarFetcher>();
fetcher_ = fetcher.get();
service_ = base::MakeUnique<OneGoogleBarService>(&signin_manager_,
service_ = base::MakeUnique<OneGoogleBarService>(&cookie_service_,
std::move(fetcher));
}
FakeOneGoogleBarFetcher* fetcher() { return fetcher_; }
OneGoogleBarService* service() { return service_.get(); }
void SignIn() {
cookie_service_.SetListAccountsResponseOneAccount("user@gmail.com",
"gaia_id");
cookie_service_.TriggerListAccounts(GaiaConstants::kChromeSource);
task_environment_.RunUntilIdle();
}
void SignOut() {
cookie_service_.SetListAccountsResponseNoAccounts();
cookie_service_.TriggerListAccounts(GaiaConstants::kChromeSource);
task_environment_.RunUntilIdle();
}
private:
base::test::ScopedTaskEnvironment task_environment_;
sync_preferences::TestingPrefServiceSyncable pref_service_;
TestSigninClient signin_client_;
AccountTrackerService account_tracker_;
FakeSigninManagerBase signin_manager_;
FakeOAuth2TokenService token_service_;
net::FakeURLFetcherFactory fetcher_factory_;
FakeGaiaCookieManagerService cookie_service_;
// Owned by the service.
FakeOneGoogleBarFetcher* fetcher_;
......@@ -192,50 +217,7 @@ TEST_F(OneGoogleBarServiceTest, ClearsCacheOnFatalError) {
service()->RemoveObserver(&observer);
}
#if !defined(OS_CHROMEOS)
// Like OneGoogleBarServiceTest, but it has a FakeSigninManager (rather than
// FakeSigninManagerBase), so it can simulate sign-in and sign-out.
class OneGoogleBarServiceSignInTest : public testing::Test {
public:
OneGoogleBarServiceSignInTest()
: signin_client_(&pref_service_),
signin_manager_(&signin_client_,
&token_service_,
&account_tracker_,
nullptr) {
SigninManagerBase::RegisterProfilePrefs(pref_service_.registry());
SigninManagerBase::RegisterPrefs(pref_service_.registry());
AccountTrackerService::RegisterPrefs(pref_service_.registry());
account_tracker_.Initialize(&signin_client_);
auto fetcher = base::MakeUnique<FakeOneGoogleBarFetcher>();
fetcher_ = fetcher.get();
service_ = base::MakeUnique<OneGoogleBarService>(&signin_manager_,
std::move(fetcher));
}
void SignIn() { signin_manager_.SignIn("account", "username", "pass"); }
void SignOut() { signin_manager_.ForceSignOut(); }
FakeOneGoogleBarFetcher* fetcher() { return fetcher_; }
OneGoogleBarService* service() { return service_.get(); }
private:
sync_preferences::TestingPrefServiceSyncable pref_service_;
TestSigninClient signin_client_;
FakeProfileOAuth2TokenService token_service_;
AccountTrackerService account_tracker_;
FakeSigninManager signin_manager_;
// Owned by the service.
FakeOneGoogleBarFetcher* fetcher_;
std::unique_ptr<OneGoogleBarService> service_;
};
TEST_F(OneGoogleBarServiceSignInTest, ResetsOnSignIn) {
TEST_F(OneGoogleBarServiceTest, ResetsOnSignIn) {
// Load some data.
service()->Refresh();
OneGoogleBarData data;
......@@ -248,7 +230,7 @@ TEST_F(OneGoogleBarServiceSignInTest, ResetsOnSignIn) {
EXPECT_THAT(service()->one_google_bar_data(), Eq(base::nullopt));
}
TEST_F(OneGoogleBarServiceSignInTest, ResetsOnSignOut) {
TEST_F(OneGoogleBarServiceTest, ResetsOnSignOut) {
SignIn();
// Load some data.
......@@ -262,5 +244,3 @@ TEST_F(OneGoogleBarServiceSignInTest, ResetsOnSignOut) {
SignOut();
EXPECT_THAT(service()->one_google_bar_data(), Eq(base::nullopt));
}
#endif // OS_CHROMEOS
chrome/browser/ui/search/local_ntp_browsertest.cc
View file @ 042810ea
......@@ -20,7 +20,7 @@
#include "chrome/browser/search/one_google_bar/one_google_bar_service_factory.h"
#include "chrome/browser/search/search.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/search/instant_test_base.h"
......@@ -35,7 +35,6 @@
#include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_data.h"
#include "components/search_engines/template_url_service.h"
#include "components/signin/core/browser/signin_manager.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
......@@ -398,10 +397,11 @@ class LocalNTPOneGoogleBarSmokeTest : public InProcessBrowserTest {
private:
static std::unique_ptr<KeyedService> CreateOneGoogleBarService(
content::BrowserContext* context) {
SigninManagerBase* signin_manager = SigninManagerFactory::GetForProfile(
Profile::FromBrowserContext(context));
GaiaCookieManagerService* cookie_service =
GaiaCookieManagerServiceFactory::GetForProfile(
Profile::FromBrowserContext(context));
return base::MakeUnique<OneGoogleBarService>(
signin_manager, base::MakeUnique<FakeOneGoogleBarFetcher>());
cookie_service, base::MakeUnique<FakeOneGoogleBarFetcher>());
}
void OnWillCreateBrowserContextServices(content::BrowserContext* context) {
......
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