Commit 77d581a5 authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

[s13n] Convert safe_search_url_reporter.cc to IdentityManager

CL replaces the Use of OAuth2TokenService, OAuth2TokenService::Consumer
and SigninManager APIs with the corresponding IdentityManager counterparts.

The unittests could not be only minimally adapted, and converted on a
follow up pass, given that CL needed to replace the uses of
FakeProfileOAuth2TokenService with IdentityTestEnvironment upfront.

TBR=blundell@chromium.org,treib@chromium.org (both have already approved
the previously incarnation of this CL. The CL was accidentally deleted
when being sent to CQ).

BUG=907529

Change-Id: If359e0a0728f87281a9f0c6bbe09ee10db2c82d3
Reviewed-on: https://chromium-review.googlesource.com/c/1348670
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: default avatarAntonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#610480}
parent e69a4fba
...@@ -9,13 +9,9 @@ ...@@ -9,13 +9,9 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.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 "chrome/browser/supervised_user/supervised_user_constants.h" #include "chrome/browser/supervised_user/supervised_user_constants.h"
#include "components/data_use_measurement/core/data_use_user_data.h" #include "components/data_use_measurement/core/data_use_user_data.h"
#include "components/signin/core/browser/profile_oauth2_token_service.h"
#include "components/signin/core/browser/signin_manager.h"
#include "components/signin/core/browser/signin_manager_base.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"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
...@@ -23,6 +19,9 @@ ...@@ -23,6 +19,9 @@
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/http/http_status_code.h" #include "net/http/http_status_code.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
#include "services/identity/public/cpp/access_token_fetcher.h"
#include "services/identity/public/cpp/access_token_info.h"
#include "services/identity/public/cpp/identity_manager.h"
#include "services/network/public/cpp/resource_request.h" #include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h" #include "services/network/public/cpp/simple_url_loader.h"
...@@ -41,7 +40,8 @@ struct SafeSearchURLReporter::Report { ...@@ -41,7 +40,8 @@ struct SafeSearchURLReporter::Report {
GURL url; GURL url;
SuccessCallback callback; SuccessCallback callback;
std::unique_ptr<OAuth2TokenService::Request> access_token_request; std::unique_ptr<identity::AccessTokenFetcher> access_token_fetcher;
std::string access_token; std::string access_token;
bool access_token_expired; bool access_token_expired;
std::unique_ptr<network::SimpleURLLoader> simple_url_loader; std::unique_ptr<network::SimpleURLLoader> simple_url_loader;
...@@ -53,11 +53,10 @@ SafeSearchURLReporter::Report::Report(const GURL& url, SuccessCallback callback) ...@@ -53,11 +53,10 @@ SafeSearchURLReporter::Report::Report(const GURL& url, SuccessCallback callback)
SafeSearchURLReporter::Report::~Report() {} SafeSearchURLReporter::Report::~Report() {}
SafeSearchURLReporter::SafeSearchURLReporter( SafeSearchURLReporter::SafeSearchURLReporter(
OAuth2TokenService* oauth2_token_service, identity::IdentityManager* identity_manager,
const std::string& account_id, const std::string& account_id,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: OAuth2TokenService::Consumer("safe_search_url_reporter"), : identity_manager_(identity_manager),
oauth2_token_service_(oauth2_token_service),
account_id_(account_id), account_id_(account_id),
url_loader_factory_(std::move(url_loader_factory)) {} url_loader_factory_(std::move(url_loader_factory)) {}
...@@ -66,11 +65,9 @@ SafeSearchURLReporter::~SafeSearchURLReporter() {} ...@@ -66,11 +65,9 @@ SafeSearchURLReporter::~SafeSearchURLReporter() {}
// static // static
std::unique_ptr<SafeSearchURLReporter> SafeSearchURLReporter::CreateWithProfile( std::unique_ptr<SafeSearchURLReporter> SafeSearchURLReporter::CreateWithProfile(
Profile* profile) { Profile* profile) {
ProfileOAuth2TokenService* token_service = auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
SigninManagerBase* signin = SigninManagerFactory::GetForProfile(profile);
return base::WrapUnique(new SafeSearchURLReporter( return base::WrapUnique(new SafeSearchURLReporter(
token_service, signin->GetAuthenticatedAccountId(), identity_manager, identity_manager->GetPrimaryAccountId(),
content::BrowserContext::GetDefaultStoragePartition(profile) content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess())); ->GetURLLoaderFactoryForBrowserProcess()));
} }
...@@ -82,24 +79,40 @@ void SafeSearchURLReporter::ReportUrl(const GURL& url, ...@@ -82,24 +79,40 @@ void SafeSearchURLReporter::ReportUrl(const GURL& url,
} }
void SafeSearchURLReporter::StartFetching(Report* report) { void SafeSearchURLReporter::StartFetching(Report* report) {
OAuth2TokenService::ScopeSet scopes; identity::ScopeSet scopes;
scopes.insert(kSafeSearchReportApiScope); scopes.insert(kSafeSearchReportApiScope);
report->access_token_request = // It is safe to use Unretained(this) here given that the callback
oauth2_token_service_->StartRequest(account_id_, scopes, this); // will not be invoke if this object is deleted. Likewise, |report|
// only comes from |reports_|, which are owned by this object too.
report->access_token_fetcher =
identity_manager_->CreateAccessTokenFetcherForAccount(
account_id_, "safe_search_url_reporter", scopes,
base::BindOnce(&SafeSearchURLReporter::OnAccessTokenFetchComplete,
base::Unretained(this), report),
identity::AccessTokenFetcher::Mode::kImmediate);
} }
void SafeSearchURLReporter::OnGetTokenSuccess( void SafeSearchURLReporter::OnAccessTokenFetchComplete(
const OAuth2TokenService::Request* request, Report* report,
const OAuth2AccessTokenConsumer::TokenResponse& token_response) { GoogleServiceAuthError error,
identity::AccessTokenInfo token_info) {
auto it = reports_.begin(); auto it = reports_.begin();
while (it != reports_.end()) { while (it != reports_.end()) {
if (request == (*it)->access_token_request.get()) if (report->access_token_fetcher.get() ==
(*it)->access_token_fetcher.get()) {
break; break;
}
it++; it++;
} }
DCHECK(it != reports_.end()); DCHECK(it != reports_.end());
(*it)->access_token = token_response.access_token; if (error.state() != GoogleServiceAuthError::NONE) {
LOG(WARNING) << "Token error: " << error.ToString();
DispatchResult(it, false);
return;
}
(*it)->access_token = token_info.token;
net::NetworkTrafficAnnotationTag traffic_annotation = net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("safe_search_url_reporter", R"( net::DefineNetworkTrafficAnnotation("safe_search_url_reporter", R"(
...@@ -134,7 +147,7 @@ void SafeSearchURLReporter::OnGetTokenSuccess( ...@@ -134,7 +147,7 @@ void SafeSearchURLReporter::OnGetTokenSuccess(
resource_request->headers.SetHeader( resource_request->headers.SetHeader(
net::HttpRequestHeaders::kAuthorization, net::HttpRequestHeaders::kAuthorization,
base::StringPrintf(supervised_users::kAuthorizationHeaderFormat, base::StringPrintf(supervised_users::kAuthorizationHeaderFormat,
token_response.access_token.c_str())); token_info.token.c_str()));
(*it)->simple_url_loader = network::SimpleURLLoader::Create( (*it)->simple_url_loader = network::SimpleURLLoader::Create(
std::move(resource_request), traffic_annotation); std::move(resource_request), traffic_annotation);
...@@ -155,20 +168,6 @@ void SafeSearchURLReporter::OnGetTokenSuccess( ...@@ -155,20 +168,6 @@ void SafeSearchURLReporter::OnGetTokenSuccess(
base::Unretained(this), std::move(it))); base::Unretained(this), std::move(it)));
} }
void SafeSearchURLReporter::OnGetTokenFailure(
const OAuth2TokenService::Request* request,
const GoogleServiceAuthError& error) {
auto it = reports_.begin();
while (it != reports_.end()) {
if (request == (*it)->access_token_request.get())
break;
it++;
}
DCHECK(it != reports_.end());
LOG(WARNING) << "Token error: " << error.ToString();
DispatchResult(it, false);
}
void SafeSearchURLReporter::OnSimpleLoaderComplete( void SafeSearchURLReporter::OnSimpleLoaderComplete(
ReportList::iterator it, ReportList::iterator it,
std::unique_ptr<std::string> response_body) { std::unique_ptr<std::string> response_body) {
...@@ -183,9 +182,9 @@ void SafeSearchURLReporter::OnSimpleLoaderComplete( ...@@ -183,9 +182,9 @@ void SafeSearchURLReporter::OnSimpleLoaderComplete(
if (response_code == net::HTTP_UNAUTHORIZED && if (response_code == net::HTTP_UNAUTHORIZED &&
!report->access_token_expired) { !report->access_token_expired) {
(*it)->access_token_expired = true; (*it)->access_token_expired = true;
OAuth2TokenService::ScopeSet scopes; identity::ScopeSet scopes;
scopes.insert(kSafeSearchReportApiScope); scopes.insert(kSafeSearchReportApiScope);
oauth2_token_service_->InvalidateAccessToken(account_id_, scopes, identity_manager_->RemoveAccessTokenFromCache(account_id_, scopes,
report->access_token); report->access_token);
StartFetching(report); StartFetching(report);
return; return;
......
...@@ -5,30 +5,36 @@ ...@@ -5,30 +5,36 @@
#ifndef CHROME_BROWSER_SUPERVISED_USER_EXPERIMENTAL_SAFE_SEARCH_URL_REPORTER_H_ #ifndef CHROME_BROWSER_SUPERVISED_USER_EXPERIMENTAL_SAFE_SEARCH_URL_REPORTER_H_
#define CHROME_BROWSER_SUPERVISED_USER_EXPERIMENTAL_SAFE_SEARCH_URL_REPORTER_H_ #define CHROME_BROWSER_SUPERVISED_USER_EXPERIMENTAL_SAFE_SEARCH_URL_REPORTER_H_
#include <list>
#include <memory> #include <memory>
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "google_apis/gaia/oauth2_token_service.h" #include "google_apis/gaia/google_service_auth_error.h"
#include "url/gurl.h" #include "url/gurl.h"
class GURL; class GURL;
class Profile; class Profile;
namespace identity {
class IdentityManager;
struct AccessTokenInfo;
} // namespace identity
namespace network { namespace network {
class SharedURLLoaderFactory; class SharedURLLoaderFactory;
} }
class SafeSearchURLReporter : public OAuth2TokenService::Consumer { class SafeSearchURLReporter {
public: public:
using SuccessCallback = base::OnceCallback<void(bool)>; using SuccessCallback = base::OnceCallback<void(bool)>;
SafeSearchURLReporter( SafeSearchURLReporter(
OAuth2TokenService* oauth2_token_service, identity::IdentityManager* identity_manager,
const std::string& account_id, const std::string& account_id,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory); scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
~SafeSearchURLReporter() override; ~SafeSearchURLReporter();
static std::unique_ptr<SafeSearchURLReporter> CreateWithProfile( static std::unique_ptr<SafeSearchURLReporter> CreateWithProfile(
Profile* profile); Profile* profile);
...@@ -39,12 +45,9 @@ class SafeSearchURLReporter : public OAuth2TokenService::Consumer { ...@@ -39,12 +45,9 @@ class SafeSearchURLReporter : public OAuth2TokenService::Consumer {
struct Report; struct Report;
using ReportList = std::list<std::unique_ptr<Report>>; using ReportList = std::list<std::unique_ptr<Report>>;
// OAuth2TokenService::Consumer implementation: void OnAccessTokenFetchComplete(Report* report,
void OnGetTokenSuccess( GoogleServiceAuthError error,
const OAuth2TokenService::Request* request, identity::AccessTokenInfo token_info);
const OAuth2AccessTokenConsumer::TokenResponse& token_response) override;
void OnGetTokenFailure(const OAuth2TokenService::Request* request,
const GoogleServiceAuthError& error) override;
void OnSimpleLoaderComplete(ReportList::iterator it, void OnSimpleLoaderComplete(ReportList::iterator it,
std::unique_ptr<std::string> response_body); std::unique_ptr<std::string> response_body);
...@@ -55,7 +58,7 @@ class SafeSearchURLReporter : public OAuth2TokenService::Consumer { ...@@ -55,7 +58,7 @@ class SafeSearchURLReporter : public OAuth2TokenService::Consumer {
void DispatchResult(ReportList::iterator it, bool success); void DispatchResult(ReportList::iterator it, bool success);
OAuth2TokenService* oauth2_token_service_; identity::IdentityManager* identity_manager_;
std::string account_id_; std::string account_id_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
......
...@@ -9,10 +9,9 @@ ...@@ -9,10 +9,9 @@
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/values.h" #include "base/values.h"
#include "components/prefs/testing_pref_service.h"
#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/http/http_util.h" #include "net/http/http_util.h"
#include "services/identity/public/cpp/identity_test_environment.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h" #include "services/network/test/test_url_loader_factory.h"
...@@ -23,31 +22,33 @@ namespace { ...@@ -23,31 +22,33 @@ namespace {
const char kSafeSearchReportApiUrl[] = const char kSafeSearchReportApiUrl[] =
"https://safesearch.googleapis.com/v1:report"; "https://safesearch.googleapis.com/v1:report";
const char kAccountId[] = "account@gmail.com"; const char kEmail[] = "account@gmail.com";
} // namespace } // namespace
class SafeSearchURLReporterTest : public testing::Test { class SafeSearchURLReporterTest : public testing::Test {
public: public:
SafeSearchURLReporterTest() SafeSearchURLReporterTest()
: token_service_(&pref_service_), : test_shared_loader_factory_(
test_shared_loader_factory_(
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>( base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)), &test_url_loader_factory_)) {
report_url_(&token_service_, kAccountId, test_shared_loader_factory_) { AccountInfo account_info = identity_test_env_.MakeAccountAvailable(kEmail);
token_service_.UpdateCredentials(kAccountId, "refresh_token"); account_id_ = account_info.account_id;
report_url_ = std::make_unique<SafeSearchURLReporter>(
identity_test_env_.identity_manager(), account_id_,
test_shared_loader_factory_);
} }
protected: protected:
void IssueAccessTokens() { void IssueAccessTokens() {
token_service_.IssueAllTokensForAccount( identity_test_env_.WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
kAccountId, "access_token", account_id_, "access_token",
base::Time::Now() + base::TimeDelta::FromHours(1)); base::Time::Now() + base::TimeDelta::FromHours(1));
} }
void IssueAccessTokenErrors() { void IssueAccessTokenErrors() {
token_service_.IssueErrorForAllPendingRequestsForAccount( identity_test_env_.WaitForAccessTokenRequestIfNecessaryAndRespondWithError(
kAccountId, GoogleServiceAuthError::FromServiceError("Error!")); account_id_, GoogleServiceAuthError::FromServiceError("Error!"));
} }
void SetupResponse(net::Error error) { void SetupResponse(net::Error error) {
...@@ -61,7 +62,7 @@ class SafeSearchURLReporterTest : public testing::Test { ...@@ -61,7 +62,7 @@ class SafeSearchURLReporterTest : public testing::Test {
} }
void CreateRequest(const GURL& url) { void CreateRequest(const GURL& url) {
report_url_.ReportUrl( report_url_->ReportUrl(
url, base::BindOnce(&SafeSearchURLReporterTest::OnRequestCreated, url, base::BindOnce(&SafeSearchURLReporterTest::OnRequestCreated,
base::Unretained(this))); base::Unretained(this)));
} }
...@@ -71,19 +72,17 @@ class SafeSearchURLReporterTest : public testing::Test { ...@@ -71,19 +72,17 @@ class SafeSearchURLReporterTest : public testing::Test {
MOCK_METHOD1(OnRequestCreated, void(bool success)); MOCK_METHOD1(OnRequestCreated, void(bool success));
base::MessageLoop message_loop_; base::MessageLoop message_loop_;
TestingPrefServiceSimple pref_service_; std::string account_id_;
FakeProfileOAuth2TokenService token_service_; identity::IdentityTestEnvironment identity_test_env_;
network::TestURLLoaderFactory test_url_loader_factory_; network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
SafeSearchURLReporter report_url_; std::unique_ptr<SafeSearchURLReporter> report_url_;
}; };
TEST_F(SafeSearchURLReporterTest, Success) { TEST_F(SafeSearchURLReporterTest, Success) {
CreateRequest(GURL("http://google.com")); CreateRequest(GURL("http://google.com"));
CreateRequest(GURL("http://url.com")); CreateRequest(GURL("http://url.com"));
EXPECT_GT(token_service_.GetPendingRequests().size(), 0U);
IssueAccessTokens(); IssueAccessTokens();
EXPECT_CALL(*this, OnRequestCreated(true)).Times(2); EXPECT_CALL(*this, OnRequestCreated(true)).Times(2);
...@@ -95,8 +94,6 @@ TEST_F(SafeSearchURLReporterTest, Success) { ...@@ -95,8 +94,6 @@ TEST_F(SafeSearchURLReporterTest, Success) {
TEST_F(SafeSearchURLReporterTest, AccessTokenError) { TEST_F(SafeSearchURLReporterTest, AccessTokenError) {
CreateRequest(GURL("http://google.com")); CreateRequest(GURL("http://google.com"));
EXPECT_EQ(1U, token_service_.GetPendingRequests().size());
EXPECT_CALL(*this, OnRequestCreated(false)); EXPECT_CALL(*this, OnRequestCreated(false));
IssueAccessTokenErrors(); IssueAccessTokenErrors();
} }
...@@ -104,8 +101,6 @@ TEST_F(SafeSearchURLReporterTest, AccessTokenError) { ...@@ -104,8 +101,6 @@ TEST_F(SafeSearchURLReporterTest, AccessTokenError) {
TEST_F(SafeSearchURLReporterTest, NetworkError) { TEST_F(SafeSearchURLReporterTest, NetworkError) {
CreateRequest(GURL("http://google.com")); CreateRequest(GURL("http://google.com"));
EXPECT_EQ(1U, token_service_.GetPendingRequests().size());
IssueAccessTokens(); IssueAccessTokens();
EXPECT_CALL(*this, OnRequestCreated(false)); EXPECT_CALL(*this, OnRequestCreated(false));
......
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