Commit 74b53bf1 authored by Mark Pilgrim's avatar Mark Pilgrim Committed by Commit Bot

Migrate SMSService to SimpleURLLoader

Bug: 844920
Change-Id: Iabbe4f3367ca8ade1afa09d595bc05483f0e1d49
TBR: pkasting@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1070328
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561687}
parent a5d910ff
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include "components/sync_preferences/pref_service_mock_factory.h" #include "components/sync_preferences/pref_service_mock_factory.h"
#include "components/sync_preferences/testing_pref_service_syncable.h" #include "components/sync_preferences/testing_pref_service_syncable.h"
#include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_browser_thread_bundle.h"
#include "services/network/public/cpp/shared_url_loader_factory.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"
......
...@@ -18,9 +18,9 @@ ...@@ -18,9 +18,9 @@
#include "net/http/http_status_code.h" #include "net/http/http_status_code.h"
#include "net/http/http_util.h" #include "net/http/http_util.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
#include "net/url_request/url_fetcher.h" #include "services/network/public/cpp/resource_request.h"
#include "net/url_request/url_fetcher_delegate.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "net/url_request/url_request_context_getter.h" #include "services/network/public/cpp/simple_url_loader.h"
namespace { namespace {
...@@ -41,8 +41,7 @@ const char kSendSMSPromoFormat[] = "{promo_id:%s}"; ...@@ -41,8 +41,7 @@ const char kSendSMSPromoFormat[] = "{promo_id:%s}";
const size_t kMaxRetries = 1; const size_t kMaxRetries = 1;
class RequestImpl : public SMSService::Request, class RequestImpl : public SMSService::Request,
private OAuth2TokenService::Consumer, private OAuth2TokenService::Consumer {
private net::URLFetcherDelegate {
public: public:
~RequestImpl() override {} ~RequestImpl() override {}
...@@ -58,16 +57,15 @@ class RequestImpl : public SMSService::Request, ...@@ -58,16 +57,15 @@ class RequestImpl : public SMSService::Request,
private: private:
friend class ::SMSService; friend class ::SMSService;
RequestImpl( RequestImpl(OAuth2TokenService* token_service,
OAuth2TokenService* token_service, SigninManagerBase* signin_manager,
SigninManagerBase* signin_manager, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const scoped_refptr<net::URLRequestContextGetter>& request_context, const GURL& url,
const GURL& url, const SMSService::CompletionCallback& callback)
const SMSService::CompletionCallback& callback)
: OAuth2TokenService::Consumer("desktop_ios_promotion"), : OAuth2TokenService::Consumer("desktop_ios_promotion"),
token_service_(token_service), token_service_(token_service),
signin_manager_(signin_manager), signin_manager_(signin_manager),
request_context_(request_context), url_loader_factory_(std::move(url_loader_factory)),
url_(url), url_(url),
post_data_mime_type_(kPostDataMimeType), post_data_mime_type_(kPostDataMimeType),
response_code_(0), response_code_(0),
...@@ -76,7 +74,7 @@ class RequestImpl : public SMSService::Request, ...@@ -76,7 +74,7 @@ class RequestImpl : public SMSService::Request,
is_pending_(false) { is_pending_(false) {
DCHECK(token_service_); DCHECK(token_service_);
DCHECK(signin_manager_); DCHECK(signin_manager_);
DCHECK(request_context_); DCHECK(url_loader_factory_);
} }
void Start() override { void Start() override {
...@@ -87,10 +85,13 @@ class RequestImpl : public SMSService::Request, ...@@ -87,10 +85,13 @@ class RequestImpl : public SMSService::Request,
is_pending_ = true; is_pending_ = true;
} }
// content::URLFetcherDelegate interface. void OnSimpleLoaderComplete(std::unique_ptr<std::string> response_body) {
void OnURLFetchComplete(const net::URLFetcher* source) override { response_code_ = -1;
DCHECK_EQ(source, url_fetcher_.get()); if (simple_url_loader_->ResponseInfo() &&
response_code_ = url_fetcher_->GetResponseCode(); simple_url_loader_->ResponseInfo()->headers) {
response_code_ =
simple_url_loader_->ResponseInfo()->headers->response_code();
}
UMA_HISTOGRAM_CUSTOM_ENUMERATION( UMA_HISTOGRAM_CUSTOM_ENUMERATION(
"DesktopIOSPromotion.OAuthTokenResponseCode", "DesktopIOSPromotion.OAuthTokenResponseCode",
...@@ -110,10 +111,15 @@ class RequestImpl : public SMSService::Request, ...@@ -110,10 +111,15 @@ class RequestImpl : public SMSService::Request,
Start(); Start();
return; return;
} }
url_fetcher_->GetResponseAsString(&response_body_); bool success = !!response_body;
url_fetcher_.reset(); if (success) {
response_body_ = std::move(*response_body);
} else {
response_body_.clear();
}
simple_url_loader_.reset();
is_pending_ = false; is_pending_ = false;
callback_.Run(this, response_code_ == net::HTTP_OK); callback_.Run(this, success);
// It is valid for the callback to delete |this|, so do not access any // It is valid for the callback to delete |this|, so do not access any
// members below here. // members below here.
} }
...@@ -129,27 +135,6 @@ class RequestImpl : public SMSService::Request, ...@@ -129,27 +135,6 @@ class RequestImpl : public SMSService::Request,
UMA_HISTOGRAM_BOOLEAN("DesktopIOSPromotion.OAuthTokenCompletion", true); UMA_HISTOGRAM_BOOLEAN("DesktopIOSPromotion.OAuthTokenCompletion", true);
// Got an access token -- start the actual API request. // Got an access token -- start the actual API request.
url_fetcher_ = CreateUrlFetcher(access_token);
url_fetcher_->Start();
}
void OnGetTokenFailure(const OAuth2TokenService::Request* request,
const GoogleServiceAuthError& error) override {
token_request_.reset();
is_pending_ = false;
UMA_HISTOGRAM_BOOLEAN("DesktopIOSPromotion.OAuthTokenCompletion", false);
callback_.Run(this, false);
// It is valid for the callback to delete |this|, so do not access any
// members below here.
}
// Helper for creating a new URLFetcher for the API request.
std::unique_ptr<net::URLFetcher> CreateUrlFetcher(
const std::string& access_token) {
net::URLFetcher::RequestType request_type =
post_data_ ? net::URLFetcher::POST : net::URLFetcher::GET;
net::NetworkTrafficAnnotationTag traffic_annotation = net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("desktop_ios_promotion", R"( net::DefineNetworkTrafficAnnotation("desktop_ios_promotion", R"(
semantics { semantics {
...@@ -179,20 +164,39 @@ class RequestImpl : public SMSService::Request, ...@@ -179,20 +164,39 @@ class RequestImpl : public SMSService::Request,
"Not implemented, considered not useful as it does not upload any " "Not implemented, considered not useful as it does not upload any "
"data and just downloads a recovery number." "data and just downloads a recovery number."
})"); })");
std::unique_ptr<net::URLFetcher> fetcher = auto resource_request = std::make_unique<network::ResourceRequest>();
net::URLFetcher::Create(url_, request_type, this, traffic_annotation); resource_request->url = url_;
fetcher->SetRequestContext(request_context_.get()); resource_request->load_flags =
fetcher->SetMaxRetriesOn5xx(kMaxRetries); net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES;
fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES | resource_request->method = post_data_ ? "POST" : "GET";
net::LOAD_DO_NOT_SAVE_COOKIES); resource_request->headers.AddHeaderFromString("Authorization: Bearer " +
fetcher->AddExtraRequestHeader("Authorization: Bearer " + access_token); access_token);
fetcher->AddExtraRequestHeader( resource_request->headers.AddHeaderFromString(
"X-Developer-Key: " + "X-Developer-Key: " +
GaiaUrls::GetInstance()->oauth2_chrome_client_id()); GaiaUrls::GetInstance()->oauth2_chrome_client_id());
simple_url_loader_ = network::SimpleURLLoader::Create(
std::move(resource_request), traffic_annotation);
simple_url_loader_->SetRetryOptions(kMaxRetries,
network::SimpleURLLoader::RETRY_ON_5XX);
if (post_data_) if (post_data_)
fetcher->SetUploadData(post_data_mime_type_, post_data_.value()); simple_url_loader_->AttachStringForUpload(post_data_.value(),
return fetcher; post_data_mime_type_);
simple_url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory_.get(),
base::BindOnce(&RequestImpl::OnSimpleLoaderComplete,
base::Unretained(this)));
}
void OnGetTokenFailure(const OAuth2TokenService::Request* request,
const GoogleServiceAuthError& error) override {
token_request_.reset();
is_pending_ = false;
UMA_HISTOGRAM_BOOLEAN("DesktopIOSPromotion.OAuthTokenCompletion", false);
callback_.Run(this, false);
// It is valid for the callback to delete |this|, so do not access any
// members below here.
} }
void SetPostData(const std::string& post_data) override { void SetPostData(const std::string& post_data) override {
...@@ -207,7 +211,7 @@ class RequestImpl : public SMSService::Request, ...@@ -207,7 +211,7 @@ class RequestImpl : public SMSService::Request,
OAuth2TokenService* token_service_; OAuth2TokenService* token_service_;
SigninManagerBase* signin_manager_; SigninManagerBase* signin_manager_;
scoped_refptr<net::URLRequestContextGetter> request_context_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
// The URL of the API endpoint. // The URL of the API endpoint.
GURL url_; GURL url_;
...@@ -225,7 +229,7 @@ class RequestImpl : public SMSService::Request, ...@@ -225,7 +229,7 @@ class RequestImpl : public SMSService::Request,
std::string access_token_; std::string access_token_;
// Handles the actual API requests after the OAuth token is acquired. // Handles the actual API requests after the OAuth token is acquired.
std::unique_ptr<net::URLFetcher> url_fetcher_; std::unique_ptr<network::SimpleURLLoader> simple_url_loader_;
// Holds the response code received from the server. // Holds the response code received from the server.
int response_code_; int response_code_;
...@@ -253,10 +257,10 @@ SMSService::Request::~Request() {} ...@@ -253,10 +257,10 @@ SMSService::Request::~Request() {}
SMSService::SMSService( SMSService::SMSService(
OAuth2TokenService* token_service, OAuth2TokenService* token_service,
SigninManagerBase* signin_manager, SigninManagerBase* signin_manager,
const scoped_refptr<net::URLRequestContextGetter>& request_context) scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: token_service_(token_service), : token_service_(token_service),
signin_manager_(signin_manager), signin_manager_(signin_manager),
request_context_(request_context), url_loader_factory_(std::move(url_loader_factory)),
weak_ptr_factory_(this) {} weak_ptr_factory_(this) {}
SMSService::~SMSService() {} SMSService::~SMSService() {}
...@@ -264,8 +268,8 @@ SMSService::~SMSService() {} ...@@ -264,8 +268,8 @@ SMSService::~SMSService() {}
SMSService::Request* SMSService::CreateRequest( SMSService::Request* SMSService::CreateRequest(
const GURL& url, const GURL& url,
const CompletionCallback& callback) { const CompletionCallback& callback) {
return new RequestImpl(token_service_, signin_manager_, request_context_, url, return new RequestImpl(token_service_, signin_manager_, url_loader_factory_,
callback); url, callback);
} }
void SMSService::QueryPhoneNumber(const PhoneNumberCallback& callback) { void SMSService::QueryPhoneNumber(const PhoneNumberCallback& callback) {
......
...@@ -19,8 +19,8 @@ ...@@ -19,8 +19,8 @@
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace net { namespace network {
class URLRequestContextGetter; class SharedURLLoaderFactory;
} }
class OAuth2TokenService; class OAuth2TokenService;
...@@ -61,10 +61,9 @@ class SMSService : public KeyedService { ...@@ -61,10 +61,9 @@ class SMSService : public KeyedService {
PhoneNumberCallback; PhoneNumberCallback;
typedef base::Callback<void(Request*, bool success)> CompletionCallback; typedef base::Callback<void(Request*, bool success)> CompletionCallback;
SMSService( SMSService(OAuth2TokenService* token_service,
OAuth2TokenService* token_service, SigninManagerBase* signin_manager,
SigninManagerBase* signin_manager, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
const scoped_refptr<net::URLRequestContextGetter>& request_context);
~SMSService() override; ~SMSService() override;
// Query the logged in user's verified phone number. // Query the logged in user's verified phone number.
...@@ -96,7 +95,7 @@ class SMSService : public KeyedService { ...@@ -96,7 +95,7 @@ class SMSService : public KeyedService {
SigninManagerBase* signin_manager_; SigninManagerBase* signin_manager_;
// Request context getter to use. // Request context getter to use.
scoped_refptr<net::URLRequestContextGetter> request_context_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
// Pending expiration requests to be canceled if not complete by profile // Pending expiration requests to be canceled if not complete by profile
// shutdown. // shutdown.
......
...@@ -12,7 +12,9 @@ ...@@ -12,7 +12,9 @@
#include "components/keyed_service/content/browser_context_dependency_manager.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/profile_oauth2_token_service.h"
#include "components/signin/core/browser/signin_manager.h" #include "components/signin/core/browser/signin_manager.h"
#include "net/url_request/url_request_context_getter.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
// static // static
SMSServiceFactory* SMSServiceFactory::GetInstance() { SMSServiceFactory* SMSServiceFactory::GetInstance() {
...@@ -31,7 +33,8 @@ KeyedService* SMSServiceFactory::BuildServiceInstanceFor( ...@@ -31,7 +33,8 @@ KeyedService* SMSServiceFactory::BuildServiceInstanceFor(
return new SMSService( return new SMSService(
ProfileOAuth2TokenServiceFactory::GetForProfile(profile), ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
SigninManagerFactory::GetForProfile(profile), SigninManagerFactory::GetForProfile(profile),
profile->GetRequestContext()); content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess());
} }
SMSServiceFactory::SMSServiceFactory() SMSServiceFactory::SMSServiceFactory()
......
...@@ -11,6 +11,9 @@ ...@@ -11,6 +11,9 @@
#include "components/signin/core/browser/fake_signin_manager.h" #include "components/signin/core/browser/fake_signin_manager.h"
#include "components/signin/core/browser/test_signin_client.h" #include "components/signin/core/browser/test_signin_client.h"
#include "net/http/http_status_code.h" #include "net/http/http_status_code.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/test/test_url_loader_factory.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"
...@@ -23,8 +26,8 @@ class TestingSMSService : public SMSService { ...@@ -23,8 +26,8 @@ class TestingSMSService : public SMSService {
explicit TestingSMSService( explicit TestingSMSService(
ProfileOAuth2TokenService* token_service, ProfileOAuth2TokenService* token_service,
SigninManagerBase* signin_manager, SigninManagerBase* signin_manager,
const scoped_refptr<net::URLRequestContextGetter>& request_context) scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: SMSService(token_service, signin_manager, request_context) {} : SMSService(token_service, signin_manager, url_loader_factory) {}
~TestingSMSService() override {} ~TestingSMSService() override {}
...@@ -125,9 +128,12 @@ class SMSServiceTest : public testing::Test { ...@@ -125,9 +128,12 @@ class SMSServiceTest : public testing::Test {
SMSServiceTest() SMSServiceTest()
: signin_client_(nullptr), : signin_client_(nullptr),
signin_manager_(&signin_client_, &account_tracker_), signin_manager_(&signin_client_, &account_tracker_),
url_request_context_(new net::TestURLRequestContextGetter( test_shared_loader_factory_(
base::ThreadTaskRunnerHandle::Get())), base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
sms_service_(&token_service_, &signin_manager_, url_request_context_) {} &test_url_loader_factory_)),
sms_service_(&token_service_,
&signin_manager_,
test_shared_loader_factory_) {}
~SMSServiceTest() override {} ~SMSServiceTest() override {}
...@@ -151,7 +157,8 @@ class SMSServiceTest : public testing::Test { ...@@ -151,7 +157,8 @@ class SMSServiceTest : public testing::Test {
AccountTrackerService account_tracker_; AccountTrackerService account_tracker_;
TestSigninClient signin_client_; TestSigninClient signin_client_;
FakeSigninManagerBase signin_manager_; FakeSigninManagerBase signin_manager_;
scoped_refptr<net::URLRequestContextGetter> url_request_context_; network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
TestingSMSService sms_service_; TestingSMSService sms_service_;
DISALLOW_COPY_AND_ASSIGN(SMSServiceTest); DISALLOW_COPY_AND_ASSIGN(SMSServiceTest);
......
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