Commit c21c1fe3 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

[NTP Snippets] Have ContextualSuggestionsFetcherImpl use IdentityManager

This CL converts ContextualSuggestionsFetcherImpl from using
//components/signin to using the Identity Service client library.
The conversion is straightforward:

- Get info of primary account via IdentityManager rather than
SigninManager.
- Create PrimaryAccountAccessTokenFetcher via IdentityManager rather
than directly.

I tested the relevant behavior manually by going to the NTP for a
signed-in and syncing test user on Android and confirming that their
content suggestions showed up after this change identically to before
this change.

A followup change will convert the test to use IdentityTestEnvironment.

Bug: 798413
Change-Id: I8da6bc3a812960078c0e22a1239bb00142f1d25e
Reviewed-on: https://chromium-review.googlesource.com/870831
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530461}
parent e08183b6
...@@ -9,8 +9,7 @@ ...@@ -9,8 +9,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/suggestions/image_decoder_impl.h" #include "chrome/browser/search/suggestions/image_decoder_impl.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h" #include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "components/image_fetcher/core/image_decoder.h" #include "components/image_fetcher/core/image_decoder.h"
#include "components/image_fetcher/core/image_fetcher.h" #include "components/image_fetcher/core/image_fetcher.h"
#include "components/image_fetcher/core/image_fetcher_impl.h" #include "components/image_fetcher/core/image_fetcher_impl.h"
...@@ -20,8 +19,6 @@ ...@@ -20,8 +19,6 @@
#include "components/ntp_snippets/remote/cached_image_fetcher.h" #include "components/ntp_snippets/remote/cached_image_fetcher.h"
#include "components/ntp_snippets/remote/remote_suggestions_database.h" #include "components/ntp_snippets/remote/remote_suggestions_database.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.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" #include "content/public/browser/browser_context.h"
#include "content/public/common/service_manager_connection.h" #include "content/public/common/service_manager_connection.h"
#include "services/data_decoder/public/cpp/safe_json_parser.h" #include "services/data_decoder/public/cpp/safe_json_parser.h"
...@@ -75,8 +72,7 @@ ContextualContentSuggestionsServiceFactory:: ...@@ -75,8 +72,7 @@ ContextualContentSuggestionsServiceFactory::
: BrowserContextKeyedServiceFactory( : BrowserContextKeyedServiceFactory(
"ContextualContentSuggestionsService", "ContextualContentSuggestionsService",
BrowserContextDependencyManager::GetInstance()) { BrowserContextDependencyManager::GetInstance()) {
DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance()); DependsOn(IdentityManagerFactory::GetInstance());
DependsOn(SigninManagerFactory::GetInstance());
} }
ContextualContentSuggestionsServiceFactory:: ContextualContentSuggestionsServiceFactory::
...@@ -92,15 +88,13 @@ ContextualContentSuggestionsServiceFactory::BuildServiceInstanceFor( ...@@ -92,15 +88,13 @@ ContextualContentSuggestionsServiceFactory::BuildServiceInstanceFor(
} }
PrefService* pref_service = profile->GetPrefs(); PrefService* pref_service = profile->GetPrefs();
SigninManagerBase* signin_manager = identity::IdentityManager* identity_manager =
SigninManagerFactory::GetForProfile(profile); IdentityManagerFactory::GetForProfile(profile);
OAuth2TokenService* token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
scoped_refptr<net::URLRequestContextGetter> request_context = scoped_refptr<net::URLRequestContextGetter> request_context =
profile->GetRequestContext(); profile->GetRequestContext();
auto contextual_suggestions_fetcher = auto contextual_suggestions_fetcher =
base::MakeUnique<ContextualSuggestionsFetcherImpl>( base::MakeUnique<ContextualSuggestionsFetcherImpl>(
signin_manager, token_service, request_context, pref_service, identity_manager, request_context, pref_service,
base::Bind(&data_decoder::SafeJsonParser::Parse, base::Bind(&data_decoder::SafeJsonParser::Parse,
content::ServiceManagerConnection::GetForProcess() content::ServiceManagerConnection::GetForProcess()
->GetConnector())); ->GetConnector()));
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#include "net/url_request/url_fetcher.h" #include "net/url_request/url_fetcher.h"
#include "net/url_request/url_request_status.h" #include "net/url_request/url_request_status.h"
#include "services/identity/public/cpp/identity_manager.h"
#include "services/identity/public/cpp/primary_account_access_token_fetcher.h" #include "services/identity/public/cpp/primary_account_access_token_fetcher.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -110,13 +111,11 @@ bool AddSuggestionsFromListValue(bool content_suggestions_api, ...@@ -110,13 +111,11 @@ bool AddSuggestionsFromListValue(bool content_suggestions_api,
} // namespace } // namespace
ContextualSuggestionsFetcherImpl::ContextualSuggestionsFetcherImpl( ContextualSuggestionsFetcherImpl::ContextualSuggestionsFetcherImpl(
SigninManagerBase* signin_manager, identity::IdentityManager* identity_manager,
OAuth2TokenService* token_service,
scoped_refptr<URLRequestContextGetter> url_request_context_getter, scoped_refptr<URLRequestContextGetter> url_request_context_getter,
PrefService* pref_service, PrefService* pref_service,
const ParseJSONCallback& parse_json_callback) const ParseJSONCallback& parse_json_callback)
: signin_manager_(signin_manager), : identity_manager_(identity_manager),
token_service_(token_service),
url_request_context_getter_(std::move(url_request_context_getter)), url_request_context_getter_(std::move(url_request_context_getter)),
parse_json_callback_(parse_json_callback), parse_json_callback_(parse_json_callback),
fetch_url_(GetFetchEndpoint()) {} fetch_url_(GetFetchEndpoint()) {}
...@@ -152,7 +151,7 @@ void ContextualSuggestionsFetcherImpl::StartRequest( ...@@ -152,7 +151,7 @@ void ContextualSuggestionsFetcherImpl::StartRequest(
SuggestionsAvailableCallback callback, SuggestionsAvailableCallback callback,
const std::string& oauth_access_token) { const std::string& oauth_access_token) {
builder.SetUrl(fetch_url_) builder.SetUrl(fetch_url_)
.SetAuthentication(signin_manager_->GetAuthenticatedAccountId(), .SetAuthentication(identity_manager_->GetPrimaryAccountInfo().account_id,
base::StringPrintf(kAuthorizationRequestHeaderFormat, base::StringPrintf(kAuthorizationRequestHeaderFormat,
oauth_access_token.c_str())); oauth_access_token.c_str()));
DVLOG(1) << "ContextualSuggestionsFetcherImpl::StartRequest"; DVLOG(1) << "ContextualSuggestionsFetcherImpl::StartRequest";
...@@ -170,8 +169,8 @@ void ContextualSuggestionsFetcherImpl::StartTokenRequest() { ...@@ -170,8 +169,8 @@ void ContextualSuggestionsFetcherImpl::StartTokenRequest() {
} }
OAuth2TokenService::ScopeSet scopes{kContentSuggestionsApiScope}; OAuth2TokenService::ScopeSet scopes{kContentSuggestionsApiScope};
token_fetcher_ = std::make_unique<identity::PrimaryAccountAccessTokenFetcher>( token_fetcher_ = identity_manager_->CreateAccessTokenFetcherForPrimaryAccount(
"ntp_snippets", signin_manager_, token_service_, scopes, "ntp_snippets", scopes,
base::BindOnce( base::BindOnce(
&ContextualSuggestionsFetcherImpl::AccessTokenFetchFinished, &ContextualSuggestionsFetcherImpl::AccessTokenFetchFinished,
base::Unretained(this)), base::Unretained(this)),
......
...@@ -21,11 +21,10 @@ ...@@ -21,11 +21,10 @@
#include "components/ntp_snippets/status.h" #include "components/ntp_snippets/status.h"
#include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_context_getter.h"
class OAuth2TokenService;
class PrefService; class PrefService;
class SigninManagerBase;
namespace identity { namespace identity {
class IdentityManager;
class PrimaryAccountAccessTokenFetcher; class PrimaryAccountAccessTokenFetcher;
} }
...@@ -36,8 +35,7 @@ namespace ntp_snippets { ...@@ -36,8 +35,7 @@ namespace ntp_snippets {
class ContextualSuggestionsFetcherImpl : public ContextualSuggestionsFetcher { class ContextualSuggestionsFetcherImpl : public ContextualSuggestionsFetcher {
public: public:
ContextualSuggestionsFetcherImpl( ContextualSuggestionsFetcherImpl(
SigninManagerBase* signin_manager, identity::IdentityManager* identity_manager,
OAuth2TokenService* token_service,
scoped_refptr<net::URLRequestContextGetter> url_request_context_getter, scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
PrefService* pref_service, PrefService* pref_service,
const ParseJSONCallback& parse_json_callback); const ParseJSONCallback& parse_json_callback);
...@@ -77,8 +75,7 @@ class ContextualSuggestionsFetcherImpl : public ContextualSuggestionsFetcher { ...@@ -77,8 +75,7 @@ class ContextualSuggestionsFetcherImpl : public ContextualSuggestionsFetcher {
ContextualSuggestion::PtrVector* suggestions); ContextualSuggestion::PtrVector* suggestions);
// Authentication for signed-in users. // Authentication for signed-in users.
SigninManagerBase* signin_manager_; identity::IdentityManager* identity_manager_;
OAuth2TokenService* token_service_;
std::unique_ptr<identity::PrimaryAccountAccessTokenFetcher> token_fetcher_; std::unique_ptr<identity::PrimaryAccountAccessTokenFetcher> token_fetcher_;
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "google_apis/gaia/fake_oauth2_token_service_delegate.h" #include "google_apis/gaia/fake_oauth2_token_service_delegate.h"
#include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_test_util.h" #include "net/url_request/url_request_test_util.h"
#include "services/identity/public/cpp/identity_manager.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -117,14 +118,16 @@ class ContextualSuggestionsFetcherTest ...@@ -117,14 +118,16 @@ class ContextualSuggestionsFetcherTest
fake_token_service_ = std::make_unique<FakeProfileOAuth2TokenService>( fake_token_service_ = std::make_unique<FakeProfileOAuth2TokenService>(
std::make_unique<FakeOAuth2TokenServiceDelegate>( std::make_unique<FakeOAuth2TokenServiceDelegate>(
request_context_getter.get())); request_context_getter.get()));
identity_manager_ = std::make_unique<identity::IdentityManager>(
test_utils_.fake_signin_manager(), fake_token_service_.get());
fetcher_ = std::make_unique<ContextualSuggestionsFetcherImpl>( fetcher_ = std::make_unique<ContextualSuggestionsFetcherImpl>(
test_utils_.fake_signin_manager(), fake_token_service_.get(), identity_manager_.get(), std::move(request_context_getter),
std::move(request_context_getter), test_utils_.pref_service(), test_utils_.pref_service(), base::BindRepeating(&ParseJson));
base::Bind(&ParseJson));
fake_token_service_->AddDiagnosticsObserver(this); fake_token_service_->AddDiagnosticsObserver(this);
} }
~ContextualSuggestionsFetcherTest() override { ~ContextualSuggestionsFetcherTest() override {
identity_manager_.reset();
fake_token_service_->RemoveDiagnosticsObserver(this); fake_token_service_->RemoveDiagnosticsObserver(this);
} }
...@@ -133,12 +136,8 @@ class ContextualSuggestionsFetcherTest ...@@ -133,12 +136,8 @@ class ContextualSuggestionsFetcherTest
} }
void InitializeFakeCredentials() { void InitializeFakeCredentials() {
#if defined(OS_CHROMEOS) identity_manager_->SetPrimaryAccountSynchronouslyForTests(
test_utils_.fake_signin_manager()->SignIn(kTestEmail); kTestEmail, kTestEmail, "token");
#else
test_utils_.fake_signin_manager()->SignIn(kTestEmail, "user", "password");
#endif
fake_token_service_->GetDelegate()->UpdateCredentials(kTestEmail, "token");
} }
void IssueOAuth2Token() { void IssueOAuth2Token() {
...@@ -180,6 +179,9 @@ class ContextualSuggestionsFetcherTest ...@@ -180,6 +179,9 @@ class ContextualSuggestionsFetcherTest
} }
std::unique_ptr<FakeProfileOAuth2TokenService> fake_token_service_; std::unique_ptr<FakeProfileOAuth2TokenService> fake_token_service_;
// TODO(blundell): Convert this test to use IdentityTestEnvironment.
std::unique_ptr<identity::IdentityManager> identity_manager_;
std::unique_ptr<net::FakeURLFetcherFactory> fake_url_fetcher_factory_; std::unique_ptr<net::FakeURLFetcherFactory> fake_url_fetcher_factory_;
std::unique_ptr<ContextualSuggestionsFetcherImpl> fetcher_; std::unique_ptr<ContextualSuggestionsFetcherImpl> fetcher_;
MockSuggestionsAvailableCallback mock_suggestions_available_callback_; MockSuggestionsAvailableCallback mock_suggestions_available_callback_;
......
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