Skip to content

Commit

Permalink
S13n: Port oauth2_api_call_flow to SimpleURLLoader
Browse files Browse the repository at this point in the history
URLFetcher will stop working in the browser process with
advent of Network Service, and SimpleURLLoader is the replacement
API for most clients.

Bug: 865015
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I248ff39534aeab11be4a60826edf7e89256ad2dc
Reviewed-on: https://chromium-review.googlesource.com/1138795
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581556}
  • Loading branch information
Maks Orlovich authored and Commit Bot committed Aug 8, 2018
1 parent d47866c commit 6246ea8
Show file tree
Hide file tree
Showing 41 changed files with 421 additions and 424 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ std::unique_ptr<cryptauth::CryptAuthClientFactory>
CreateCryptAuthClientFactoryImpl(Profile* profile) {
return std::make_unique<cryptauth::CryptAuthClientFactoryImpl>(
IdentityManagerFactory::GetForProfile(profile),
profile->GetRequestContext(),
profile->GetURLLoaderFactory(),
cryptauth::device_classifier_util::GetDeviceClassifier());
}

Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/chromeos/drive/drive_integration_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,9 @@ class DriveIntegrationService::DriveFsHolder

private:
// drivefs::DriveFsHost::Delegate:
net::URLRequestContextGetter* GetRequestContext() override {
return profile_->GetRequestContext();
scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory()
override {
return profile_->GetURLLoaderFactory();
}

service_manager::Connector* GetConnector() override {
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/extensions/api/identity/identity_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "services/identity/public/cpp/identity_manager.h"
#include "services/identity/public/cpp/identity_test_utils.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -169,7 +170,7 @@ class TestHangOAuth2MintTokenFlow : public OAuth2MintTokenFlow {
TestHangOAuth2MintTokenFlow()
: OAuth2MintTokenFlow(NULL, OAuth2MintTokenFlow::Parameters()) {}

void Start(net::URLRequestContextGetter* context,
void Start(scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const std::string& access_token) override {
// Do nothing, simulating a hanging network call.
}
Expand All @@ -191,7 +192,7 @@ class TestOAuth2MintTokenFlow : public OAuth2MintTokenFlow {
result_(result),
delegate_(delegate) {}

void Start(net::URLRequestContextGetter* context,
void Start(scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const std::string& access_token) override {
switch (result_) {
case ISSUE_ADVICE_SUCCESS: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ void IdentityGetAuthTokenFunction::StartGaiaRequest(
const std::string& login_access_token) {
DCHECK(!login_access_token.empty());
mint_token_flow_.reset(CreateMintTokenFlow());
mint_token_flow_->Start(GetProfile()->GetRequestContext(),
mint_token_flow_->Start(GetProfile()->GetURLLoaderFactory(),
login_access_token);
}

Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/profiles/profile_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,8 @@ ProfileImpl::CreateDeviceSyncService() {
return std::make_unique<chromeos::device_sync::DeviceSyncService>(
IdentityManagerFactory::GetForProfile(this),
gcm::GCMProfileServiceFactory::GetForProfile(this)->driver(),
chromeos::GcmDeviceInfoProviderImpl::GetInstance(), GetRequestContext());
chromeos::GcmDeviceInfoProviderImpl::GetInstance(),
GetURLLoaderFactory());
}

std::unique_ptr<service_manager::Service>
Expand Down
2 changes: 2 additions & 0 deletions chromeos/components/drivefs/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ component("drivefs") {
"//mojo/public/cpp/platform",
"//net",
"//services/identity/public/mojom",
"//services/network/public/cpp:cpp",
"//services/service_manager/public/cpp",
]
defines = [ "IS_DRIVEFS_IMPL" ]
Expand Down Expand Up @@ -63,6 +64,7 @@ source_set("unit_tests") {
"//mojo/public/cpp/bindings",
"//net",
"//services/identity/public/mojom",
"//services/network/public/cpp:cpp",
"//services/service_manager/public/cpp",
"//services/service_manager/public/cpp/test:test_support",
"//testing/gmock",
Expand Down
3 changes: 2 additions & 1 deletion chromeos/components/drivefs/drivefs_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "mojo/public/cpp/platform/platform_channel_endpoint.h"
#include "mojo/public/cpp/system/invitation.h"
#include "services/identity/public/mojom/constants.mojom.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/service_manager/public/cpp/connector.h"

namespace drivefs {
Expand Down Expand Up @@ -262,7 +263,7 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
OnMintTokenFailure(error);
return;
}
mint_token_flow_->Start(host_->delegate_->GetRequestContext(),
mint_token_flow_->Start(host_->delegate_->GetURLLoaderFactory(),
*access_token);
}

Expand Down
9 changes: 5 additions & 4 deletions chromeos/components/drivefs/drivefs_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
#include "google_apis/gaia/oauth2_mint_token_flow.h"
#include "services/identity/public/mojom/identity_manager.mojom.h"

namespace net {
class URLRequestContextGetter;
} // namespace net
namespace network {
class SharedURLLoaderFactory;
} // namespace network

namespace service_manager {
class Connector;
Expand Down Expand Up @@ -57,7 +57,8 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsHost
Delegate() = default;
virtual ~Delegate() = default;

virtual net::URLRequestContextGetter* GetRequestContext() = 0;
virtual scoped_refptr<network::SharedURLLoaderFactory>
GetURLLoaderFactory() = 0;
virtual service_manager::Connector* GetConnector() = 0;
virtual const AccountId& GetAccountId() = 0;
virtual std::string GetObfuscatedAccountId() = 0;
Expand Down
10 changes: 7 additions & 3 deletions chromeos/components/drivefs/drivefs_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "chromeos/disks/mock_disk_mount_manager.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "services/service_manager/public/cpp/connector.h"
#include "services/service_manager/public/cpp/service.h"
Expand Down Expand Up @@ -94,9 +95,9 @@ class ForwardingOAuth2MintTokenFlow : public OAuth2MintTokenFlow {
MockOAuth2MintTokenFlow* mock)
: OAuth2MintTokenFlow(delegate, {}), delegate_(delegate), mock_(mock) {}

void Start(net::URLRequestContextGetter* context_getter,
void Start(scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const std::string& access_token) override {
EXPECT_EQ(nullptr, context_getter);
EXPECT_EQ(nullptr, url_loader_factory);
mock_->Start(delegate_, access_token);
}

Expand Down Expand Up @@ -125,7 +126,10 @@ class TestingDriveFsHostDelegate : public DriveFsHost::Delegate {

private:
// DriveFsHost::Delegate:
net::URLRequestContextGetter* GetRequestContext() override { return nullptr; }
scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory()
override {
return nullptr;
}
service_manager::Connector* GetConnector() override {
return connector_.get();
}
Expand Down
1 change: 1 addition & 0 deletions chromeos/services/device_sync/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ source_set("unit_tests") {
"//components/gcm_driver:test_support",
"//components/prefs:test_support",
"//services/identity/public/cpp:test_support",
"//services/network:test_support",
"//services/service_manager/public/cpp:service_test_support",
"//services/service_manager/public/cpp/test:test_support",
"//testing/gtest",
Expand Down
18 changes: 9 additions & 9 deletions chromeos/services/device_sync/device_sync_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
#include "components/cryptauth/software_feature_manager_impl.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "net/url_request/url_request_context_getter.h"
#include "services/identity/public/cpp/identity_manager.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/service_manager/public/cpp/connector.h"

namespace chromeos {
Expand All @@ -51,17 +51,17 @@ std::unique_ptr<DeviceSyncBase> DeviceSyncImpl::Factory::NewInstance(
gcm::GCMDriver* gcm_driver,
service_manager::Connector* connector,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context) {
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
if (test_factory_instance_) {
return test_factory_instance_->BuildInstance(
identity_manager, gcm_driver, connector, gcm_device_info_provider,
std::move(url_request_context));
std::move(url_loader_factory));
}

static base::NoDestructor<DeviceSyncImpl::Factory> default_factory;
return default_factory->BuildInstance(identity_manager, gcm_driver, connector,
gcm_device_info_provider,
std::move(url_request_context));
std::move(url_loader_factory));
}

// static
Expand All @@ -76,10 +76,10 @@ std::unique_ptr<DeviceSyncBase> DeviceSyncImpl::Factory::BuildInstance(
gcm::GCMDriver* gcm_driver,
service_manager::Connector* connector,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context) {
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
return base::WrapUnique(new DeviceSyncImpl(
identity_manager, gcm_driver, connector, gcm_device_info_provider,
std::move(url_request_context), base::DefaultClock::GetInstance(),
std::move(url_loader_factory), base::DefaultClock::GetInstance(),
std::make_unique<PrefConnectionDelegate>()));
}

Expand All @@ -105,14 +105,14 @@ DeviceSyncImpl::DeviceSyncImpl(
gcm::GCMDriver* gcm_driver,
service_manager::Connector* connector,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
base::Clock* clock,
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate)
: identity_manager_(identity_manager),
gcm_driver_(gcm_driver),
connector_(connector),
gcm_device_info_provider_(gcm_device_info_provider),
url_request_context_(url_request_context),
url_loader_factory_(std::move(url_loader_factory)),
clock_(clock),
pref_connection_delegate_(std::move(pref_connection_delegate)),
status_(Status::FETCHING_ACCOUNT_INFO),
Expand Down Expand Up @@ -329,7 +329,7 @@ void DeviceSyncImpl::InitializeCryptAuthManagementObjects() {

cryptauth_client_factory_ =
std::make_unique<cryptauth::CryptAuthClientFactoryImpl>(
identity_manager_, url_request_context_,
identity_manager_, url_loader_factory_,
cryptauth::device_classifier_util::GetDeviceClassifier());

// Initialize |crypauth_device_manager_| and start observing. Start() is not
Expand Down
14 changes: 7 additions & 7 deletions chromeos/services/device_sync/device_sync_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ namespace identity {
class IdentityManager;
} // namespace identity

namespace net {
class URLRequestContextGetter;
} // namespace net
namespace network {
class SharedURLLoaderFactory;
} // namespace network

namespace service_manager {
class Connector;
Expand Down Expand Up @@ -69,7 +69,7 @@ class DeviceSyncImpl : public DeviceSyncBase,
gcm::GCMDriver* gcm_driver,
service_manager::Connector* connector,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context);
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
static void SetInstanceForTesting(Factory* test_factory);

virtual ~Factory();
Expand All @@ -78,7 +78,7 @@ class DeviceSyncImpl : public DeviceSyncBase,
gcm::GCMDriver* gcm_driver,
service_manager::Connector* connector,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context);
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);

private:
static Factory* test_factory_instance_;
Expand Down Expand Up @@ -137,7 +137,7 @@ class DeviceSyncImpl : public DeviceSyncBase,
gcm::GCMDriver* gcm_driver,
service_manager::Connector* connector,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
base::Clock* clock,
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);

Expand Down Expand Up @@ -176,7 +176,7 @@ class DeviceSyncImpl : public DeviceSyncBase,
gcm::GCMDriver* gcm_driver_;
service_manager::Connector* connector_;
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider_;
scoped_refptr<net::URLRequestContextGetter> url_request_context_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
base::Clock* clock_;
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate_;

Expand Down
8 changes: 4 additions & 4 deletions chromeos/services/device_sync/device_sync_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "chromeos/components/proximity_auth/logging/logging.h"
#include "chromeos/services/device_sync/device_sync_base.h"
#include "chromeos/services/device_sync/device_sync_impl.h"
#include "net/url_request/url_request_context_getter.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/service_manager/public/cpp/service_context.h"

namespace chromeos {
Expand All @@ -18,11 +18,11 @@ DeviceSyncService::DeviceSyncService(
identity::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context)
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: identity_manager_(identity_manager),
gcm_driver_(gcm_driver),
gcm_device_info_provider_(gcm_device_info_provider),
url_request_context_(url_request_context) {}
url_loader_factory_(std::move(url_loader_factory)) {}

DeviceSyncService::~DeviceSyncService() = default;

Expand All @@ -33,7 +33,7 @@ void DeviceSyncService::OnStart() {
// |device_sync_impl_| cannot be initialized until OnStart().
device_sync_ = DeviceSyncImpl::Factory::NewInstance(
identity_manager_, gcm_driver_, context()->connector(),
gcm_device_info_provider_, url_request_context_);
gcm_device_info_provider_, url_loader_factory_);

registry_.AddInterface(base::Bind(&DeviceSyncBase::BindRequest,
base::Unretained(device_sync_.get())));
Expand Down
10 changes: 5 additions & 5 deletions chromeos/services/device_sync/device_sync_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ namespace identity {
class IdentityManager;
} // namespace identity

namespace net {
class URLRequestContextGetter;
} // namespace net
namespace network {
class SharedURLLoaderFactory;
} // namespace network

namespace chromeos {

Expand All @@ -43,7 +43,7 @@ class DeviceSyncService : public service_manager::Service {
identity::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider,
scoped_refptr<net::URLRequestContextGetter> url_request_context);
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
~DeviceSyncService() override;

protected:
Expand All @@ -57,7 +57,7 @@ class DeviceSyncService : public service_manager::Service {
identity::IdentityManager* identity_manager_;
gcm::GCMDriver* gcm_driver_;
const cryptauth::GcmDeviceInfoProvider* gcm_device_info_provider_;
scoped_refptr<net::URLRequestContextGetter> url_request_context_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;

std::unique_ptr<DeviceSyncBase> device_sync_;
service_manager::BinderRegistry registry_;
Expand Down
Loading

0 comments on commit 6246ea8

Please sign in to comment.