diff --git a/chrome/browser/android/signin/signin_manager_android.cc b/chrome/browser/android/signin/signin_manager_android.cc index 0ff10d2a76ab69..92b751f51f62f2 100644 --- a/chrome/browser/android/signin/signin_manager_android.cc +++ b/chrome/browser/android/signin/signin_manager_android.cc @@ -44,6 +44,7 @@ #include "components/signin/core/browser/signin_pref_names.h" #include "content/public/browser/browsing_data_filter_builder.h" #include "content/public/browser/browsing_data_remover.h" +#include "content/public/browser/storage_partition.h" #include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/gaia_constants.h" #include "jni/SigninManager_jni.h" @@ -172,11 +173,15 @@ void SigninManagerAndroid::FetchPolicyBeforeSignIn( if (!dm_token_.empty()) { policy::UserPolicySigninService* service = policy::UserPolicySigninServiceFactory::GetForProfile(profile_); + scoped_refptr url_loader_factory = + content::BrowserContext::GetDefaultStoragePartition(profile_) + ->GetURLLoaderFactoryForBrowserProcess(); service->FetchPolicyForSignedInUser( AccountTrackerServiceFactory::GetForProfile(profile_) ->FindAccountInfoByEmail(username_) .GetAccountId(), dm_token_, client_id_, profile_->GetRequestContext(), + url_loader_factory, base::Bind(&SigninManagerAndroid::OnPolicyFetchDone, weak_factory_.GetWeakPtr())); dm_token_.clear(); diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index 8c98e72db60b66..a02e15ba60a2a2 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -1130,7 +1130,9 @@ void BrowserProcessImpl::PreMainMessageLoopRun() { // requires that threads are running; this Init() call lets the connector // resume its initialization now that the loops are spinning and the // system request context is available for the fetchers. - browser_policy_connector()->Init(local_state(), system_request_context()); + browser_policy_connector()->Init( + local_state(), system_request_context(), + system_network_context_manager()->GetSharedURLLoaderFactory()); if (local_state_->IsManagedPreference(prefs::kDefaultBrowserSettingEnabled)) ApplyDefaultBrowserPolicy(); diff --git a/chrome/browser/chromeos/drive/drive_integration_service.cc b/chrome/browser/chromeos/drive/drive_integration_service.cc index 54e1c1fddee010..0c4ddc482c910b 100644 --- a/chrome/browser/chromeos/drive/drive_integration_service.cc +++ b/chrome/browser/chromeos/drive/drive_integration_service.cc @@ -23,6 +23,7 @@ #include "chrome/browser/download/download_core_service_factory.h" #include "chrome/browser/download/download_prefs.h" #include "chrome/browser/drive/drive_notification_manager_factory.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/profiles/incognito_helpers.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" @@ -56,6 +57,7 @@ #include "net/traffic_annotation/network_traffic_annotation.h" #include "services/device/public/mojom/constants.mojom.h" #include "services/device/public/mojom/wake_lock_provider.mojom.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/service_manager/public/cpp/connector.h" #include "storage/browser/fileapi/external_mount_points.h" #include "ui/base/l10n/l10n_util.h" @@ -362,9 +364,15 @@ DriveIntegrationService::DriveIntegrationService( if (test_drive_service) { drive_service_.reset(test_drive_service); } else { + scoped_refptr url_loader_factory; + if (g_browser_process->system_network_context_manager()) { + // system_network_context_manager() returns nullptr in unit-tests. + url_loader_factory = g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(); + } drive_service_.reset(new DriveAPIService( oauth_service, g_browser_process->system_request_context(), - blocking_task_runner_.get(), + url_loader_factory, blocking_task_runner_.get(), GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction), GURL(google_apis::DriveApiUrlGenerator::kBaseThumbnailUrlForProduction), GetDriveUserAgent(), NO_TRAFFIC_ANNOTATION_YET)); diff --git a/chrome/browser/chromeos/extensions/file_manager/private_api_drive.cc b/chrome/browser/chromeos/extensions/file_manager/private_api_drive.cc index d8d74ec41ed1f6..6ef652b86b56d2 100644 --- a/chrome/browser/chromeos/extensions/file_manager/private_api_drive.cc +++ b/chrome/browser/chromeos/extensions/file_manager/private_api_drive.cc @@ -26,6 +26,7 @@ #include "chrome/browser/chromeos/fileapi/external_file_url_util.h" #include "chrome/browser/chromeos/fileapi/file_system_backend.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" @@ -39,10 +40,12 @@ #include "components/signin/core/browser/profile_oauth2_token_service.h" #include "components/signin/core/browser/signin_manager.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/storage_partition.h" #include "google_apis/drive/auth_service.h" #include "google_apis/drive/drive_api_url_generator.h" #include "google_apis/drive/drive_switches.h" #include "mojo/public/cpp/bindings/callback_helpers.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "storage/common/fileapi/file_system_info.h" #include "storage/common/fileapi/file_system_util.h" #include "url/gurl.h" @@ -1522,9 +1525,12 @@ void FileManagerPrivateInternalGetDownloadUrlFunction::OnGotDownloadUrl( std::vector scopes; scopes.emplace_back("https://www.googleapis.com/auth/drive.readonly"); + scoped_refptr url_loader_factory = + content::BrowserContext::GetDefaultStoragePartition(GetProfile()) + ->GetURLLoaderFactoryForBrowserProcess(); auth_service_ = std::make_unique( oauth2_token_service, account_id, GetProfile()->GetRequestContext(), - scopes); + url_loader_factory, scopes); auth_service_->StartAuthentication(base::Bind( &FileManagerPrivateInternalGetDownloadUrlFunction::OnTokenFetched, this)); } diff --git a/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc b/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc index 743c1ed29c2c0e..0273e04c0324b6 100644 --- a/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc +++ b/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc @@ -35,6 +35,7 @@ #include "chrome/browser/devtools/devtools_window.h" #include "chrome/browser/extensions/devtools_util.h" #include "chrome/browser/lifetime/application_lifetime.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profiles_state.h" @@ -63,6 +64,7 @@ #include "extensions/browser/app_window/app_window.h" #include "extensions/browser/app_window/app_window_registry.h" #include "google_apis/drive/auth_service.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "storage/browser/fileapi/external_mount_points.h" #include "storage/common/fileapi/file_system_types.h" #include "ui/base/webui/web_ui_util.h" @@ -374,7 +376,10 @@ bool FileManagerPrivateRequestWebStoreAccessTokenFunction::RunAsync() { SigninManagerFactory::GetForProfile(GetProfile()); auth_service_ = std::make_unique( oauth_service, signin_manager->GetAuthenticatedAccountId(), - url_request_context_getter, scopes); + url_request_context_getter, + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(), + scopes); auth_service_->StartAuthentication(base::Bind( &FileManagerPrivateRequestWebStoreAccessTokenFunction:: OnAccessTokenFetched, diff --git a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc index 1d10ef18c3d92f..a404bd166d7095 100644 --- a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc +++ b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc @@ -20,12 +20,14 @@ #include "chrome/browser/chromeos/policy/enrollment_status_chromeos.h" #include "chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chromeos/chromeos_switches.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "components/policy/core/common/cloud/cloud_policy_constants.h" #include "google_apis/gaia/gaia_auth_consumer.h" #include "google_apis/gaia/gaia_auth_fetcher.h" #include "google_apis/gaia/gaia_constants.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace { @@ -98,6 +100,8 @@ void EnterpriseEnrollmentHelperImpl::EnrollUsingAuthCode( oauth_fetcher_.reset(policy::PolicyOAuth2TokenFetcher::CreateInstance()); oauth_fetcher_->StartWithAuthCode( auth_code, g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(), base::Bind(&EnterpriseEnrollmentHelperImpl::OnTokenFetched, weak_ptr_factory_.GetWeakPtr(), fetch_additional_token /* is_additional_token */)); @@ -267,6 +271,8 @@ void EnterpriseEnrollmentHelperImpl::OnTokenFetched( oauth_fetcher_.reset(policy::PolicyOAuth2TokenFetcher::CreateInstance()); oauth_fetcher_->StartWithRefreshToken( refresh_token, g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(), base::Bind(&EnterpriseEnrollmentHelperImpl::OnTokenFetched, weak_ptr_factory_.GetWeakPtr(), false /* is_additional_token */)); diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc index 8fbd0123d4862b..d558eab36be2c7 100644 --- a/chrome/browser/chromeos/login/existing_user_controller.cc +++ b/chrome/browser/chromeos/login/existing_user_controller.cc @@ -51,6 +51,7 @@ #include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/settings/cros_settings.h" #include "chrome/browser/chromeos/system/device_disabling_manager.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/aura/accessibility/automation_manager_aura.h" #include "chrome/browser/ui/webui/chromeos/login/l10n_util.h" @@ -98,6 +99,7 @@ #include "net/http/http_transaction_factory.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "ui/accessibility/ax_enums.mojom.h" #include "ui/base/l10n/l10n_util.h" #include "ui/views/widget/widget.h" @@ -1067,10 +1069,17 @@ void ExistingUserController::OnOldEncryptionDetected( // Use signin profile request context net::URLRequestContextGetter* const signin_profile_context = ProfileHelper::GetSigninProfile()->GetRequestContext(); + scoped_refptr + sigin_profile_url_loader_factory = + content::BrowserContext::GetDefaultStoragePartition( + ProfileHelper::GetSigninProfile()) + ->GetURLLoaderFactoryForBrowserProcess(); + auto cloud_policy_client = std::make_unique( std::string() /* machine_id */, std::string() /* machine_model */, std::string() /* brand_code */, device_management_service, - signin_profile_context, nullptr /* signing_service */, + signin_profile_context, sigin_profile_url_loader_factory, + nullptr /* signing_service */, chromeos::GetDeviceDMTokenForUserPolicyGetter( user_context.GetAccountId())); pre_signin_policy_fetcher_ = std::make_unique( diff --git a/chrome/browser/chromeos/oauth2_token_service_delegate.cc b/chrome/browser/chromeos/oauth2_token_service_delegate.cc index 6bc35a6396fdb9..a77b672e923ec8 100644 --- a/chrome/browser/chromeos/oauth2_token_service_delegate.cc +++ b/chrome/browser/chromeos/oauth2_token_service_delegate.cc @@ -11,6 +11,7 @@ #include "base/logging.h" #include "chromeos/account_manager/account_manager.h" #include "components/signin/core/browser/account_tracker_service.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace chromeos { @@ -30,6 +31,7 @@ OAuth2AccessTokenFetcher* ChromeOSOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_EQ(LOAD_CREDENTIALS_FINISHED_WITH_SUCCESS, load_credentials_state_); @@ -41,7 +43,7 @@ ChromeOSOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( // |OAuth2TokenService| will manage the lifetime of the released pointer. return account_manager_ - ->CreateAccessTokenFetcher(account_key, getter, consumer) + ->CreateAccessTokenFetcher(account_key, url_loader_factory, consumer) .release(); } diff --git a/chrome/browser/chromeos/oauth2_token_service_delegate.h b/chrome/browser/chromeos/oauth2_token_service_delegate.h index aeb0d1f9211c0b..6b9bcfd5f0a1e4 100644 --- a/chrome/browser/chromeos/oauth2_token_service_delegate.h +++ b/chrome/browser/chromeos/oauth2_token_service_delegate.h @@ -36,6 +36,7 @@ class ChromeOSOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate, OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) override; bool RefreshTokenIsAvailable(const std::string& account_id) const override; void UpdateAuthError(const std::string& account_id, diff --git a/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl_unittest.cc b/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl_unittest.cc index 4401c148c39a0f..bd309759d743b4 100644 --- a/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl_unittest.cc +++ b/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl_unittest.cc @@ -10,6 +10,7 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" +#include "base/memory/ref_counted.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h" #include "chrome/browser/chromeos/settings/cros_settings.h" @@ -35,6 +36,8 @@ #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_service.h" #include "content/public/test/test_browser_thread_bundle.h" +#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" +#include "services/network/test/test_url_loader_factory.h" #include "testing/gtest/include/gtest/gtest.h" namespace policy { @@ -137,6 +140,9 @@ class AffiliatedInvalidationServiceProviderImplTest : public testing::Test { std::unique_ptr test_device_settings_service_; std::unique_ptr test_cros_settings_; + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr + test_shared_loader_factory_; TestingProfileManager profile_manager_; }; @@ -197,6 +203,9 @@ AffiliatedInvalidationServiceProviderImplTest:: chromeos::ScopedStubInstallAttributes::CreateCloudManaged( "example.com", "device_id")), + test_shared_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)), profile_manager_(TestingBrowserProcess::GetGlobal()) {} void AffiliatedInvalidationServiceProviderImplTest::SetUp() { @@ -207,7 +216,8 @@ void AffiliatedInvalidationServiceProviderImplTest::SetUp() { test_device_settings_service_.reset(new chromeos::ScopedTestDeviceSettingsService); test_cros_settings_.reset(new chromeos::ScopedTestCrosSettings); - chromeos::DeviceOAuth2TokenServiceFactory::Initialize(); + chromeos::DeviceOAuth2TokenServiceFactory::Initialize( + test_shared_loader_factory_); invalidation::ProfileInvalidationProviderFactory::GetInstance()-> RegisterTestingFactory(BuildProfileInvalidationProvider); @@ -219,6 +229,7 @@ void AffiliatedInvalidationServiceProviderImplTest::TearDown() { consumer_.reset(); provider_->Shutdown(); provider_.reset(); + test_shared_loader_factory_->Detach(); invalidation::ProfileInvalidationProviderFactory::GetInstance()-> RegisterTestingFactory(nullptr); diff --git a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc index 6b26d839e0ba9b..c41c3011e74c0d 100644 --- a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc +++ b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc @@ -63,6 +63,7 @@ #include "content/public/browser/browser_thread.h" #include "google_apis/gaia/gaia_auth_util.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" using content::BrowserThread; @@ -147,9 +148,11 @@ BrowserPolicyConnectorChromeOS::~BrowserPolicyConnectorChromeOS() {} void BrowserPolicyConnectorChromeOS::Init( PrefService* local_state, - scoped_refptr request_context) { + scoped_refptr request_context, + scoped_refptr url_loader_factory) { local_state_ = local_state; - ChromeBrowserPolicyConnector::Init(local_state, request_context); + ChromeBrowserPolicyConnector::Init(local_state, request_context, + url_loader_factory); affiliated_invalidation_service_provider_ = std::make_unique(); @@ -177,7 +180,7 @@ void BrowserPolicyConnectorChromeOS::Init( GetBackgroundTaskRunner(), content::BrowserThread::GetTaskRunnerForThread( content::BrowserThread::IO), - request_context); + request_context, url_loader_factory); device_local_account_policy_service_->Connect(device_management_service()); } diff --git a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h index 4cd9bc30ac0fe5..ec9d371e9fc555 100644 --- a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h +++ b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h @@ -30,10 +30,6 @@ class AttestationFlow; } // namespace chromeos -namespace net { -class URLRequestContextGetter; -} - namespace policy { class AffiliatedCloudPolicyInvalidator; @@ -60,9 +56,10 @@ class BrowserPolicyConnectorChromeOS ~BrowserPolicyConnectorChromeOS() override; // ChromeBrowserPolicyConnector: - void Init( - PrefService* local_state, - scoped_refptr request_context) override; + void Init(PrefService* local_state, + scoped_refptr request_context, + scoped_refptr url_loader_factory) + override; // Checks whether this devices is under any kind of enterprise management. bool IsEnterpriseManaged() const override; diff --git a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc index d857b0a9855a47..a0744625acd6fd 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc @@ -191,7 +191,8 @@ void CloudExternalDataPolicyObserverTest::SetUp() { base::ThreadTaskRunnerHandle::Get(), base::ThreadTaskRunnerHandle::Get(), base::ThreadTaskRunnerHandle::Get(), - base::ThreadTaskRunnerHandle::Get(), nullptr)); + base::ThreadTaskRunnerHandle::Get(), /*request_context=*/nullptr, + /*url_loader_factory=*/nullptr)); url_fetcher_factory_.set_remove_fetcher_on_delete(true); EXPECT_CALL(user_policy_provider_, IsInitializationComplete(_)) diff --git a/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc b/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc index 30b6ca0ef45f33..e87f253ca51a95 100644 --- a/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc +++ b/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc @@ -23,6 +23,7 @@ #include "chrome/browser/chromeos/policy/enrollment_status_chromeos.h" #include "chrome/browser/chromeos/policy/server_backed_device_state.h" #include "chrome/browser/chromeos/settings/install_attributes.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/common/chrome_content_client.h" #include "chrome/common/pref_names.h" #include "chromeos/attestation/attestation.pb.h" @@ -70,6 +71,11 @@ void DeviceCloudPolicyInitializer::SetSigningServiceForTesting( signing_service_ = std::move(signing_service); } +void DeviceCloudPolicyInitializer::SetSystemURLLoaderFactoryForTesting( + scoped_refptr system_url_loader_factory) { + system_url_loader_factory_for_testing_ = system_url_loader_factory; +} + DeviceCloudPolicyInitializer::~DeviceCloudPolicyInitializer() { DCHECK(!is_initialized_); } @@ -297,6 +303,10 @@ std::unique_ptr DeviceCloudPolicyInitializer::CreateClient( return std::make_unique( statistics_provider_->GetEnterpriseMachineID(), machine_model, brand_code, device_management_service, g_browser_process->system_request_context(), + system_url_loader_factory_for_testing_ + ? system_url_loader_factory_for_testing_ + : g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(), signing_service_.get(), CloudPolicyClient::DeviceDMTokenCallback()); } diff --git a/chrome/browser/chromeos/policy/device_cloud_policy_initializer.h b/chrome/browser/chromeos/policy/device_cloud_policy_initializer.h index c09b3ae76ba5d2..b09df8ebac75fe 100644 --- a/chrome/browser/chromeos/policy/device_cloud_policy_initializer.h +++ b/chrome/browser/chromeos/policy/device_cloud_policy_initializer.h @@ -19,6 +19,7 @@ #include "components/policy/core/common/cloud/cloud_policy_constants.h" #include "components/policy/core/common/cloud/cloud_policy_store.h" #include "components/policy/core/common/cloud/signing_service.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" class PrefService; @@ -121,6 +122,8 @@ class DeviceCloudPolicyInitializer : public CloudPolicyStore::Observer { // Allows testing code to set a signing service tailored to its needs. void SetSigningServiceForTesting( std::unique_ptr signing_service); + void SetSystemURLLoaderFactoryForTesting( + scoped_refptr system_url_loader_factory); private: // Signing class implementing the policy::SigningService interface to @@ -180,6 +183,10 @@ class DeviceCloudPolicyInitializer : public CloudPolicyStore::Observer { // Our signing service. std::unique_ptr signing_service_; + // The URLLoaderFactory set in tests. + scoped_refptr + system_url_loader_factory_for_testing_; + DISALLOW_COPY_AND_ASSIGN(DeviceCloudPolicyInitializer); }; diff --git a/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc index 8ecf4c63f7083f..acf27f5246adf9 100644 --- a/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc +++ b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc @@ -16,6 +16,7 @@ #include "base/compiler_specific.h" #include "base/macros.h" #include "base/memory/ptr_util.h" +#include "base/memory/ref_counted.h" #include "base/run_loop.h" #include "base/threading/thread_task_runner_handle.h" #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h" @@ -59,6 +60,8 @@ #include "google_apis/gaia/gaia_oauth_client.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_request_test_util.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/gtest/include/gtest/gtest.h" @@ -118,7 +121,10 @@ class DeviceCloudPolicyManagerChromeOSTest DeviceCloudPolicyManagerChromeOSTest() : fake_cryptohome_client_(new chromeos::FakeCryptohomeClient()), state_keys_broker_(&fake_session_manager_client_), - store_(NULL) { + store_(nullptr), + test_shared_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)) { fake_statistics_provider_.SetMachineStatistic( chromeos::system::kSerialNumberKeyForTest, "test_sn"); fake_statistics_provider_.SetMachineStatistic( @@ -165,7 +171,9 @@ class DeviceCloudPolicyManagerChromeOSTest TestingBrowserProcess::GetGlobal()->SetLocalState(&local_state_); // SystemSaltGetter is used in DeviceOAuth2TokenService. chromeos::SystemSaltGetter::Initialize(); - chromeos::DeviceOAuth2TokenServiceFactory::Initialize(); + chromeos::DeviceOAuth2TokenServiceFactory::Initialize( + test_shared_loader_factory_); + url_fetcher_response_code_ = 200; url_fetcher_response_string_ = "{\"access_token\":\"accessToken4Test\"," "\"expires_in\":1234," @@ -196,6 +204,7 @@ class DeviceCloudPolicyManagerChromeOSTest chromeos::DeviceOAuth2TokenServiceFactory::Shutdown(); chromeos::SystemSaltGetter::Shutdown(); TestingBrowserProcess::GetGlobal()->SetLocalState(NULL); + test_shared_loader_factory_->Detach(); } void LockDevice() { @@ -224,6 +233,8 @@ class DeviceCloudPolicyManagerChromeOSTest &fake_statistics_provider_); initializer_->SetSigningServiceForTesting( std::make_unique()); + initializer_->SetSystemURLLoaderFactoryForTesting( + test_shared_loader_factory_); initializer_->Init(); } @@ -275,6 +286,10 @@ class DeviceCloudPolicyManagerChromeOSTest std::unique_ptr initializer_; private: + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr + test_shared_loader_factory_; + DISALLOW_COPY_AND_ASSIGN(DeviceCloudPolicyManagerChromeOSTest); }; diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_service.cc b/chrome/browser/chromeos/policy/device_local_account_policy_service.cc index 629bd10ee33348..0091e48f505ecc 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_service.cc +++ b/chrome/browser/chromeos/policy/device_local_account_policy_service.cc @@ -64,7 +64,8 @@ std::string GetDeviceDMToken( std::unique_ptr CreateClient( chromeos::DeviceSettingsService* device_settings_service, DeviceManagementService* device_management_service, - scoped_refptr system_request_context) { + scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory) { const em::PolicyData* policy_data = device_settings_service->policy_data(); if (!policy_data || !policy_data->has_request_token() || @@ -77,7 +78,8 @@ std::unique_ptr CreateClient( std::make_unique( std::string() /* machine_id */, std::string() /* machine_model */, std::string() /* brand_code */, device_management_service, - system_request_context, nullptr /* signing_service */, + system_request_context, system_url_loader_factory, + nullptr /* signing_service */, base::BindRepeating(&GetDeviceDMToken, device_settings_service)); std::vector user_affiliation_ids( policy_data->user_affiliation_ids().begin(), @@ -188,12 +190,14 @@ bool DeviceLocalAccountPolicyBroker::HasInvalidatorForTest() const { void DeviceLocalAccountPolicyBroker::ConnectIfPossible( chromeos::DeviceSettingsService* device_settings_service, DeviceManagementService* device_management_service, - scoped_refptr request_context) { + scoped_refptr request_context, + scoped_refptr url_loader_factory) { if (core_.client()) return; - std::unique_ptr client(CreateClient( - device_settings_service, device_management_service, request_context)); + std::unique_ptr client( + CreateClient(device_settings_service, device_management_service, + request_context, url_loader_factory)); if (!client) return; @@ -270,7 +274,8 @@ DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService( scoped_refptr external_data_service_backend_task_runner, scoped_refptr io_task_runner, - scoped_refptr request_context) + scoped_refptr request_context, + scoped_refptr url_loader_factory) : session_manager_client_(session_manager_client), device_settings_service_(device_settings_service), cros_settings_(cros_settings), @@ -283,6 +288,7 @@ DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService( resource_cache_task_runner_(base::CreateSequencedTaskRunnerWithTraits( {base::MayBlock(), base::TaskPriority::BACKGROUND})), request_context_(request_context), + url_loader_factory_(url_loader_factory), local_accounts_subscription_(cros_settings_->AddSettingsObserver( chromeos::kAccountsPrefDeviceLocalAccounts, base::Bind( @@ -319,8 +325,8 @@ void DeviceLocalAccountPolicyService::Connect( for (PolicyBrokerMap::iterator it(policy_brokers_.begin()); it != policy_brokers_.end(); ++it) { it->second->ConnectIfPossible(device_settings_service_, - device_management_service_, - request_context_); + device_management_service_, request_context_, + url_loader_factory_); } } @@ -493,8 +499,8 @@ void DeviceLocalAccountPolicyService::UpdateAccountList() { // Fire up the cloud connection for fetching policy for the account from // the cloud if this is an enterprise-managed device. broker->ConnectIfPossible(device_settings_service_, - device_management_service_, - request_context_); + device_management_service_, request_context_, + url_loader_factory_); policy_brokers_[it->user_id] = broker.release(); if (!broker_initialized) { diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_service.h b/chrome/browser/chromeos/policy/device_local_account_policy_service.h index 3646b595e54549..2db069f06b37d7 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_service.h +++ b/chrome/browser/chromeos/policy/device_local_account_policy_service.h @@ -25,6 +25,7 @@ #include "components/policy/core/common/cloud/cloud_policy_store.h" #include "components/policy/core/common/cloud/component_cloud_policy_service.h" #include "components/policy/core/common/schema_registry.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace base { class SequencedTaskRunner; @@ -107,7 +108,8 @@ class DeviceLocalAccountPolicyBroker void ConnectIfPossible( chromeos::DeviceSettingsService* device_settings_service, DeviceManagementService* device_management_service, - scoped_refptr request_context); + scoped_refptr request_context, + scoped_refptr url_loader_factory); // Reads the refresh delay from policy and configures the refresh scheduler. void UpdateRefreshDelay(); @@ -175,7 +177,8 @@ class DeviceLocalAccountPolicyService { scoped_refptr external_data_service_backend_task_runner, scoped_refptr io_task_runner, - scoped_refptr request_context); + scoped_refptr request_context, + scoped_refptr url_loader_factory); virtual ~DeviceLocalAccountPolicyService(); // Shuts down the service and prevents further policy fetches from the cloud. @@ -273,6 +276,7 @@ class DeviceLocalAccountPolicyService { std::unique_ptr external_data_service_; scoped_refptr request_context_; + scoped_refptr url_loader_factory_; const std::unique_ptr local_accounts_subscription_; diff --git a/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc b/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc index 592d6bddc8d767..2b2959e75ac570 100644 --- a/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc +++ b/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc @@ -47,6 +47,7 @@ #include "components/policy/proto/device_management_backend.pb.h" #include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_test_util.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "testing/gtest/include/gtest/gtest.h" using testing::AnyNumber; @@ -167,8 +168,8 @@ void DeviceLocalAccountPolicyServiceTestBase::CreatePolicyService() { &affiliated_invalidation_service_provider_, base::ThreadTaskRunnerHandle::Get(), extension_cache_task_runner_, base::ThreadTaskRunnerHandle::Get(), base::ThreadTaskRunnerHandle::Get(), - new net::TestURLRequestContextGetter( - base::ThreadTaskRunnerHandle::Get()))); + new net::TestURLRequestContextGetter(base::ThreadTaskRunnerHandle::Get()), + /*url_loader_factory=*/nullptr)); } void DeviceLocalAccountPolicyServiceTestBase:: diff --git a/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.cc b/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.cc index a2230a8f25a744..620f0f2a96d41d 100644 --- a/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.cc +++ b/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.cc @@ -20,6 +20,7 @@ #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" using content::BrowserThread; @@ -48,13 +49,17 @@ class PolicyOAuth2TokenFetcherImpl : public PolicyOAuth2TokenFetcher, void StartWithSigninContext( net::URLRequestContextGetter* auth_context_getter, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, + const TokenCallback& callback) override; + void StartWithAuthCode( + const std::string& auth_code, + net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) override; - void StartWithAuthCode(const std::string& auth_code, - net::URLRequestContextGetter* system_context_getter, - const TokenCallback& callback) override; void StartWithRefreshToken( const std::string& oauth2_refresh_token, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) override; // Returns true if we have previously attempted to fetch tokens with this @@ -100,6 +105,7 @@ class PolicyOAuth2TokenFetcherImpl : public PolicyOAuth2TokenFetcher, scoped_refptr auth_context_getter_; scoped_refptr system_context_getter_; + scoped_refptr system_url_loader_factory_; std::unique_ptr refresh_token_fetcher_; std::unique_ptr access_token_fetcher_; @@ -132,11 +138,13 @@ PolicyOAuth2TokenFetcherImpl::~PolicyOAuth2TokenFetcherImpl() {} void PolicyOAuth2TokenFetcherImpl::StartWithSigninContext( net::URLRequestContextGetter* auth_context_getter, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) { DCHECK(!refresh_token_fetcher_ && !access_token_fetcher_); auth_context_getter_ = auth_context_getter; system_context_getter_ = system_context_getter; + system_url_loader_factory_ = system_url_loader_factory; callback_ = callback; StartFetchingRefreshToken(); } @@ -144,11 +152,13 @@ void PolicyOAuth2TokenFetcherImpl::StartWithSigninContext( void PolicyOAuth2TokenFetcherImpl::StartWithAuthCode( const std::string& auth_code, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) { DCHECK(!refresh_token_fetcher_ && !access_token_fetcher_); auth_code_ = auth_code; system_context_getter_ = system_context_getter; + system_url_loader_factory_ = system_url_loader_factory; callback_ = callback; StartFetchingRefreshToken(); } @@ -156,11 +166,13 @@ void PolicyOAuth2TokenFetcherImpl::StartWithAuthCode( void PolicyOAuth2TokenFetcherImpl::StartWithRefreshToken( const std::string& oauth2_refresh_token, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) { DCHECK(!refresh_token_fetcher_ && !access_token_fetcher_); oauth2_refresh_token_ = oauth2_refresh_token; system_context_getter_ = system_context_getter; + system_url_loader_factory_ = system_url_loader_factory; callback_ = callback; StartFetchingAccessToken(); } @@ -192,10 +204,8 @@ void PolicyOAuth2TokenFetcherImpl::StartFetchingAccessToken() { std::vector scopes; scopes.push_back(GaiaConstants::kDeviceManagementServiceOAuth); scopes.push_back(GaiaConstants::kOAuthWrapBridgeUserInfoScope); - access_token_fetcher_.reset( - new OAuth2AccessTokenFetcherImpl(this, - system_context_getter_.get(), - oauth2_refresh_token_)); + access_token_fetcher_.reset(new OAuth2AccessTokenFetcherImpl( + this, system_url_loader_factory_, oauth2_refresh_token_)); access_token_fetcher_->Start( GaiaUrls::GetInstance()->oauth2_chrome_client_id(), GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), @@ -277,19 +287,23 @@ class PolicyOAuth2TokenFetcherFake : public PolicyOAuth2TokenFetcher { void StartWithSigninContext( net::URLRequestContextGetter* auth_context_getter, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) override { ForwardPolicyToken(callback); } - void StartWithAuthCode(const std::string& auth_code, - net::URLRequestContextGetter* system_context_getter, - const TokenCallback& callback) override { + void StartWithAuthCode( + const std::string& auth_code, + net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, + const TokenCallback& callback) override { ForwardPolicyToken(callback); } void StartWithRefreshToken( const std::string& oauth2_refresh_token, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) override { ForwardPolicyToken(callback); } diff --git a/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h b/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h index a42bf803aaee2a..f32574df6d9b10 100644 --- a/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h +++ b/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h @@ -10,12 +10,17 @@ #include "base/callback.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "google_apis/gaia/gaia_auth_consumer.h" namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} + namespace policy { // Fetches the OAuth2 token for the device management service. Since Profile @@ -43,14 +48,17 @@ class PolicyOAuth2TokenFetcher { virtual void StartWithSigninContext( net::URLRequestContextGetter* auth_context_getter, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) = 0; virtual void StartWithAuthCode( const std::string& auth_code, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) = 0; virtual void StartWithRefreshToken( const std::string& oauth2_refresh_token, net::URLRequestContextGetter* system_context_getter, + scoped_refptr system_url_loader_factory, const TokenCallback& callback) = 0; // Returns true if we have previously attempted to fetch tokens with this diff --git a/chrome/browser/chromeos/policy/upload_job_unittest.cc b/chrome/browser/chromeos/policy/upload_job_unittest.cc index ab2bec38749597..657e7270d7381c 100644 --- a/chrome/browser/chromeos/policy/upload_job_unittest.cc +++ b/chrome/browser/chromeos/policy/upload_job_unittest.cc @@ -28,6 +28,7 @@ #include "net/test/embedded_test_server/http_response.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "net/url_request/url_request_test_util.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "testing/gtest/include/gtest/gtest.h" namespace policy { @@ -69,12 +70,14 @@ class MockOAuth2TokenService : public FakeOAuth2TokenService { ~MockOAuth2TokenService() override; // OAuth2TokenService: - void FetchOAuth2Token(RequestImpl* request, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes) override; + void FetchOAuth2Token( + RequestImpl* request, + const std::string& account_id, + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes) override; // OAuth2TokenService: void InvalidateAccessTokenImpl(const std::string& account_id, @@ -104,6 +107,7 @@ void MockOAuth2TokenService::FetchOAuth2Token( OAuth2TokenService::RequestImpl* request, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const FakeOAuth2TokenService::ScopeSet& scopes) { diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc index 586bc19145f6aa..7c860b67e98934 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc @@ -34,6 +34,7 @@ #include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/invalidation/profile_invalidation_provider_factory.h" #include "chrome/browser/lifetime/application_lifetime.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/policy/cloud/remote_commands_invalidator_impl.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_content_client.h" @@ -56,6 +57,7 @@ #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_source.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "url/gurl.h" namespace em = enterprise_management; @@ -179,12 +181,18 @@ void UserCloudPolicyManagerChromeOS::ForceTimeoutForTest() { OnPolicyRefreshTimeout(); } +void UserCloudPolicyManagerChromeOS::SetSystemURLLoaderFactoryForTests( + scoped_refptr system_url_loader_factory) { + system_url_loader_factory_for_tests_ = system_url_loader_factory; +} + UserCloudPolicyManagerChromeOS::~UserCloudPolicyManagerChromeOS() {} void UserCloudPolicyManagerChromeOS::Connect( PrefService* local_state, DeviceManagementService* device_management_service, - scoped_refptr system_request_context) { + scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory) { DCHECK(device_management_service); DCHECK(local_state); @@ -209,7 +217,8 @@ void UserCloudPolicyManagerChromeOS::Connect( std::make_unique( std::string() /* machine_id */, std::string() /* machine_model */, std::string() /* brand_code */, device_management_service, - system_request_context, nullptr /* signing_service */, + system_request_context, system_url_loader_factory, + nullptr /* signing_service */, chromeos::GetDeviceDMTokenForUserPolicyGetter(account_id_)); CreateComponentCloudPolicyService( dm_protocol::kChromeExtensionPolicyType, component_policy_cache_path_, @@ -535,6 +544,17 @@ void UserCloudPolicyManagerChromeOS::FetchPolicyOAuthToken() { return; } + // TODO(jcivelli): Connect() is passed a SharedURLLoaderFactory but here we + // retrieve it from |g_browser_process|. We should move away from retrieving + // it from |g_browser_process| at which point we can remove + // SetSystemURLLoaderFactoryForTests(). + scoped_refptr system_url_loader_factory = + system_url_loader_factory_for_tests_; + if (!system_url_loader_factory) { + system_url_loader_factory = + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(); + } const std::string& refresh_token = chromeos::UserSessionManager::GetInstance() ->user_context() .GetRefreshToken(); @@ -542,6 +562,7 @@ void UserCloudPolicyManagerChromeOS::FetchPolicyOAuthToken() { token_fetcher_.reset(PolicyOAuth2TokenFetcher::CreateInstance()); token_fetcher_->StartWithRefreshToken( refresh_token, g_browser_process->system_request_context(), + system_url_loader_factory, base::Bind(&UserCloudPolicyManagerChromeOS::OnOAuth2PolicyTokenFetched, base::Unretained(this))); return; @@ -559,6 +580,7 @@ void UserCloudPolicyManagerChromeOS::FetchPolicyOAuthToken() { token_fetcher_.reset(PolicyOAuth2TokenFetcher::CreateInstance()); token_fetcher_->StartWithSigninContext( signin_context.get(), g_browser_process->system_request_context(), + system_url_loader_factory, base::Bind(&UserCloudPolicyManagerChromeOS::OnOAuth2PolicyTokenFetched, base::Unretained(this))); } diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h index 7601e8f4abc290..99508edc2e9de7 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h @@ -39,6 +39,10 @@ namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} + namespace policy { class AppInstallEventLogUploader; @@ -114,7 +118,8 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, void Connect( PrefService* local_state, DeviceManagementService* device_management_service, - scoped_refptr system_request_context); + scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory); // This class is one of the policy providers, and must be ready for the // creation of the Profile's PrefService; all the other KeyedServices depend @@ -163,6 +168,11 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, // policy server. StatusUploader* GetStatusUploader() const { return status_uploader_.get(); } + // Sets a SharedURLLoaderFactory that should be used for tests instead of + // retrieving one from the BrowserProcess object in FetchPolicyOAuthToken(). + void SetSystemURLLoaderFactoryForTests( + scoped_refptr system_url_loader_factory); + protected: // CloudPolicyManager: void GetChromePolicy(PolicyMap* policy_map) override; @@ -295,6 +305,10 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, std::unique_ptr shutdown_notifier_; + // The SharedURLLoaderFactory used in some tests to simulate network requests. + scoped_refptr + system_url_loader_factory_for_tests_; + DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyManagerChromeOS); }; diff --git a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc index 12e792cebb8694..04024e2f860c36 100644 --- a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc +++ b/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc @@ -59,6 +59,9 @@ #include "net/url_request/url_fetcher_delegate.h" #include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_status.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/gtest/include/gtest/gtest.h" @@ -81,26 +84,26 @@ namespace policy { using PolicyEnforcement = UserCloudPolicyManagerChromeOS::PolicyEnforcement; -const char kAccountId[] = "user@example.com"; -const char kTestGaiaId[] = "12345"; +constexpr char kAccountId[] = "user@example.com"; +constexpr char kTestGaiaId[] = "12345"; -const char kChildAccountId[] = "child@example.com"; -const char kChildTestGaiaId[] = "54321"; +constexpr char kChildAccountId[] = "child@example.com"; +constexpr char kChildTestGaiaId[] = "54321"; -const char kOAuthCodeCookie[] = "oauth_code=1234; Secure; HttpOnly"; +constexpr char kOAuthCodeCookie[] = "oauth_code=1234; Secure; HttpOnly"; -const char kOAuth2TokenPairData[] = - "{" - " \"refresh_token\": \"1234\"," - " \"access_token\": \"5678\"," - " \"expires_in\": 3600" - "}"; +constexpr char kOAuth2TokenPairData[] = R"( + { + "refresh_token": "1234", + "access_token": "5678", + "expires_in": 3600 + })"; -const char kOAuth2AccessTokenData[] = - "{" - " \"access_token\": \"5678\"," - " \"expires_in\": 3600" - "}"; +constexpr char kOAuth2AccessTokenData[] = R"( + { + "access_token": "5678", + "expires_in": 3600 + })"; class UserCloudPolicyManagerChromeOSTest : public testing::Test { public: @@ -122,13 +125,16 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { protected: UserCloudPolicyManagerChromeOSTest() - : store_(NULL), - external_data_manager_(NULL), + : store_(nullptr), + external_data_manager_(nullptr), task_runner_(new base::TestSimpleTaskRunner()), - profile_(NULL), - signin_profile_(NULL), + profile_(nullptr), + signin_profile_(nullptr), user_manager_(new chromeos::FakeChromeUserManager()), - user_manager_enabler_(base::WrapUnique(user_manager_)) {} + user_manager_enabler_(base::WrapUnique(user_manager_)), + test_shared_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)) {} void AddAndSwitchToChildAccountWithProfile() { const AccountId child_account_id = @@ -217,6 +223,7 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { signin_profile_ = NULL; profile_ = NULL; profile_manager_->DeleteTestingProfile(chrome::kInitialProfile); + test_shared_loader_factory_->Detach(); chromeos::DBusThreadManager::Shutdown(); } @@ -251,6 +258,19 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { return fetcher; } + void SimulateOAuth2TokenResponse(const std::string& content) { + GURL url = GaiaUrls::GetInstance()->oauth2_token_url(); + ASSERT_TRUE(test_url_loader_factory_.IsPending(url.spec())); + ASSERT_EQ(url, (*test_url_loader_factory_.pending_requests())[0].url); + + network::TestURLLoaderFactory::PendingRequest request = + std::move((*test_url_loader_factory_.pending_requests())[0]); + test_url_loader_factory_.pending_requests()->erase( + test_url_loader_factory_.pending_requests()->begin()); + network::TestURLLoaderFactory::SimulateResponse(std::move(request), + content); + } + // Issues the OAuth2 tokens and returns the device management register job // if the flow succeeded. MockDeviceManagementJob* IssueOAuthToken(bool has_request_token) { @@ -265,13 +285,13 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { if (!has_request_token) { GaiaUrls* gaia_urls = GaiaUrls::GetInstance(); - net::TestURLFetcher* fetcher = NULL; + net::TestURLFetcher* fetcher = nullptr; // Issue the oauth_token cookie first. fetcher = PrepareOAuthFetcher( gaia_urls->deprecated_client_login_to_oauth2_url()); if (!fetcher) - return NULL; + return nullptr; scoped_refptr reponse_headers = new net::HttpResponseHeaders(""); @@ -280,18 +300,16 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { fetcher->delegate()->OnURLFetchComplete(fetcher); // Issue the refresh token. + // This uses GaiaAuthFetcher() which still uses URLRequests. fetcher = PrepareOAuthFetcher(gaia_urls->oauth2_token_url()); if (!fetcher) return NULL; fetcher->SetResponseString(kOAuth2TokenPairData); fetcher->delegate()->OnURLFetchComplete(fetcher); - // Issue the access token. - fetcher = PrepareOAuthFetcher(gaia_urls->oauth2_token_url()); - if (!fetcher) - return NULL; - fetcher->SetResponseString(kOAuth2AccessTokenData); - fetcher->delegate()->OnURLFetchComplete(fetcher); + // Issue the access token. This uses OAuth2AccessTokenFetcher which uses + // the network service. + SimulateOAuth2TokenResponse(kOAuth2AccessTokenData); } else { // Since the refresh token is available, OAuth2TokenService was used // to request the access token and not UserCloudPolicyTokenForwarder. @@ -404,12 +422,15 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { base::Unretained(this)), active_user->GetAccountId(), task_runner_, task_runner_)); manager_->AddObserver(&observer_); + manager_->SetSystemURLLoaderFactoryForTests(test_shared_loader_factory_); should_create_token_forwarder_ = fetch_timeout.is_zero(); } void InitAndConnectManager() { manager_->Init(&schema_registry_); - manager_->Connect(&prefs_, &device_management_service_, NULL); + manager_->Connect(&prefs_, &device_management_service_, + /*system_request_context=*/nullptr, + /*system_url_loader_factory=*/nullptr); if (should_create_token_forwarder_) { // Create the UserCloudPolicyTokenForwarder, which fetches the access // token using the OAuth2PolicyFetcher and forwards it to the @@ -433,6 +454,10 @@ class UserCloudPolicyManagerChromeOSTest : public testing::Test { bool should_create_token_forwarder_ = false; bool fatal_error_encountered_ = false; + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr + test_shared_loader_factory_; + DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyManagerChromeOSTest); }; diff --git a/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc b/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc index c41203cceccf57..5a376bef9f9581 100644 --- a/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc +++ b/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc @@ -28,6 +28,7 @@ #include "chrome/browser/chromeos/settings/cros_settings.h" #include "chrome/browser/chromeos/settings/install_attributes.h" #include "chrome/browser/lifetime/application_lifetime.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/policy/schema_registry_service.h" #include "chrome/browser/policy/schema_registry_service_factory.h" #include "chrome/browser/profiles/profile.h" @@ -49,6 +50,7 @@ #include "components/user_manager/user_manager.h" #include "content/public/browser/browser_thread.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" using user_manager::known_user::ProfileRequiresPolicy; namespace policy { @@ -419,7 +421,9 @@ UserPolicyManagerFactoryChromeOS::CreateManagerForProfile( SchemaRegistryServiceFactory::GetForContext(profile)->registry()); manager->Connect(g_browser_process->local_state(), device_management_service, - g_browser_process->system_request_context()); + g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory()); cloud_managers_[profile] = manager.get(); return std::move(manager); diff --git a/chrome/browser/chromeos/policy/wildcard_login_checker.cc b/chrome/browser/chromeos/policy/wildcard_login_checker.cc index b0192d86e47c68..7d12bce9df1b88 100644 --- a/chrome/browser/chromeos/policy/wildcard_login_checker.cc +++ b/chrome/browser/chromeos/policy/wildcard_login_checker.cc @@ -9,8 +9,10 @@ #include "base/metrics/histogram_macros.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "components/policy/core/browser/browser_policy_connector.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace policy { @@ -46,6 +48,8 @@ void WildcardLoginChecker::StartWithSigninContext( token_fetcher_.reset(PolicyOAuth2TokenFetcher::CreateInstance()); token_fetcher_->StartWithSigninContext( signin_context.get(), g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(), base::Bind(&WildcardLoginChecker::OnPolicyTokenFetched, base::Unretained(this))); } @@ -62,6 +66,8 @@ void WildcardLoginChecker::StartWithRefreshToken( token_fetcher_.reset(PolicyOAuth2TokenFetcher::CreateInstance()); token_fetcher_->StartWithRefreshToken( refresh_token, g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(), base::Bind(&WildcardLoginChecker::OnPolicyTokenFetched, base::Unretained(this))); } diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service.cc b/chrome/browser/chromeos/settings/device_oauth2_token_service.cc index 0273d2c7c5158a..7742b30656973c 100644 --- a/chrome/browser/chromeos/settings/device_oauth2_token_service.cc +++ b/chrome/browser/chromeos/settings/device_oauth2_token_service.cc @@ -17,6 +17,7 @@ #include "chrome/common/pref_names.h" #include "components/prefs/pref_registry_simple.h" #include "google_apis/gaia/google_service_auth_error.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace chromeos { @@ -75,6 +76,7 @@ void DeviceOAuth2TokenService::FetchOAuth2Token( RequestImpl* request, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const ScopeSet& scopes) { @@ -98,8 +100,9 @@ void DeviceOAuth2TokenService::FetchOAuth2Token( return; case DeviceOAuth2TokenServiceDelegate::STATE_TOKEN_VALID: // Pass through to OAuth2TokenService to satisfy the request. - OAuth2TokenService::FetchOAuth2Token( - request, account_id, getter, client_id, client_secret, scopes); + OAuth2TokenService::FetchOAuth2Token(request, account_id, getter, + url_loader_factory, client_id, + client_secret, scopes); return; } @@ -122,7 +125,8 @@ void DeviceOAuth2TokenService::FlushPendingRequests( OAuth2TokenService::FetchOAuth2Token( scoped_request->request.get(), scoped_request->request->GetAccountId(), - GetDeviceDelegate()->GetRequestContext(), scoped_request->client_id, + GetDeviceDelegate()->GetRequestContext(), + GetDeviceDelegate()->GetURLLoaderFactory(), scoped_request->client_id, scoped_request->client_secret, scoped_request->scopes); } else { FailRequest(scoped_request->request.get(), error); diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service.h b/chrome/browser/chromeos/settings/device_oauth2_token_service.h index b23e93a2ec0474..7a758494f52a02 100644 --- a/chrome/browser/chromeos/settings/device_oauth2_token_service.h +++ b/chrome/browser/chromeos/settings/device_oauth2_token_service.h @@ -12,11 +12,6 @@ #include "base/macros.h" #include "chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h" #include "google_apis/gaia/oauth2_token_service.h" -#include "net/url_request/url_request_context_getter.h" - -namespace net { -class URLRequestContextGetter; -} class PrefRegistrySimple; @@ -51,12 +46,15 @@ class DeviceOAuth2TokenService protected: // Implementation of OAuth2TokenService. - void FetchOAuth2Token(RequestImpl* request, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes) override; + void FetchOAuth2Token( + RequestImpl* request, + const std::string& account_id, + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes) override; + private: friend class DeviceOAuth2TokenServiceFactory; friend class DeviceOAuth2TokenServiceTest; diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.cc b/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.cc index 7d850d1577df67..be9058b9932725 100644 --- a/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.cc +++ b/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.cc @@ -23,6 +23,7 @@ #include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace chromeos { @@ -33,20 +34,21 @@ void DeviceOAuth2TokenServiceDelegate::OnServiceAccountIdentityChanged() { DeviceOAuth2TokenServiceDelegate::DeviceOAuth2TokenServiceDelegate( net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, PrefService* local_state) : url_request_context_getter_(getter), + url_loader_factory_(url_loader_factory), local_state_(local_state), state_(STATE_LOADING), max_refresh_token_validation_retries_(3), validation_requested_(false), validation_status_delegate_(nullptr), service_account_identity_subscription_( - CrosSettings::Get() - ->AddSettingsObserver( - kServiceAccountIdentity, - base::Bind(&DeviceOAuth2TokenServiceDelegate:: - OnServiceAccountIdentityChanged, - base::Unretained(this)))), + CrosSettings::Get()->AddSettingsObserver( + kServiceAccountIdentity, + base::Bind(&DeviceOAuth2TokenServiceDelegate:: + OnServiceAccountIdentityChanged, + base::Unretained(this)))), weak_ptr_factory_(this) { // Pull in the system salt. SystemSaltGetter::Get()->GetSystemSalt( @@ -163,14 +165,21 @@ DeviceOAuth2TokenServiceDelegate::GetRequestContext() const { return url_request_context_getter_.get(); } +scoped_refptr +DeviceOAuth2TokenServiceDelegate::GetURLLoaderFactory() const { + return url_loader_factory_; +} + OAuth2AccessTokenFetcher* DeviceOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) { std::string refresh_token = GetRefreshToken(account_id); DCHECK(!refresh_token.empty()); - return new OAuth2AccessTokenFetcherImpl(consumer, getter, refresh_token); + return new OAuth2AccessTokenFetcherImpl(consumer, url_loader_factory, + refresh_token); } void DeviceOAuth2TokenServiceDelegate::DidGetSystemSalt( diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h b/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h index 7ae81df0781cca..07b4c34cc0057b 100644 --- a/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h +++ b/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h @@ -11,6 +11,7 @@ #include "base/callback.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/chromeos/settings/cros_settings.h" #include "google_apis/gaia/gaia_oauth_client.h" @@ -24,6 +25,9 @@ class GaiaOAuthClient; namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} class PrefService; @@ -33,8 +37,10 @@ class DeviceOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate, public gaia::GaiaOAuthClient::Delegate { public: - DeviceOAuth2TokenServiceDelegate(net::URLRequestContextGetter* getter, - PrefService* local_state); + DeviceOAuth2TokenServiceDelegate( + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + PrefService* local_state); ~DeviceOAuth2TokenServiceDelegate() override; typedef base::Callback StatusCallback; @@ -53,9 +59,13 @@ class DeviceOAuth2TokenServiceDelegate net::URLRequestContextGetter* GetRequestContext() const override; + scoped_refptr GetURLLoaderFactory() + const override; + OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) override; // gaia::GaiaOAuthClient::Delegate implementation. @@ -122,6 +132,7 @@ class DeviceOAuth2TokenServiceDelegate // Dependencies. scoped_refptr url_request_context_getter_; + scoped_refptr url_loader_factory_; PrefService* local_state_; // Current operational state. diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc b/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc index 61d8a0acdf36d2..239f4b764f5d1e 100644 --- a/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc +++ b/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc @@ -10,8 +10,10 @@ #include "chrome/browser/chromeos/settings/device_oauth2_token_service.h" #include "chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h" #include "chrome/browser/chromeos/settings/token_encryptor.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chromeos/cryptohome/system_salt_getter.h" #include "content/public/browser/browser_thread.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace chromeos { @@ -27,13 +29,19 @@ DeviceOAuth2TokenService* DeviceOAuth2TokenServiceFactory::Get() { return g_device_oauth2_token_service_; } -// static void DeviceOAuth2TokenServiceFactory::Initialize() { + Initialize(g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory()); +} + +// static +void DeviceOAuth2TokenServiceFactory::Initialize( + scoped_refptr url_loader_factory) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK(!g_device_oauth2_token_service_); g_device_oauth2_token_service_ = new DeviceOAuth2TokenService( std::make_unique( - g_browser_process->system_request_context(), + g_browser_process->system_request_context(), url_loader_factory, g_browser_process->local_state())); } diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h b/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h index 11bce887de34f2..e8a951cd84254c 100644 --- a/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h +++ b/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h @@ -9,6 +9,11 @@ #include #include "base/macros.h" +#include "base/memory/ref_counted.h" + +namespace network { +class SharedURLLoaderFactory; +} namespace chromeos { @@ -29,6 +34,11 @@ class DeviceOAuth2TokenServiceFactory { // available (local state, request context getter and CrosSettings). static void Initialize(); + // Same as |Initialize()| but uses |url_loader_factory| for fetching OAuth + // tokens. Used in tests. + static void Initialize( + scoped_refptr url_loader_factory); + // Called by ChromeBrowserMainPartsChromeOS in order to shutdown the // DeviceOAuth2TokenService instance and cancel all in-flight requests before // the required global data is destroyed (local state, request context getter diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc b/chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc index 651a62d342ea08..d5228dcb08f462 100644 --- a/chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc +++ b/chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc @@ -31,11 +31,14 @@ #include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_utils.h" #include "google_apis/gaia/gaia_oauth_client.h" +#include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/oauth2_token_service_test_util.h" #include "net/http/http_status_code.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher_delegate.h" #include "net/url_request/url_request_test_util.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/gtest/include/gtest/gtest.h" @@ -59,7 +62,6 @@ MockOAuth2TokenServiceObserver::~MockOAuth2TokenServiceObserver() { } // namespace -static const int kOAuthTokenServiceUrlFetcherId = 0; static const int kValidatorUrlFetcherId = gaia::GaiaOAuthClient::kUrlFetcherId; class DeviceOAuth2TokenServiceTest : public testing::Test { @@ -67,7 +69,10 @@ class DeviceOAuth2TokenServiceTest : public testing::Test { DeviceOAuth2TokenServiceTest() : scoped_testing_local_state_(TestingBrowserProcess::GetGlobal()), request_context_getter_(new net::TestURLRequestContextGetter( - base::ThreadTaskRunnerHandle::Get())) {} + base::ThreadTaskRunnerHandle::Get())), + test_shared_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)) {} ~DeviceOAuth2TokenServiceTest() override {} // Most tests just want a noop crypto impl with a dummy refresh token value in @@ -124,6 +129,7 @@ class DeviceOAuth2TokenServiceTest : public testing::Test { void TearDown() override { oauth2_service_.reset(); + test_shared_loader_factory_->Detach(); CrosSettings::Shutdown(); TestingBrowserProcess::GetGlobal()->ShutdownBrowserPolicyConnector(); base::TaskScheduler::GetInstance()->FlushForTesting(); @@ -136,7 +142,8 @@ class DeviceOAuth2TokenServiceTest : public testing::Test { void CreateService() { auto delegate = std::make_unique( - request_context_getter_.get(), scoped_testing_local_state_.Get()); + request_context_getter_.get(), test_shared_loader_factory_, + scoped_testing_local_state_.Get()); delegate->max_refresh_token_validation_retries_ = 0; oauth2_service_.reset(new DeviceOAuth2TokenService(std::move(delegate))); oauth2_service_->set_max_authorization_token_fetch_retries_for_testing(0); @@ -210,6 +217,9 @@ class DeviceOAuth2TokenServiceTest : public testing::Test { content::TestBrowserThreadBundle test_browser_thread_bundle_; ScopedTestingLocalState scoped_testing_local_state_; scoped_refptr request_context_getter_; + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr + test_shared_loader_factory_; net::TestURLFetcherFactory factory_; FakeCryptohomeClient* fake_cryptohome_client_; FakeSessionManagerClient session_manager_client_; @@ -240,6 +250,12 @@ void DeviceOAuth2TokenServiceTest::PerformURLFetchesWithResults( const std::string& tokeninfo_fetch_response, net::HttpStatusCode service_access_token_status, const std::string& service_access_token_response) { + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url().spec(), + service_access_token_response, service_access_token_status); + + // This sends a response to the OAuth token request made by + // GaiaOAuthClient::Core, that should eventually be ported to SimpleURLLoader. ReturnOAuthUrlFetchResults( kValidatorUrlFetcherId, tokeninfo_access_token_status, @@ -249,11 +265,6 @@ void DeviceOAuth2TokenServiceTest::PerformURLFetchesWithResults( kValidatorUrlFetcherId, tokeninfo_fetch_status, tokeninfo_fetch_response); - - ReturnOAuthUrlFetchResults( - kOAuthTokenServiceUrlFetcherId, - service_access_token_status, - service_access_token_response); } void DeviceOAuth2TokenServiceTest::PerformURLFetches() { diff --git a/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.cc b/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.cc index 0012a40fe87f77..a2c2510c12e9e8 100644 --- a/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.cc +++ b/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.cc @@ -11,6 +11,7 @@ #include "base/values.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/extensions/api/enterprise_reporting_private/chrome_desktop_report_request_helper.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/policy/browser_dm_token_storage.h" #include "chrome/browser/policy/chrome_browser_policy_connector.h" #include "chrome/browser/profiles/profile.h" @@ -19,6 +20,7 @@ #include "components/policy/core/common/cloud/device_management_service.h" #include "components/policy/proto/device_management_backend.pb.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace em = enterprise_management; @@ -40,17 +42,25 @@ const char kDeviceNotEnrolled[] = "This device has not been enrolled yet."; } // namespace enterprise_reporting EnterpriseReportingPrivateUploadChromeDesktopReportFunction:: - EnterpriseReportingPrivateUploadChromeDesktopReportFunction() { + EnterpriseReportingPrivateUploadChromeDesktopReportFunction() + : EnterpriseReportingPrivateUploadChromeDesktopReportFunction( + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory()) {} + +EnterpriseReportingPrivateUploadChromeDesktopReportFunction:: + EnterpriseReportingPrivateUploadChromeDesktopReportFunction( + scoped_refptr url_loader_factory) { policy::DeviceManagementService* device_management_service = g_browser_process->browser_policy_connector() ->device_management_service(); // Initial the DeviceManagementService if it exist and hasn't been initialized if (device_management_service) device_management_service->ScheduleInitialization(0); + cloud_policy_client_ = std::make_unique( std::string() /* machine_id */, std::string() /* machine_model */, std::string() /* brand_code */, device_management_service, - g_browser_process->system_request_context(), nullptr, + g_browser_process->system_request_context(), url_loader_factory, nullptr, policy::CloudPolicyClient::DeviceDMTokenCallback()); dm_token_ = policy::BrowserDMTokenStorage::Get()->RetrieveDMToken(); client_id_ = policy::BrowserDMTokenStorage::Get()->RetrieveClientId(); @@ -59,6 +69,14 @@ EnterpriseReportingPrivateUploadChromeDesktopReportFunction:: EnterpriseReportingPrivateUploadChromeDesktopReportFunction:: ~EnterpriseReportingPrivateUploadChromeDesktopReportFunction() {} +// static +EnterpriseReportingPrivateUploadChromeDesktopReportFunction* +EnterpriseReportingPrivateUploadChromeDesktopReportFunction::CreateForTesting( + scoped_refptr url_loader_factory) { + return new EnterpriseReportingPrivateUploadChromeDesktopReportFunction( + url_loader_factory); +} + ExtensionFunction::ResponseAction EnterpriseReportingPrivateUploadChromeDesktopReportFunction::Run() { VLOG(1) << "Uploading enterprise report"; diff --git a/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.h b/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.h index 7f1df151562bbc..d5b68d200434f3 100644 --- a/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.h +++ b/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.h @@ -5,12 +5,17 @@ #ifndef CHROME_BROWSER_EXTENSIONS_API_ENTERPRISE_REPORTING_PRIVATE_ENTERPRISE_REPORTING_PRIVATE_API_H_ #define CHROME_BROWSER_EXTENSIONS_API_ENTERPRISE_REPORTING_PRIVATE_ENTERPRISE_REPORTING_PRIVATE_API_H_ +#include "base/memory/ref_counted.h" #include "extensions/browser/extension_function.h" namespace policy { class CloudPolicyClient; } +namespace network { +class SharedURLLoaderFactory; +} + namespace extensions { namespace enterprise_reporting { @@ -36,7 +41,16 @@ class EnterpriseReportingPrivateUploadChromeDesktopReportFunction void SetRegistrationInfoForTesting(const std::string& dm_token, const std::string& client_id); + // Used by tests that want to overrode the URLLoaderFactory used to simulate + // network requests. + static EnterpriseReportingPrivateUploadChromeDesktopReportFunction* + CreateForTesting( + scoped_refptr url_loader_factory); + private: + explicit EnterpriseReportingPrivateUploadChromeDesktopReportFunction( + scoped_refptr url_loader_factory); + ~EnterpriseReportingPrivateUploadChromeDesktopReportFunction() override; // ExtensionFunction diff --git a/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_unittest.cc b/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_unittest.cc index 14c234c68d6e13..6cd8da022ffd11 100644 --- a/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_unittest.cc +++ b/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_unittest.cc @@ -9,6 +9,9 @@ #include "chrome/browser/extensions/extension_api_unittest.h" #include "chrome/browser/extensions/extension_function_test_utils.h" #include "components/policy/core/common/cloud/mock_cloud_policy_client.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" using ::testing::_; @@ -54,12 +57,18 @@ class MockCloudPolicyClient : public policy::MockCloudPolicyClient { class EnterpriseReportingPrivateTest : public ExtensionApiUnittest { public: - EnterpriseReportingPrivateTest() = default; + EnterpriseReportingPrivateTest() + : test_shared_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)) {} + + void TearDown() override { test_shared_loader_factory_->Detach(); } UIThreadExtensionFunction* CreateChromeDesktopReportingFunction( const std::string& dm_token) { EnterpriseReportingPrivateUploadChromeDesktopReportFunction* function = - new EnterpriseReportingPrivateUploadChromeDesktopReportFunction(); + EnterpriseReportingPrivateUploadChromeDesktopReportFunction:: + CreateForTesting(test_shared_loader_factory_); std::unique_ptr client = std::make_unique(); client_ = client.get(); @@ -83,6 +92,10 @@ class EnterpriseReportingPrivateTest : public ExtensionApiUnittest { MockCloudPolicyClient* client_; private: + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr + test_shared_loader_factory_; + DISALLOW_COPY_AND_ASSIGN(EnterpriseReportingPrivateTest); }; diff --git a/chrome/browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc b/chrome/browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc index 5685c04cc2934b..5a2d4da2f31e80 100644 --- a/chrome/browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc +++ b/chrome/browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc @@ -41,6 +41,7 @@ class FakeDriveServiceFactory std::unique_ptr CreateDriveService( OAuth2TokenService* oauth2_token_service, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, base::SequencedTaskRunner* blocking_task_runner) override { std::unique_ptr drive_service( new drive::FakeDriveService); @@ -90,12 +91,13 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest, base::ThreadTaskRunnerHandle::Get(), // ui_task_runner MakeSequencedTaskRunner(), MakeSequencedTaskRunner(), base_dir_.GetPath(), - NULL, // task_logger - NULL, // notification_manager + nullptr, // task_logger + nullptr, // notification_manager extension_service, fake_signin_manager_.get(), // signin_manager - NULL, // token_service - NULL, // request_context + nullptr, // token_service + nullptr, // request_context + nullptr, // url_loader_factory std::move(drive_service_factory), in_memory_env_.get()); remote_service_->SetSyncEnabled(true); factory->set_mock_remote_file_service( diff --git a/chrome/browser/metrics/ukm_browsertest.cc b/chrome/browser/metrics/ukm_browsertest.cc index 5d17c13d8bf714..c7856b96749db6 100644 --- a/chrome/browser/metrics/ukm_browsertest.cc +++ b/chrome/browser/metrics/ukm_browsertest.cc @@ -68,17 +68,6 @@ void UnblockOnProfileCreation(base::RunLoop* run_loop, run_loop->Quit(); } -Profile* CreateNonSyncProfile() { - ProfileManager* profile_manager = g_browser_process->profile_manager(); - base::FilePath new_path = profile_manager->GenerateNextProfileDirectoryPath(); - base::RunLoop run_loop; - profile_manager->CreateProfileAsync( - new_path, base::Bind(&UnblockOnProfileCreation, &run_loop), - base::string16(), std::string(), std::string()); - run_loop.Run(); - return profile_manager->GetProfileByPath(new_path); -} - Profile* CreateGuestProfile() { ProfileManager* profile_manager = g_browser_process->profile_manager(); base::FilePath new_path = profile_manager->GetGuestProfilePath(); @@ -215,6 +204,20 @@ class UkmBrowserTest : public SyncTest { return harness; } + Profile* CreateNonSyncProfile() { + ProfileManager* profile_manager = g_browser_process->profile_manager(); + base::FilePath new_path = + profile_manager->GenerateNextProfileDirectoryPath(); + base::RunLoop run_loop; + profile_manager->CreateProfileAsync( + new_path, base::Bind(&UnblockOnProfileCreation, &run_loop), + base::string16(), std::string(), std::string()); + run_loop.Run(); + Profile* profile = profile_manager->GetProfileByPath(new_path); + SetupMockGaiaResponsesForProfile(profile); + return profile; + } + private: ukm::UkmService* ukm_service() const { return g_browser_process->GetMetricsServicesManager()->GetUkmService(); @@ -229,7 +232,7 @@ class UkmBrowserTest : public SyncTest { class UkmConsentParamBrowserTest : public UkmBrowserTest, public testing::WithParamInterface { public: - UkmConsentParamBrowserTest() : UkmBrowserTest() {} + UkmConsentParamBrowserTest() {} static bool IsMetricsAndCrashReportingEnabled() { return ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled(); diff --git a/chrome/browser/policy/chrome_browser_policy_connector.cc b/chrome/browser/policy/chrome_browser_policy_connector.cc index 598f5feb2e8637..fd4d95a724f971 100644 --- a/chrome/browser/policy/chrome_browser_policy_connector.cc +++ b/chrome/browser/policy/chrome_browser_policy_connector.cc @@ -28,6 +28,7 @@ #include "components/policy/core/common/policy_types.h" #include "components/policy/policy_constants.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #if defined(OS_WIN) #include "base/win/registry.h" @@ -67,7 +68,8 @@ void ChromeBrowserPolicyConnector::OnResourceBundleCreated() { void ChromeBrowserPolicyConnector::Init( PrefService* local_state, - scoped_refptr request_context) { + scoped_refptr request_context, + scoped_refptr url_loader_factory) { std::unique_ptr configuration( new DeviceManagementServiceConfiguration( BrowserPolicyConnector::GetDeviceManagementUrl())); @@ -79,8 +81,8 @@ void ChromeBrowserPolicyConnector::Init( InitInternal(local_state, std::move(device_management_service)); #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) - machine_level_user_cloud_policy_controller_->Init(local_state, - request_context); + machine_level_user_cloud_policy_controller_->Init( + local_state, request_context, url_loader_factory); #endif } diff --git a/chrome/browser/policy/chrome_browser_policy_connector.h b/chrome/browser/policy/chrome_browser_policy_connector.h index c22580b25ee4cd..36af5691536a4b 100644 --- a/chrome/browser/policy/chrome_browser_policy_connector.h +++ b/chrome/browser/policy/chrome_browser_policy_connector.h @@ -17,10 +17,6 @@ class PrefService; -namespace net { -class URLRequestContextGetter; -} - namespace policy { class ConfigurationPolicyProvider; @@ -47,9 +43,10 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector { // class to notify observers. void OnResourceBundleCreated(); - void Init( - PrefService* local_state, - scoped_refptr request_context) override; + void Init(PrefService* local_state, + scoped_refptr request_context, + scoped_refptr url_loader_factory) + override; bool IsEnterpriseManaged() const override; diff --git a/chrome/browser/policy/cloud/cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/cloud_policy_browsertest.cc index 42c93d76c9d1f2..684f4000dff414 100644 --- a/chrome/browser/policy/cloud/cloud_policy_browsertest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_browsertest.cc @@ -64,6 +64,7 @@ #include "components/account_id/account_id.h" #include "components/user_manager/user_names.h" #else +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h" #include "chrome/browser/signin/signin_manager_factory.h" #include "components/policy/core/common/cloud/user_cloud_policy_manager.h" @@ -223,11 +224,14 @@ class CloudPolicyTest : public InProcessBrowserTest, UserCloudPolicyManagerFactory::GetForBrowserContext( browser()->profile()); ASSERT_TRUE(policy_manager); - policy_manager->Connect(g_browser_process->local_state(), - g_browser_process->system_request_context(), - UserCloudPolicyManager::CreateCloudPolicyClient( - connector->device_management_service(), - g_browser_process->system_request_context())); + policy_manager->Connect( + g_browser_process->local_state(), + g_browser_process->system_request_context(), + UserCloudPolicyManager::CreateCloudPolicyClient( + connector->device_management_service(), + g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory())); #endif // defined(OS_CHROMEOS) ASSERT_TRUE(policy_manager->core()->client()); diff --git a/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc b/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc index c8aca52e1942e6..e73aa4e32d7c03 100644 --- a/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc @@ -30,6 +30,7 @@ #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h" #include "chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.h" #else +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h" #include "chrome/browser/signin/signin_manager_factory.h" #include "components/policy/core/common/cloud/user_cloud_policy_manager.h" @@ -82,11 +83,14 @@ class CloudPolicyManagerTest : public InProcessBrowserTest { signin_manager->SetAuthenticatedAccountInfo("12345", "user@example.com"); ASSERT_TRUE(policy_manager()); - policy_manager()->Connect(g_browser_process->local_state(), - g_browser_process->system_request_context(), - UserCloudPolicyManager::CreateCloudPolicyClient( - connector->device_management_service(), - g_browser_process->system_request_context())); + policy_manager()->Connect( + g_browser_process->local_state(), + g_browser_process->system_request_context(), + UserCloudPolicyManager::CreateCloudPolicyClient( + connector->device_management_service(), + g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory())); #endif } diff --git a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc index 0bf1325fd0afa8..b0c9e55d2fc3c0 100644 --- a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc +++ b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc @@ -43,6 +43,7 @@ #include "chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.h" #include "chromeos/chromeos_switches.h" #else +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h" #include "chrome/browser/signin/signin_manager_factory.h" #include "components/policy/core/common/cloud/user_cloud_policy_manager.h" @@ -193,11 +194,15 @@ class ComponentCloudPolicyTest : public extensions::ExtensionBrowserTest { ASSERT_TRUE(policy_manager); policy_manager->SetSigninAccountId( PolicyBuilder::GetFakeAccountIdForTesting()); - policy_manager->Connect(g_browser_process->local_state(), - g_browser_process->system_request_context(), - UserCloudPolicyManager::CreateCloudPolicyClient( - connector->device_management_service(), - g_browser_process->system_request_context())); + policy_manager->Connect( + g_browser_process->local_state(), + g_browser_process->system_request_context(), + UserCloudPolicyManager::CreateCloudPolicyClient( + connector->device_management_service(), + g_browser_process->system_request_context(), + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory())); + #endif // defined(OS_CHROMEOS) // Register the cloud policy client. diff --git a/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.cc b/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.cc index 4185962db497fe..65caeca2e17665 100644 --- a/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.cc +++ b/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.cc @@ -20,6 +20,7 @@ #include "components/policy/core/common/cloud/machine_level_user_cloud_policy_store.h" #include "components/prefs/pref_service.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace policy { @@ -34,9 +35,11 @@ void OnPolicyFetchCompleted(bool success) { /* MachineLevelUserCloudPolicyRegistrar */ MachineLevelUserCloudPolicyRegistrar::MachineLevelUserCloudPolicyRegistrar( DeviceManagementService* device_management_service, - scoped_refptr system_request_context) + scoped_refptr system_request_context, + scoped_refptr url_loader_factory) : device_management_service_(device_management_service), - system_request_context_(system_request_context) {} + system_request_context_(system_request_context), + url_loader_factory_(url_loader_factory) {} MachineLevelUserCloudPolicyRegistrar::~MachineLevelUserCloudPolicyRegistrar() {} @@ -59,7 +62,7 @@ void MachineLevelUserCloudPolicyRegistrar::RegisterForPolicyWithEnrollmentToken( std::make_unique( std::string() /* machine_id */, std::string() /* machine_model */, std::string() /* brand_code */, device_management_service_, - system_request_context_, nullptr, + system_request_context_, url_loader_factory_, nullptr, CloudPolicyClient::DeviceDMTokenCallback()); // Fire off the registration process. Callback keeps the CloudPolicyClient @@ -88,16 +91,18 @@ MachineLevelUserCloudPolicyFetcher::MachineLevelUserCloudPolicyFetcher( MachineLevelUserCloudPolicyManager* policy_manager, PrefService* local_state, DeviceManagementService* device_management_service, - scoped_refptr system_request_context) + scoped_refptr system_request_context, + scoped_refptr url_loader_factory) : policy_manager_(policy_manager), local_state_(local_state), device_management_service_(device_management_service), - system_request_context_(system_request_context) { + system_request_context_(system_request_context), + url_loader_factory_(url_loader_factory) { std::unique_ptr client = std::make_unique( std::string() /* machine_id */, std::string() /* machine_model */, std::string() /* brand_code */, device_management_service_, - system_request_context_, nullptr, + system_request_context_, url_loader_factory, nullptr, CloudPolicyClient::DeviceDMTokenCallback()); InitializeManager(std::move(client)); } diff --git a/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.h b/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.h index 4723c30797bdf5..685f2a916b4d98 100644 --- a/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.h +++ b/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.h @@ -17,6 +17,10 @@ class PrefService; +namespace network { +class SharedURLLoaderFactory; +} + namespace policy { class CloudPolicyClient; @@ -29,7 +33,8 @@ class MachineLevelUserCloudPolicyRegistrar { public: MachineLevelUserCloudPolicyRegistrar( DeviceManagementService* device_management_service, - scoped_refptr system_request_context); + scoped_refptr system_request_context, + scoped_refptr url_loader_factory); ~MachineLevelUserCloudPolicyRegistrar(); // The callback invoked once policy registration is complete. Passed @@ -53,6 +58,7 @@ class MachineLevelUserCloudPolicyRegistrar { std::unique_ptr registration_helper_; DeviceManagementService* device_management_service_; scoped_refptr system_request_context_; + scoped_refptr url_loader_factory_; DISALLOW_COPY_AND_ASSIGN(MachineLevelUserCloudPolicyRegistrar); }; @@ -64,7 +70,8 @@ class MachineLevelUserCloudPolicyFetcher : public CloudPolicyService::Observer { MachineLevelUserCloudPolicyManager* policy_manager, PrefService* local_state, DeviceManagementService* device_management_service, - scoped_refptr system_request_context); + scoped_refptr system_request_context, + scoped_refptr url_loader_factory); ~MachineLevelUserCloudPolicyFetcher() override; // Initialize the cloud policy client and policy store then fetch @@ -84,6 +91,7 @@ class MachineLevelUserCloudPolicyFetcher : public CloudPolicyService::Observer { PrefService* local_state_; DeviceManagementService* device_management_service_; scoped_refptr system_request_context_; + scoped_refptr url_loader_factory_; DISALLOW_COPY_AND_ASSIGN(MachineLevelUserCloudPolicyFetcher); }; diff --git a/chrome/browser/policy/cloud/user_policy_signin_service.cc b/chrome/browser/policy/cloud/user_policy_signin_service.cc index 3633e08a309a31..aaaebebe4a7f90 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service.cc @@ -21,8 +21,10 @@ #include "components/signin/core/browser/signin_manager.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_source.h" +#include "content/public/browser/storage_partition.h" #include "google_apis/gaia/gaia_constants.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace policy { @@ -33,13 +35,15 @@ UserPolicySigninService::UserPolicySigninService( UserCloudPolicyManager* policy_manager, SigninManager* signin_manager, scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory, ProfileOAuth2TokenService* token_service) : UserPolicySigninServiceBase(profile, local_state, device_management_service, policy_manager, signin_manager, - system_request_context), + system_request_context, + system_url_loader_factory), profile_(profile), oauth2_token_service_(token_service) { // ProfileOAuth2TokenService should not yet have loaded its tokens since this @@ -173,7 +177,9 @@ void UserPolicySigninService::TryInitializeForSignedInUser() { InitializeForSignedInUser( signin_manager()->GetAuthenticatedAccountInfo().GetAccountId(), - profile_->GetRequestContext()); + profile_->GetRequestContext(), + content::BrowserContext::GetDefaultStoragePartition(profile_) + ->GetURLLoaderFactoryForBrowserProcess()); } void UserPolicySigninService::InitializeUserCloudPolicyManager( diff --git a/chrome/browser/policy/cloud/user_policy_signin_service.h b/chrome/browser/policy/cloud/user_policy_signin_service.h index 239852afad2ccd..50b93f8705ac1f 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service.h +++ b/chrome/browser/policy/cloud/user_policy_signin_service.h @@ -21,6 +21,9 @@ class ProfileOAuth2TokenService; namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace policy { @@ -40,6 +43,7 @@ class UserPolicySigninService : public UserPolicySigninServiceBase, UserCloudPolicyManager* policy_manager, SigninManager* signin_manager, scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory, ProfileOAuth2TokenService* oauth2_token_service); ~UserPolicySigninService() override; diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_base.cc b/chrome/browser/policy/cloud/user_policy_signin_service_base.cc index b974f67d95fad4..1c3aaac363243e 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_base.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_base.cc @@ -22,7 +22,9 @@ #include "components/policy/core/common/cloud/user_cloud_policy_manager.h" #include "components/signin/core/browser/signin_manager.h" #include "content/public/browser/notification_source.h" +#include "content/public/browser/storage_partition.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace policy { @@ -32,12 +34,14 @@ UserPolicySigninServiceBase::UserPolicySigninServiceBase( DeviceManagementService* device_management_service, UserCloudPolicyManager* policy_manager, SigninManager* signin_manager, - scoped_refptr system_request_context) + scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory) : policy_manager_(policy_manager), signin_manager_(signin_manager), local_state_(local_state), device_management_service_(device_management_service), system_request_context_(system_request_context), + system_url_loader_factory_(system_url_loader_factory), weak_factory_(this) { // Register a listener to be called back once the current profile has finished // initializing, so we can startup/shutdown the UserCloudPolicyManager. @@ -53,10 +57,12 @@ void UserPolicySigninServiceBase::FetchPolicyForSignedInUser( const std::string& dm_token, const std::string& client_id, scoped_refptr profile_request_context, + scoped_refptr profile_url_loader_factory, const PolicyFetchCallback& callback) { std::unique_ptr client = UserCloudPolicyManager::CreateCloudPolicyClient( - device_management_service_, profile_request_context); + device_management_service_, profile_request_context, + profile_url_loader_factory); client->SetupRegistration( dm_token, client_id, std::vector() /* user_affiliation_ids */); @@ -164,7 +170,8 @@ UserPolicySigninServiceBase::CreateClientForRegistrationOnly( // Create a new CloudPolicyClient for fetching the DMToken. return UserCloudPolicyManager::CreateCloudPolicyClient( - device_management_service_, system_request_context_); + device_management_service_, system_request_context_, + system_url_loader_factory_); } bool UserPolicySigninServiceBase::ShouldLoadPolicyForUser( @@ -194,12 +201,16 @@ void UserPolicySigninServiceBase::InitializeOnProfileReady(Profile* profile) { if (!account_id.is_valid()) ShutdownUserCloudPolicyManager(); else - InitializeForSignedInUser(account_id, profile->GetRequestContext()); + InitializeForSignedInUser( + account_id, profile->GetRequestContext(), + content::BrowserContext::GetDefaultStoragePartition(profile) + ->GetURLLoaderFactoryForBrowserProcess()); } void UserPolicySigninServiceBase::InitializeForSignedInUser( const AccountId& account_id, - scoped_refptr profile_request_context) { + scoped_refptr profile_request_context, + scoped_refptr profile_url_loader_factory) { DCHECK(account_id.is_valid()); if (!ShouldLoadPolicyForUser(account_id.GetUserEmail())) { DVLOG(1) << "Policy load not enabled for user: " @@ -215,7 +226,8 @@ void UserPolicySigninServiceBase::InitializeForSignedInUser( // initiate a policy fetch. InitializeUserCloudPolicyManager( account_id, UserCloudPolicyManager::CreateCloudPolicyClient( - device_management_service_, profile_request_context)); + device_management_service_, profile_request_context, + profile_url_loader_factory)); } else { manager->SetSigninAccountId(account_id); } diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_base.h b/chrome/browser/policy/cloud/user_policy_signin_service_base.h index 48a5a828361870..8bb5c8dede3e8e 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_base.h +++ b/chrome/browser/policy/cloud/user_policy_signin_service_base.h @@ -27,6 +27,9 @@ class Profile; namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace policy { @@ -69,7 +72,8 @@ class UserPolicySigninServiceBase : public KeyedService, DeviceManagementService* device_management_service, UserCloudPolicyManager* policy_manager, SigninManager* signin_manager, - scoped_refptr system_request_context); + scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory); ~UserPolicySigninServiceBase() override; // Initiates a policy fetch as part of user signin, using a |dm_token| and @@ -82,6 +86,7 @@ class UserPolicySigninServiceBase : public KeyedService, const std::string& dm_token, const std::string& client_id, scoped_refptr profile_request_context, + scoped_refptr profile_url_loader_factory, const PolicyFetchCallback& callback); // SigninManagerBase::Observer implementation: @@ -135,7 +140,9 @@ class UserPolicySigninServiceBase : public KeyedService, // user signs-in and an OAuth2 login refresh token becomes available. void InitializeForSignedInUser( const AccountId& account_id, - scoped_refptr profile_request_context); + scoped_refptr profile_request_context, + scoped_refptr + profile_url_loader_factory); // Initializes the cloud policy manager with the passed |client|. This is // called from InitializeForSignedInUser() when the Profile already has a @@ -171,6 +178,7 @@ class UserPolicySigninServiceBase : public KeyedService, PrefService* local_state_; DeviceManagementService* device_management_service_; scoped_refptr system_request_context_; + scoped_refptr system_url_loader_factory_; base::WeakPtrFactory weak_factory_; diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc b/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc index 46469e4a1b78ac..79749e7d1e725c 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_factory.cc @@ -7,6 +7,7 @@ #include "base/memory/ref_counted.h" #include "build/build_config.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/policy/chrome_browser_policy_connector.h" #include "chrome/browser/policy/cloud/user_cloud_policy_manager_factory.h" #include "chrome/browser/profiles/profile.h" @@ -18,6 +19,7 @@ #include "components/pref_registry/pref_registry_syncable.h" #include "components/prefs/pref_service.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #if defined(OS_ANDROID) #include "chrome/browser/policy/cloud/user_policy_signin_service_mobile.h" @@ -34,6 +36,11 @@ DeviceManagementService* g_device_management_service = NULL; } // namespace +// static +base::LazyInstance>::Leaky + UserPolicySigninServiceFactory::system_url_loader_factory_for_tests_ = + LAZY_INSTANCE_INITIALIZER; + UserPolicySigninServiceFactory::UserPolicySigninServiceFactory() : BrowserContextKeyedServiceFactory( "UserPolicySigninService", @@ -63,6 +70,12 @@ void UserPolicySigninServiceFactory::SetDeviceManagementServiceForTesting( g_device_management_service = device_management_service; } +// static +void UserPolicySigninServiceFactory::SetSystemURLLoaderFactoryForTesting( + scoped_refptr system_url_loader_factory) { + system_url_loader_factory_for_tests_.Get() = system_url_loader_factory; +} + KeyedService* UserPolicySigninServiceFactory::BuildServiceInstanceFor( content::BrowserContext* context) const { Profile* profile = static_cast(context); @@ -71,13 +84,21 @@ KeyedService* UserPolicySigninServiceFactory::BuildServiceInstanceFor( DeviceManagementService* device_management_service = g_device_management_service ? g_device_management_service : connector->device_management_service(); + + scoped_refptr system_url_loader_factory = + system_url_loader_factory_for_tests_.Get(); + if (!system_url_loader_factory && + g_browser_process->system_network_context_manager()) { + system_url_loader_factory = + g_browser_process->system_network_context_manager() + ->GetSharedURLLoaderFactory(); + } + UserPolicySigninService* service = new UserPolicySigninService( - profile, - g_browser_process->local_state(), - device_management_service, + profile, g_browser_process->local_state(), device_management_service, UserCloudPolicyManagerFactory::GetForBrowserContext(context), SigninManagerFactory::GetForProfile(profile), - g_browser_process->system_request_context(), + g_browser_process->system_request_context(), system_url_loader_factory, ProfileOAuth2TokenServiceFactory::GetForProfile(profile)); return service; } diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_factory.h b/chrome/browser/policy/cloud/user_policy_signin_service_factory.h index dcef00d9ed74e3..0e157623f09641 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_factory.h +++ b/chrome/browser/policy/cloud/user_policy_signin_service_factory.h @@ -5,7 +5,9 @@ #ifndef CHROME_BROWSER_POLICY_CLOUD_USER_POLICY_SIGNIN_SERVICE_FACTORY_H_ #define CHROME_BROWSER_POLICY_CLOUD_USER_POLICY_SIGNIN_SERVICE_FACTORY_H_ +#include "base/lazy_instance.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/memory/singleton.h" #include "components/keyed_service/content/browser_context_keyed_service_factory.h" @@ -15,6 +17,10 @@ namespace user_prefs { class PrefRegistrySyncable; } +namespace network { +class SharedURLLoaderFactory; +} + namespace policy { class DeviceManagementService; @@ -38,6 +44,9 @@ class UserPolicySigninServiceFactory static void SetDeviceManagementServiceForTesting( DeviceManagementService* device_management_service); + static void SetSystemURLLoaderFactoryForTesting( + scoped_refptr system_url_loader_factory); + protected: // BrowserContextKeyedServiceFactory implementation. KeyedService* BuildServiceInstanceFor( @@ -53,6 +62,9 @@ class UserPolicySigninServiceFactory private: friend struct base::DefaultSingletonTraits; + static base::LazyInstance>:: + Leaky system_url_loader_factory_for_tests_; + UserPolicySigninServiceFactory(); ~UserPolicySigninServiceFactory() override; diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc b/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc index 7536472f584979..c3aad7fcaa990f 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc @@ -25,6 +25,7 @@ #include "components/signin/core/browser/signin_manager.h" #include "net/base/network_change_notifier.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace em = enterprise_management; @@ -48,17 +49,18 @@ UserPolicySigninService::UserPolicySigninService( UserCloudPolicyManager* policy_manager, SigninManager* signin_manager, scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory, ProfileOAuth2TokenService* token_service) : UserPolicySigninServiceBase(profile, local_state, device_management_service, policy_manager, signin_manager, - system_request_context), + system_request_context, + system_url_loader_factory), oauth2_token_service_(token_service), profile_prefs_(profile->GetPrefs()), - weak_factory_(this) { -} + weak_factory_(this) {} UserPolicySigninService::~UserPolicySigninService() {} diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_mobile.h b/chrome/browser/policy/cloud/user_policy_signin_service_mobile.h index 1c3e4c8340f256..2cd2a30384ed64 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_mobile.h +++ b/chrome/browser/policy/cloud/user_policy_signin_service_mobile.h @@ -22,6 +22,9 @@ class Profile; namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace policy { @@ -38,6 +41,7 @@ class UserPolicySigninService : public UserPolicySigninServiceBase { UserCloudPolicyManager* policy_manager, SigninManager* signin_manager, scoped_refptr system_request_context, + scoped_refptr system_url_loader_factory, ProfileOAuth2TokenService* token_service); ~UserPolicySigninService() override; diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc b/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc index 3a501e5f904853..3584d95c80ae7c 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc @@ -7,6 +7,7 @@ #include "base/files/file_path.h" #include "base/run_loop.h" +#include "base/test/bind_test_util.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" #include "build/build_config.h" @@ -47,6 +48,7 @@ #include "content/public/browser/notification_source.h" #include "content/public/test/test_browser_thread_bundle.h" #include "google_apis/gaia/gaia_constants.h" +#include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/google_service_auth_error.h" #include "net/base/net_errors.h" #include "net/http/http_status_code.h" @@ -54,6 +56,8 @@ #include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_status.h" #include "net/url_request/url_request_test_util.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/gtest/include/gtest/gtest.h" @@ -73,22 +77,22 @@ namespace policy { namespace { -const char kTestGaiaId[] = "gaia-id-testuser@test.com"; -const char kTestUser[] = "testuser@test.com"; +constexpr char kTestGaiaId[] = "gaia-id-testuser@test.com"; +constexpr char kTestUser[] = "testuser@test.com"; #if !defined(OS_ANDROID) -const char kValidTokenResponse[] = - "{" - " \"access_token\": \"at1\"," - " \"expires_in\": 3600," - " \"token_type\": \"Bearer\"" - "}"; +constexpr char kValidTokenResponse[] = R"( + { + "access_token": "at1", + "expires_in": 3600, + "token_type": "Bearer" + })"; #endif -const char kHostedDomainResponse[] = - "{" - " \"hd\": \"test.com\"" - "}"; +constexpr char kHostedDomainResponse[] = R"( + { + "hd": "test.com" + })"; UserCloudPolicyManager* BuildCloudPolicyManager( content::BrowserContext* context) { @@ -108,7 +112,10 @@ class UserPolicySigninServiceTest : public testing::Test { thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP), test_account_id_( AccountId::FromUserEmailGaiaId(kTestUser, kTestGaiaId)), - register_completed_(false) {} + register_completed_(false), + test_system_shared_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)) {} MOCK_METHOD1(OnPolicyRefresh, void(bool)); @@ -132,16 +139,19 @@ class UserPolicySigninServiceTest : public testing::Test { profile_.get()->GetPrefs(), kTestGaiaId, kTestUser), "oauth2_login_refresh_token"); service->RegisterForPolicyWithAccountId(kTestUser, kTestGaiaId, callback); + ASSERT_TRUE(IsRequestActive()); #else service->RegisterForPolicyWithLoginToken(kTestUser, "mock_oauth_token", callback); + ASSERT_TRUE(IsOAuthTokenRequestActive()); #endif - ASSERT_TRUE(IsRequestActive()); } void SetUp() override { UserPolicySigninServiceFactory::SetDeviceManagementServiceForTesting( &device_management_service_); + UserPolicySigninServiceFactory::SetSystemURLLoaderFactoryForTesting( + test_system_shared_loader_factory_); local_state_.reset(new TestingPrefServiceSimple); RegisterLocalState(local_state_->registry()); @@ -152,7 +162,8 @@ class UserPolicySigninServiceTest : public testing::Test { TestingBrowserProcess::GetGlobal()->SetLocalState(local_state_.get()); g_browser_process->browser_policy_connector()->Init( - local_state_.get(), system_request_context_getter_); + local_state_.get(), system_request_context_getter_, + test_system_shared_loader_factory_); // Create a testing profile with cloud-policy-on-signin enabled, and bring // up a UserCloudPolicyManager with a MockUserCloudPolicyStore. @@ -208,6 +219,7 @@ class UserPolicySigninServiceTest : public testing::Test { testing_browser_process->SetLocalState(NULL); local_state_.reset(); testing_browser_process->ShutdownBrowserPolicyConnector(); + test_system_shared_loader_factory_->Detach(); base::RunLoop run_loop; run_loop.RunUntilIdle(); } @@ -233,6 +245,16 @@ class UserPolicySigninServiceTest : public testing::Test { return static_cast(service); } + // Reports whether an OAuth token request is active. + // Note that at this time, only OAuth token requests use the SimpleURLLoader, + // and that's why we need this method and IsRequestActive(). + // Once the other Gaia requests use it, we can have only one method. + bool IsOAuthTokenRequestActive() { + if (!GetTokenService()->GetPendingRequests().empty()) + return true; + return test_url_loader_factory_.NumPending() > 0; + } + bool IsRequestActive() { if (!GetTokenService()->GetPendingRequests().empty()) return true; @@ -240,15 +262,32 @@ class UserPolicySigninServiceTest : public testing::Test { } void MakeOAuthTokenFetchSucceed() { - ASSERT_TRUE(IsRequestActive()); #if defined(OS_ANDROID) + ASSERT_TRUE(IsRequestActive()); GetTokenService()->IssueTokenForAllPendingRequests("access_token", base::Time::Now()); #else - net::TestURLFetcher* fetcher = url_factory_.GetFetcherByID(0); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(kValidTokenResponse); - fetcher->delegate()->OnURLFetchComplete(fetcher); + ASSERT_TRUE(IsOAuthTokenRequestActive()); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url().spec(), + kValidTokenResponse); + base::RunLoop().RunUntilIdle(); + test_url_loader_factory_.ClearResponses(); +#endif + } + + void MakeOAuthTokenFetchFail() { +#if defined(OS_ANDROID) + ASSERT_TRUE(!GetTokenService()->GetPendingRequests().empty()); + GetTokenService()->IssueErrorForAllPendingRequests( + GoogleServiceAuthError::FromServiceError("fail")); +#else + ASSERT_GT(test_url_loader_factory_.NumPending(), 0); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url().spec(), "", + net::HTTP_BAD_REQUEST); + base::RunLoop().RunUntilIdle(); + test_url_loader_factory_.ClearResponses(); #endif } @@ -313,6 +352,7 @@ class UserPolicySigninServiceTest : public testing::Test { signin_service->FetchPolicyForSignedInUser( test_account_id_, dm_token_, client_id_, profile_->GetRequestContext(), + test_system_shared_loader_factory_, base::Bind(&UserPolicySigninServiceTest::OnPolicyRefresh, base::Unretained(this))); @@ -374,6 +414,9 @@ class UserPolicySigninServiceTest : public testing::Test { std::unique_ptr local_state_; scoped_refptr system_request_context_getter_; + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr + test_system_shared_loader_factory_; }; class UserPolicySigninServiceSignedInTest : public UserPolicySigninServiceTest { @@ -630,23 +673,15 @@ TEST_F(UserPolicySigninServiceTest, RegisterPolicyClientOAuthFailure) { // UserCloudPolicyManager should not be initialized. ASSERT_FALSE(manager_->core()->service()); - ASSERT_TRUE(IsRequestActive()); + ASSERT_TRUE(IsOAuthTokenRequestActive()); EXPECT_FALSE(register_completed_); // Cause the access token fetch to fail - callback should be invoked. -#if defined(OS_ANDROID) - ASSERT_TRUE(!GetTokenService()->GetPendingRequests().empty()); - GetTokenService()->IssueErrorForAllPendingRequests( - GoogleServiceAuthError::FromServiceError("fail")); -#else - net::TestURLFetcher* fetcher = url_factory_.GetFetcherByID(0); - fetcher->set_status(net::URLRequestStatus::FromError(net::ERR_FAILED)); - fetcher->delegate()->OnURLFetchComplete(fetcher); -#endif + MakeOAuthTokenFetchFail(); EXPECT_TRUE(register_completed_); EXPECT_TRUE(dm_token_.empty()); - EXPECT_FALSE(IsRequestActive()); + ASSERT_FALSE(IsOAuthTokenRequestActive()); } TEST_F(UserPolicySigninServiceTest, RegisterPolicyClientNonHostedDomain) { @@ -656,7 +691,7 @@ TEST_F(UserPolicySigninServiceTest, RegisterPolicyClientNonHostedDomain) { // UserCloudPolicyManager should not be initialized. ASSERT_FALSE(manager_->core()->service()); - ASSERT_TRUE(IsRequestActive()); + ASSERT_TRUE(IsOAuthTokenRequestActive()); // Cause the access token request to succeed. MakeOAuthTokenFetchSucceed(); @@ -675,7 +710,7 @@ TEST_F(UserPolicySigninServiceTest, RegisterPolicyClientNonHostedDomain) { // DMToken. EXPECT_TRUE(register_completed_); EXPECT_TRUE(dm_token_.empty()); - ASSERT_FALSE(IsRequestActive()); + ASSERT_FALSE(IsOAuthTokenRequestActive()); } TEST_F(UserPolicySigninServiceTest, RegisterPolicyClientFailedRegistration) { @@ -770,7 +805,7 @@ TEST_F(UserPolicySigninServiceTest, FetchPolicyFailed) { UserPolicySigninServiceFactory::GetForProfile(profile_.get()); signin_service->FetchPolicyForSignedInUser( test_account_id_, "mock_dm_token", "mock_client_id", - profile_->GetRequestContext(), + profile_->GetRequestContext(), test_system_shared_loader_factory_, base::Bind(&UserPolicySigninServiceTest::OnPolicyRefresh, base::Unretained(this))); ASSERT_TRUE(fetch_request); diff --git a/chrome/browser/policy/machine_level_user_cloud_policy_controller.cc b/chrome/browser/policy/machine_level_user_cloud_policy_controller.cc index 6d3edb375c0d65..ee17b3a5341d15 100644 --- a/chrome/browser/policy/machine_level_user_cloud_policy_controller.cc +++ b/chrome/browser/policy/machine_level_user_cloud_policy_controller.cc @@ -27,6 +27,7 @@ #include "components/policy/core/common/cloud/machine_level_user_cloud_policy_store.h" #include "content/public/browser/browser_thread.h" #include "content/public/common/content_switches.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #if defined(OS_WIN) #include "chrome/install_static/install_util.h" @@ -90,7 +91,8 @@ MachineLevelUserCloudPolicyController::CreatePolicyManager() { void MachineLevelUserCloudPolicyController::Init( PrefService* local_state, - scoped_refptr request_context) { + scoped_refptr request_context, + scoped_refptr url_loader_factory) { MachineLevelUserCloudPolicyManager* policy_manager = g_browser_process->browser_policy_connector() ->machine_level_user_cloud_policy_manager(); @@ -111,8 +113,8 @@ void MachineLevelUserCloudPolicyController::Init( if (!dm_token.empty()) { policy_fetcher_ = std::make_unique( - policy_manager, local_state, device_management_service, - request_context); + policy_manager, local_state, device_management_service, request_context, + url_loader_factory); return; } @@ -123,9 +125,10 @@ void MachineLevelUserCloudPolicyController::Init( DCHECK(!client_id.empty()); policy_registrar_ = std::make_unique( - device_management_service, request_context); + device_management_service, request_context, url_loader_factory); policy_fetcher_ = std::make_unique( - policy_manager, local_state, device_management_service, request_context); + policy_manager, local_state, device_management_service, request_context, + url_loader_factory); if (dm_token.empty()) { policy_register_watcher_ = diff --git a/chrome/browser/policy/machine_level_user_cloud_policy_controller.h b/chrome/browser/policy/machine_level_user_cloud_policy_controller.h index f0293f29677a53..9a291531e6e03b 100644 --- a/chrome/browser/policy/machine_level_user_cloud_policy_controller.h +++ b/chrome/browser/policy/machine_level_user_cloud_policy_controller.h @@ -17,6 +17,10 @@ class PrefService; +namespace network { +class SharedURLLoaderFactory; +} + namespace policy { class MachineLevelUserCloudPolicyManager; class MachineLevelUserCloudPolicyFetcher; @@ -55,7 +59,8 @@ class MachineLevelUserCloudPolicyController { CreatePolicyManager(); void Init(PrefService* local_state, - scoped_refptr request_context); + scoped_refptr request_context, + scoped_refptr url_loader_factory); RegisterResult WaitUntilPolicyEnrollmentFinished(); diff --git a/chrome/browser/signin/chrome_signin_client.cc b/chrome/browser/signin/chrome_signin_client.cc index 1e55772b947d66..40b57fd468e06d 100644 --- a/chrome/browser/signin/chrome_signin_client.cc +++ b/chrome/browser/signin/chrome_signin_client.cc @@ -45,6 +45,7 @@ #include "components/signin/core/browser/signin_header_helper.h" #include "components/signin/core/browser/signin_pref_names.h" #include "components/signin/core/browser/signin_switches.h" +#include "content/public/browser/browser_context.h" #include "content/public/browser/storage_partition.h" #include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/gaia_urls.h" @@ -206,6 +207,9 @@ net::URLRequestContextGetter* ChromeSigninClient::GetURLRequestContext() { scoped_refptr ChromeSigninClient::GetURLLoaderFactory() { + if (url_loader_factory_for_testing_) + return url_loader_factory_for_testing_; + return content::BrowserContext::GetDefaultStoragePartition(profile_) ->GetURLLoaderFactoryForBrowserProcess(); } @@ -478,6 +482,11 @@ void ChromeSigninClient::SetReadyForDiceMigration(bool is_ready) { #endif } +void ChromeSigninClient::SetURLLoaderFactoryForTest( + scoped_refptr url_loader_factory) { + url_loader_factory_for_testing_ = url_loader_factory; +} + void ChromeSigninClient::OnCloseBrowsersSuccess( const base::Callback& sign_out, const signin_metrics::ProfileSignout signout_source_metric, diff --git a/chrome/browser/signin/chrome_signin_client.h b/chrome/browser/signin/chrome_signin_client.h index d2ea062dfac7f2..b11f238272f429 100644 --- a/chrome/browser/signin/chrome_signin_client.h +++ b/chrome/browser/signin/chrome_signin_client.h @@ -15,6 +15,7 @@ #include "google_apis/gaia/gaia_oauth_client.h" #include "google_apis/gaia/oauth2_token_service.h" #include "net/cookies/cookie_change_dispatcher.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/mojom/network_change_manager.mojom.h" #if !defined(OS_CHROMEOS) @@ -109,6 +110,11 @@ class ChromeSigninClient void AfterCredentialsCopied() override; void SetReadyForDiceMigration(bool is_ready) override; + // Used in tests to override the URLLoaderFactory returned by + // GetURLLoaderFactory(). + void SetURLLoaderFactoryForTest( + scoped_refptr url_loader_factory); + protected: virtual void ShowUserManager(const base::FilePath& profile_path); virtual void LockForceSigninProfile(const base::FilePath& profile_path); @@ -137,6 +143,9 @@ class ChromeSigninClient std::unique_ptr oauth_client_; std::unique_ptr oauth_request_; + scoped_refptr + url_loader_factory_for_testing_; + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(ChromeSigninClient); diff --git a/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc b/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc index 55c0fb69670504..066bd8b324682b 100644 --- a/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc +++ b/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc @@ -26,6 +26,7 @@ #include "google_apis/gaia/oauth2_access_token_fetcher_immediate_error.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace { @@ -376,6 +377,7 @@ OAuth2AccessTokenFetcher* MutableProfileOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) { ValidateAccountId(account_id); // check whether the account has persistent error. @@ -392,7 +394,8 @@ MutableProfileOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( } std::string refresh_token = GetRefreshToken(account_id); DCHECK(!refresh_token.empty()); - return new OAuth2AccessTokenFetcherImpl(consumer, getter, refresh_token); + return new OAuth2AccessTokenFetcherImpl(consumer, url_loader_factory, + refresh_token); } GoogleServiceAuthError MutableProfileOAuth2TokenServiceDelegate::GetAuthError( @@ -467,6 +470,11 @@ MutableProfileOAuth2TokenServiceDelegate::GetRequestContext() const { return client_->GetURLRequestContext(); } +scoped_refptr +MutableProfileOAuth2TokenServiceDelegate::GetURLLoaderFactory() const { + return client_->GetURLLoaderFactory(); +} + OAuth2TokenServiceDelegate::LoadCredentialsState MutableProfileOAuth2TokenServiceDelegate::GetLoadCredentialsState() const { return load_credentials_state_; diff --git a/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h b/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h index c7393535624ba3..7037b09f27d6af 100644 --- a/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h +++ b/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h @@ -44,6 +44,7 @@ class MutableProfileOAuth2TokenServiceDelegate OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) override; // Updates the internal cache of the result from the most-recently-completed @@ -56,6 +57,8 @@ class MutableProfileOAuth2TokenServiceDelegate const std::string& account_id) const override; std::vector GetAccounts() override; net::URLRequestContextGetter* GetRequestContext() const override; + scoped_refptr GetURLLoaderFactory() + const override; void LoadCredentials(const std::string& primary_account_id) override; void UpdateCredentials(const std::string& account_id, diff --git a/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc b/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc index 7c18508591fa6e..2c0399caddc5e1 100644 --- a/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc +++ b/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc @@ -34,6 +34,7 @@ #include "google_apis/gaia/oauth2_token_service_test_util.h" #include "net/http/http_status_code.h" #include "net/url_request/test_url_fetcher_factory.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "testing/gtest/include/gtest/gtest.h" // Defining constant here to handle backward compatiblity tests, but this @@ -105,6 +106,8 @@ class MutableProfileOAuth2TokenServiceDelegateTest client_.reset(new TestSigninClient(&pref_service_)); client_->SetURLRequestContext(new net::TestURLRequestContextGetter( base::ThreadTaskRunnerHandle::Get())); + client_->test_url_loader_factory()->AddResponse( + GaiaUrls::GetInstance()->oauth2_revoke_url().spec(), ""); client_->LoadTokenDatabase(); account_tracker_service_.Initialize(client_.get()); } @@ -115,6 +118,12 @@ class MutableProfileOAuth2TokenServiceDelegateTest OSCryptMocker::TearDown(); } + void AddSuccessfulOAuhTokenResponse() { + client_->test_url_loader_factory()->AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url().spec(), + GetValidTokenResponse("token", 3600)); + } + void CreateOAuth2ServiceDelegate( signin::AccountConsistencyMethod account_consistency) { oauth2_service_delegate_.reset(new MutableProfileOAuth2TokenServiceDelegate( @@ -853,9 +862,7 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, FetchPersistentError) { oauth2_service_delegate_->GetAuthError(kEmail)); // Create a "success" fetch we don't expect to get called. - factory_.SetFakeResponse(GaiaUrls::GetInstance()->oauth2_token_url(), - GetValidTokenResponse("token", 3600), net::HTTP_OK, - net::URLRequestStatus::SUCCESS); + AddSuccessfulOAuhTokenResponse(); EXPECT_EQ(0, access_token_success_count_); EXPECT_EQ(0, access_token_failure_count_); @@ -863,7 +870,8 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, FetchPersistentError) { scope_list.push_back("scope"); std::unique_ptr fetcher( oauth2_service_delegate_->CreateAccessTokenFetcher( - kEmail, oauth2_service_delegate_->GetRequestContext(), this)); + kEmail, oauth2_service_delegate_->GetRequestContext(), + oauth2_service_delegate_->GetURLLoaderFactory(), this)); fetcher->Start("foo", "bar", scope_list); base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, access_token_success_count_); @@ -886,9 +894,7 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, RetryBackoff) { oauth2_service_delegate_->GetAuthError(kEmail)); // Create a "success" fetch we don't expect to get called just yet. - factory_.SetFakeResponse(GaiaUrls::GetInstance()->oauth2_token_url(), - GetValidTokenResponse("token", 3600), net::HTTP_OK, - net::URLRequestStatus::SUCCESS); + AddSuccessfulOAuhTokenResponse(); // Transient error will repeat until backoff period expires. EXPECT_EQ(0, access_token_success_count_); @@ -897,7 +903,8 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, RetryBackoff) { scope_list.push_back("scope"); std::unique_ptr fetcher1( oauth2_service_delegate_->CreateAccessTokenFetcher( - kEmail, oauth2_service_delegate_->GetRequestContext(), this)); + kEmail, oauth2_service_delegate_->GetRequestContext(), + oauth2_service_delegate_->GetURLLoaderFactory(), this)); fetcher1->Start("foo", "bar", scope_list); base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, access_token_success_count_); @@ -911,7 +918,8 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, RetryBackoff) { base::TimeTicks()); std::unique_ptr fetcher2( oauth2_service_delegate_->CreateAccessTokenFetcher( - kEmail, oauth2_service_delegate_->GetRequestContext(), this)); + kEmail, oauth2_service_delegate_->GetRequestContext(), + oauth2_service_delegate_->GetURLLoaderFactory(), this)); fetcher2->Start("foo", "bar", scope_list); base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, access_token_success_count_); @@ -934,9 +942,7 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, ResetBackoff) { oauth2_service_delegate_->GetAuthError(kEmail)); // Create a "success" fetch we don't expect to get called just yet. - factory_.SetFakeResponse(GaiaUrls::GetInstance()->oauth2_token_url(), - GetValidTokenResponse("token", 3600), net::HTTP_OK, - net::URLRequestStatus::SUCCESS); + AddSuccessfulOAuhTokenResponse(); // Transient error will repeat until backoff period expires. EXPECT_EQ(0, access_token_success_count_); @@ -945,7 +951,8 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, ResetBackoff) { scope_list.push_back("scope"); std::unique_ptr fetcher1( oauth2_service_delegate_->CreateAccessTokenFetcher( - kEmail, oauth2_service_delegate_->GetRequestContext(), this)); + kEmail, oauth2_service_delegate_->GetRequestContext(), + oauth2_service_delegate_->GetURLLoaderFactory(), this)); fetcher1->Start("foo", "bar", scope_list); base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, access_token_success_count_); @@ -956,7 +963,8 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, ResetBackoff) { net::NetworkChangeNotifier::CONNECTION_WIFI); std::unique_ptr fetcher2( oauth2_service_delegate_->CreateAccessTokenFetcher( - kEmail, oauth2_service_delegate_->GetRequestContext(), this)); + kEmail, oauth2_service_delegate_->GetRequestContext(), + oauth2_service_delegate_->GetURLLoaderFactory(), this)); fetcher2->Start("foo", "bar", scope_list); base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, access_token_success_count_); diff --git a/chrome/browser/signin/oauth2_token_service_delegate_android.cc b/chrome/browser/signin/oauth2_token_service_delegate_android.cc index 13815e7a14a4c9..992b64fcd68a87 100644 --- a/chrome/browser/signin/oauth2_token_service_delegate_android.cc +++ b/chrome/browser/signin/oauth2_token_service_delegate_android.cc @@ -261,6 +261,7 @@ OAuth2AccessTokenFetcher* OAuth2TokenServiceDelegateAndroid::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_factory, OAuth2AccessTokenConsumer* consumer) { DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::CreateAccessTokenFetcher" << " account= " << account_id; diff --git a/chrome/browser/signin/oauth2_token_service_delegate_android.h b/chrome/browser/signin/oauth2_token_service_delegate_android.h index b4f78f85ee53f4..5dd1f0890c5f94 100644 --- a/chrome/browser/signin/oauth2_token_service_delegate_android.h +++ b/chrome/browser/signin/oauth2_token_service_delegate_android.h @@ -99,6 +99,7 @@ class OAuth2TokenServiceDelegateAndroid : public OAuth2TokenServiceDelegate { OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_factory, OAuth2AccessTokenConsumer* consumer) override; // Overridden from OAuth2TokenService to intercept token fetch requests and diff --git a/chrome/browser/sync/test/integration/sync_auth_test.cc b/chrome/browser/sync/test/integration/sync_auth_test.cc index d234fa31261248..bd243ec5ecc9ee 100644 --- a/chrome/browser/sync/test/integration/sync_auth_test.cc +++ b/chrome/browser/sync/test/integration/sync_auth_test.cc @@ -21,32 +21,32 @@ using bookmarks_helper::AddURL; -const char kShortLivedOAuth2Token[] = - "{" - " \"refresh_token\": \"short_lived_refresh_token\"," - " \"access_token\": \"short_lived_access_token\"," - " \"expires_in\": 5," // 5 seconds. - " \"token_type\": \"Bearer\"" - "}"; +constexpr char kShortLivedOAuth2Token[] = R"( + { + "refresh_token": "short_lived_refresh_token", + "access_token": "short_lived_access_token", + "expires_in": 5, // 5 seconds. + "token_type": "Bearer" + })"; -const char kValidOAuth2Token[] = "{" - " \"refresh_token\": \"new_refresh_token\"," - " \"access_token\": \"new_access_token\"," - " \"expires_in\": 3600," // 1 hour. - " \"token_type\": \"Bearer\"" - "}"; +constexpr char kValidOAuth2Token[] = R"({ + "refresh_token": "new_refresh_token", + "access_token": "new_access_token", + "expires_in": 3600, // 1 hour. + "token_type": "Bearer" + })"; -const char kInvalidGrantOAuth2Token[] = "{" - " \"error\": \"invalid_grant\"" - "}"; +constexpr char kInvalidGrantOAuth2Token[] = R"({ + "error": "invalid_grant" + })"; -const char kInvalidClientOAuth2Token[] = "{" - " \"error\": \"invalid_client\"" - "}"; +constexpr char kInvalidClientOAuth2Token[] = R"({ + "error": "invalid_client" + })"; -const char kEmptyOAuth2Token[] = ""; +constexpr char kEmptyOAuth2Token[] = ""; -const char kMalformedOAuth2Token[] = "{ \"foo\": "; +constexpr char kMalformedOAuth2Token[] = R"({ "foo": )"; // Waits until local changes are committed or an auth error is encountered. class TestForAuthError : public UpdatedProgressMarkerChecker { @@ -106,7 +106,6 @@ class SyncAuthTest : public SyncTest { set_max_authorization_token_fetch_retries_for_testing(0); } - private: int GetNextBookmarkIndex() { return bookmark_index_++; diff --git a/chrome/browser/sync/test/integration/sync_test.cc b/chrome/browser/sync/test/integration/sync_test.cc index d0ba2b5d8da1f6..4a05d96ae2f61f 100644 --- a/chrome/browser/sync/test/integration/sync_test.cc +++ b/chrome/browser/sync/test/integration/sync_test.cc @@ -39,6 +39,7 @@ #include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profiles_state.h" #include "chrome/browser/search_engines/template_url_service_factory.h" +#include "chrome/browser/signin/chrome_signin_client_factory.h" #include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/sync/test/integration/fake_server_invalidation_service.h" #include "chrome/browser/sync/test/integration/p2p_invalidation_forwarder.h" @@ -87,9 +88,9 @@ #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher.h" -#include "net/url_request/url_fetcher_delegate.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/weak_wrapper_shared_url_loader_factory.h" #include "url/gurl.h" #if defined(OS_CHROMEOS) @@ -198,7 +199,10 @@ SyncTest::SyncTest(TestType test_type) previous_profile_(nullptr), num_clients_(-1), use_verifier_(true), - create_gaia_account_at_runtime_(false) { + create_gaia_account_at_runtime_(false), + test_shared_url_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)) { sync_datatype_helper::AssociateWithTest(this); switch (test_type_) { case SINGLE_CLIENT: @@ -274,6 +278,7 @@ void SyncTest::TearDown() { // Return OSCrypt to its real behaviour OSCryptMocker::TearDown(); + test_shared_url_loader_factory_->Detach(); fake_server_.reset(); } @@ -359,7 +364,10 @@ bool SyncTest::CreateProfile(int index) { profile_delegates_[index] = std::make_unique(base::Bind( &SyncTest::InitializeProfile, base::Unretained(this), index)); - MakeTestProfile(profile_path, index); + Profile* profile = MakeTestProfile(profile_path, index); + ChromeSigninClient* signin_client = static_cast( + ChromeSigninClientFactory::GetForProfile(profile)); + signin_client->SetURLLoaderFactoryForTest(test_shared_url_loader_factory_); } // Once profile initialization has kicked off, wait for it to finish. @@ -624,6 +632,12 @@ void SyncTest::DisableNotificationsForClient(int index) { fake_server_->RemoveObserver(fake_server_invalidation_services_[index]); } +void SyncTest::SetupMockGaiaResponsesForProfile(Profile* profile) { + ChromeSigninClient* signin_client = static_cast( + ChromeSigninClientFactory::GetForProfile(profile)); + signin_client->SetURLLoaderFactoryForTest(test_shared_url_loader_factory_); +} + void SyncTest::InitializeInvalidations(int index) { configuration_refresher_ = std::make_unique(); if (UsingExternalServers()) { @@ -798,6 +812,9 @@ void SyncTest::TearDownOnMainThread() { } void SyncTest::SetUpOnMainThread() { + if (!UsingExternalServers()) + SetupMockGaiaResponsesForProfile(ProfileManager::GetActiveUserProfile()); + // Allows google.com as well as country-specific TLDs. host_resolver()->AllowDirectLookup("*.google.com"); host_resolver()->AllowDirectLookup("accounts.google.*"); @@ -845,74 +862,55 @@ void SyncTest::ReadPasswordFile() { } void SyncTest::SetupMockGaiaResponses() { - factory_ = std::make_unique(); - fake_factory_ = std::make_unique(factory_.get()); - fake_factory_->SetFakeResponse( - GaiaUrls::GetInstance()->get_user_info_url(), - "email=user@gmail.com\ndisplayEmail=user@gmail.com", - net::HTTP_OK, - net::URLRequestStatus::SUCCESS); - fake_factory_->SetFakeResponse( - GaiaUrls::GetInstance()->issue_auth_token_url(), - "auth", - net::HTTP_OK, - net::URLRequestStatus::SUCCESS); - fake_factory_->SetFakeResponse( - GURL(GoogleURLTracker::kSearchDomainCheckURL), - ".google.com", - net::HTTP_OK, - net::URLRequestStatus::SUCCESS); - fake_factory_->SetFakeResponse( - GaiaUrls::GetInstance()->deprecated_client_login_to_oauth2_url(), - "some_response", net::HTTP_OK, net::URLRequestStatus::SUCCESS); - fake_factory_->SetFakeResponse( - GaiaUrls::GetInstance()->oauth2_token_url(), - "{" - " \"refresh_token\": \"rt1\"," - " \"access_token\": \"at1\"," - " \"expires_in\": 3600," - " \"token_type\": \"Bearer\"" - "}", - net::HTTP_OK, - net::URLRequestStatus::SUCCESS); - fake_factory_->SetFakeResponse( - GaiaUrls::GetInstance()->oauth_user_info_url(), - "{" - " \"id\": \"12345\"" - "}", - net::HTTP_OK, - net::URLRequestStatus::SUCCESS); - fake_factory_->SetFakeResponse( - GaiaUrls::GetInstance()->oauth1_login_url(), - "SID=sid\nLSID=lsid\nAuth=auth_token", - net::HTTP_OK, - net::URLRequestStatus::SUCCESS); - fake_factory_->SetFakeResponse( - GaiaUrls::GetInstance()->oauth2_revoke_url(), - "", - net::HTTP_OK, - net::URLRequestStatus::SUCCESS); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->get_user_info_url().spec(), + "email=user@gmail.com\ndisplayEmail=user@gmail.com"); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->issue_auth_token_url().spec(), "auth"); + test_url_loader_factory_.AddResponse(GoogleURLTracker::kSearchDomainCheckURL, + ".google.com"); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->deprecated_client_login_to_oauth2_url().spec(), + "some_response"); + + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url().spec(), + R"({ + "refresh_token": "rt1", + "access_token": "at1", + "expires_in": 3600, + "token_type": "Bearer" + })"); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth_user_info_url().spec(), + "{ \"id\": \"12345\" }"); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth1_login_url().spec(), + "SID=sid\nLSID=lsid\nAuth=auth_token"); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth2_revoke_url().spec(), ""); } void SyncTest::SetOAuth2TokenResponse(const std::string& response_data, - net::HttpStatusCode response_code, + net::HttpStatusCode status_code, net::URLRequestStatus::Status status) { - ASSERT_NE(nullptr, fake_factory_.get()); - fake_factory_->SetFakeResponse(GaiaUrls::GetInstance()->oauth2_token_url(), - response_data, response_code, status); + network::URLLoaderCompletionStatus completion_status(status); + completion_status.decoded_body_length = response_data.size(); + + std::string response = base::StringPrintf("HTTP/1.1 %d %s\r\n", status_code, + GetHttpReasonPhrase(status_code)); + network::ResourceResponseHead resource_response; + resource_response.headers = + base::MakeRefCounted(response); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url(), resource_response, + response_data, completion_status); + base::RunLoop().RunUntilIdle(); } void SyncTest::ClearMockGaiaResponses() { // Clear any mock gaia responses that might have been set. - if (fake_factory_) { - fake_factory_->ClearFakeResponses(); - fake_factory_.reset(); - } - - // Cancel any outstanding URL fetches and destroy the URLFetcherImplFactory we - // created. - net::URLFetcher::CancelAll(); - factory_.reset(); + test_url_loader_factory_.ClearResponses(); } void SyncTest::DecideServerType() { diff --git a/chrome/browser/sync/test/integration/sync_test.h b/chrome/browser/sync/test/integration/sync_test.h index ea88d953c29401..c3d32bb3c6c298 100644 --- a/chrome/browser/sync/test/integration/sync_test.h +++ b/chrome/browser/sync/test/integration/sync_test.h @@ -25,6 +25,7 @@ #include "components/sync/test/local_sync_test_server.h" #include "net/http/http_status_code.h" #include "net/url_request/url_request_status.h" +#include "services/network/test/test_url_loader_factory.h" #if BUILDFLAG(ENABLE_APP_LIST) #include "chrome/browser/ui/app_list/app_list_syncable_service.h" @@ -73,6 +74,10 @@ class FakeURLFetcherFactory; class URLFetcherImplFactory; } // namespace net +namespace network { +class WeakWrapperSharedURLLoaderFactory; +} // namespace network + // This is the base class for integration tests for all sync data types. Derived // classes must be defined for each sync data type. Individual tests are defined // using the IN_PROC_BROWSER_TEST_F macro. @@ -287,6 +292,11 @@ class SyncTest : public InProcessBrowserTest { // Stops notificatinos being sent to a client. void DisableNotificationsForClient(int index); + // Sets up fake responses for kClientLoginUrl, kIssueAuthTokenUrl, + // kGetUserInfoUrl and kSearchDomainCheckUrl in order to mock out calls to + // GAIA servers. + void SetupMockGaiaResponsesForProfile(Profile* profile); + base::test::ScopedFeatureList feature_list_; // GAIA account used by the test case. @@ -463,6 +473,13 @@ class SyncTest : public InProcessBrowserTest { // Used to start and stop the local test server. base::Process test_server_; + // The factory used to mock out GAIA signin. + network::TestURLLoaderFactory test_url_loader_factory_; + + // The shared URLLoaderFactory backed by |test_url_loader_factory_|. + scoped_refptr + test_shared_url_loader_factory_; + // Fake URLFetcher factory used to mock out GAIA signin. std::unique_ptr fake_factory_; diff --git a/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc b/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc index 7ccd355f3d497f..c499d7ee3a8a4a 100644 --- a/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc +++ b/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc @@ -124,6 +124,7 @@ class DriveBackendSyncTest : public testing::Test, nullptr, // signin_manager nullptr, // token_service nullptr, // request_context + nullptr, // url_loader_factory nullptr, // drive_service in_memory_env_.get())); remote_sync_service_->AddServiceObserver(this); diff --git a/chrome/browser/sync_file_system/drive_backend/sync_engine.cc b/chrome/browser/sync_file_system/drive_backend/sync_engine.cc index 1ad7d997cc62fc..a7cd7751b65dc1 100644 --- a/chrome/browser/sync_file_system/drive_backend/sync_engine.cc +++ b/chrome/browser/sync_file_system/drive_backend/sync_engine.cc @@ -98,10 +98,12 @@ std::unique_ptr SyncEngine::DriveServiceFactory::CreateDriveService( OAuth2TokenService* oauth2_token_service, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, base::SequencedTaskRunner* blocking_task_runner) { return std::unique_ptr< drive::DriveServiceInterface>(new drive::DriveAPIService( - oauth2_token_service, url_request_context_getter, blocking_task_runner, + oauth2_token_service, url_request_context_getter, url_loader_factory, + blocking_task_runner, GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction), GURL(google_apis::DriveApiUrlGenerator::kBaseThumbnailUrlForProduction), std::string(), /* custom_user_agent */ @@ -218,13 +220,16 @@ std::unique_ptr SyncEngine::CreateForBrowserContext( scoped_refptr request_context = content::BrowserContext::GetDefaultStoragePartition(context)-> GetURLRequestContext(); + scoped_refptr url_loader_factory = + content::BrowserContext::GetDefaultStoragePartition(context) + ->GetURLLoaderFactoryForBrowserProcess(); std::unique_ptr sync_engine(new SyncEngine( ui_task_runner.get(), worker_task_runner.get(), drive_task_runner.get(), GetSyncFileSystemDir(context->GetPath()), task_logger, notification_manager, extension_service, signin_manager, token_service, - request_context.get(), std::make_unique(), - nullptr /* env_override */)); + request_context.get(), url_loader_factory, + std::make_unique(), nullptr /* env_override */)); sync_engine->Initialize(); return sync_engine; @@ -277,7 +282,8 @@ void SyncEngine::Initialize() { DCHECK(drive_service_factory_); std::unique_ptr drive_service = drive_service_factory_->CreateDriveService( - token_service_, request_context_.get(), drive_task_runner_.get()); + token_service_, request_context_.get(), url_loader_factory_, + drive_task_runner_.get()); device::mojom::WakeLockProviderPtr wake_lock_provider(nullptr); DCHECK(content::ServiceManagerConnection::GetForProcess()); @@ -725,6 +731,7 @@ SyncEngine::SyncEngine( SigninManagerBase* signin_manager, OAuth2TokenService* token_service, net::URLRequestContextGetter* request_context, + scoped_refptr url_loader_factory, std::unique_ptr drive_service_factory, leveldb::Env* env_override) : ui_task_runner_(ui_task_runner), @@ -737,6 +744,7 @@ SyncEngine::SyncEngine( signin_manager_(signin_manager), token_service_(token_service), request_context_(request_context), + url_loader_factory_(url_loader_factory), drive_service_factory_(std::move(drive_service_factory)), remote_change_processor_(nullptr), service_state_(REMOTE_SERVICE_TEMPORARY_UNAVAILABLE), diff --git a/chrome/browser/sync_file_system/drive_backend/sync_engine.h b/chrome/browser/sync_file_system/drive_backend/sync_engine.h index 9859fad6dc45bc..1acf17f6884032 100644 --- a/chrome/browser/sync_file_system/drive_backend/sync_engine.h +++ b/chrome/browser/sync_file_system/drive_backend/sync_engine.h @@ -45,6 +45,9 @@ class Env; namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace sync_file_system { @@ -75,6 +78,7 @@ class SyncEngine : public RemoteFileSyncService, virtual std::unique_ptr CreateDriveService( OAuth2TokenService* oauth2_token_service, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, base::SequencedTaskRunner* blocking_task_runner); private: @@ -167,6 +171,7 @@ class SyncEngine : public RemoteFileSyncService, SigninManagerBase* signin_manager, OAuth2TokenService* token_service, net::URLRequestContextGetter* request_context, + scoped_refptr url_loader_factory, std::unique_ptr drive_service_factory, leveldb::Env* env_override); @@ -199,6 +204,7 @@ class SyncEngine : public RemoteFileSyncService, OAuth2TokenService* token_service_; scoped_refptr request_context_; + scoped_refptr url_loader_factory_; std::unique_ptr drive_service_factory_; diff --git a/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc b/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc index 57020f9db313a2..1cd0d8274b53da 100644 --- a/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc +++ b/chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc @@ -23,6 +23,7 @@ #include "content/public/browser/browser_thread.h" #include "content/public/test/test_browser_thread_bundle.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "testing/gtest/include/gtest/gtest.h" namespace sync_file_system { @@ -57,6 +58,7 @@ class SyncEngineTest : public testing::Test, nullptr, // signin_manager nullptr, // token_service nullptr, // request_context + nullptr, // url_loader_factory nullptr, // drive_service_factory nullptr)); // in_memory_env diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc index ee2e5e439cd84f..bf7f2db196d4fe 100644 --- a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc +++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc @@ -43,6 +43,7 @@ #include "components/signin/core/browser/signin_manager.h" #include "components/signin/core/browser/signin_metrics.h" #include "components/sync/base/sync_prefs.h" +#include "content/public/browser/storage_partition.h" #include "net/base/url_util.h" #include "net/url_request/url_request_context_getter.h" #include "ui/base/l10n/l10n_util.h" @@ -266,6 +267,8 @@ void OneClickSigninSyncStarter::LoadPolicyWithCachedCredentials() { : AccountId::FromUserEmailGaiaId(username, gaia_id); policy_service->FetchPolicyForSignedInUser( account_id, dm_token_, client_id_, profile_->GetRequestContext(), + content::BrowserContext::GetDefaultStoragePartition(profile_) + ->GetURLLoaderFactoryForBrowserProcess(), base::Bind(&OneClickSigninSyncStarter::OnPolicyFetchComplete, weak_pointer_factory_.GetWeakPtr())); } diff --git a/chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc b/chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc index 90ee886c403f9d..2488a4195d151d 100644 --- a/chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc +++ b/chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc @@ -12,6 +12,7 @@ #include "base/compiler_specific.h" #include "base/location.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/run_loop.h" #include "base/single_thread_task_runner.h" #include "base/threading/thread_task_runner_handle.h" @@ -20,6 +21,8 @@ #include "chrome/browser/media/router/providers/cast/dual_media_sink_service.h" #include "chrome/browser/media/router/test/noop_dual_media_sink_service.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/signin/chrome_signin_client_factory.h" #include "chrome/browser/signin/identity_manager_factory.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" #include "chrome/browser/signin/signin_manager_factory.h" @@ -36,6 +39,9 @@ #include "net/url_request/url_request_status.h" #include "net/url_request/url_request_test_util.h" #include "services/identity/public/cpp/identity_test_utils.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" #if defined(OS_CHROMEOS) #include "chrome/common/pref_names.h" @@ -347,10 +353,12 @@ class MockableFakeURLFetcherCreator { class LocalDiscoveryUITest : public WebUIBrowserTest { public: - LocalDiscoveryUITest() : fake_fetcher_factory_( - &fetcher_impl_factory_, - fake_url_fetcher_creator_.callback()) { - } + LocalDiscoveryUITest() + : test_shared_loader_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)), + fake_fetcher_factory_(&fetcher_impl_factory_, + fake_url_fetcher_creator_.callback()) {} ~LocalDiscoveryUITest() override { } @@ -377,6 +385,11 @@ class LocalDiscoveryUITest : public WebUIBrowserTest { &TestMessageLoopCondition::Signal)) .WillRepeatedly(Return()); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url().spec(), kResponseGaiaToken); + test_url_loader_factory_.AddResponse( + GaiaUrls::GetInstance()->oauth_user_info_url().spec(), kResponseGaiaId); + fake_fetcher_factory().SetFakeResponse( GURL(kURLInfo), kResponseInfo, @@ -433,10 +446,16 @@ class LocalDiscoveryUITest : public WebUIBrowserTest { kSampleUser); AddLibrary(base::FilePath(FILE_PATH_LITERAL("local_discovery_ui_test.js"))); + + ChromeSigninClient* signin_client = static_cast( + ChromeSigninClientFactory::GetForProfile( + ProfileManager::GetActiveUserProfile())); + signin_client->SetURLLoaderFactoryForTest(test_shared_loader_factory_); } void TearDownOnMainThread() override { test_service_discovery_client_ = nullptr; + test_shared_loader_factory_->Detach(); WebUIBrowserTest::TearDownOnMainThread(); } @@ -483,6 +502,10 @@ class LocalDiscoveryUITest : public WebUIBrowserTest { scoped_refptr test_service_discovery_client_; TestMessageLoopCondition condition_devices_listed_; + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr + test_shared_loader_factory_; + net::URLFetcherImplFactory fetcher_impl_factory_; StrictMock fake_url_fetcher_creator_; net::FakeURLFetcherFactory fake_fetcher_factory_; @@ -521,8 +544,7 @@ IN_PROC_BROWSER_TEST_F(LocalDiscoveryUITest, AddRowTest) { IN_PROC_BROWSER_TEST_F(LocalDiscoveryUITest, RegisterTest) { TestMessageLoopCondition condition_token_claimed; - ui_test_utils::NavigateToURL(browser(), GURL( - chrome::kChromeUIDevicesURL)); + ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIDevicesURL)); condition_devices_listed().Wait(); test_service_discovery_client()->SimulateReceive( diff --git a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc index 3773b85e3c878c..63cc6a71dd4b10 100644 --- a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc +++ b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc @@ -35,6 +35,7 @@ #include "components/signin/core/browser/signin_metrics.h" #include "components/signin/core/browser/signin_pref_names.h" #include "components/sync/base/sync_prefs.h" +#include "content/public/browser/storage_partition.h" #include "net/url_request/url_request_context_getter.h" namespace { @@ -253,6 +254,8 @@ void DiceTurnSyncOnHelper::LoadPolicyWithCachedCredentials() { policy_service->FetchPolicyForSignedInUser( account_info_.GetAccountId(), dm_token_, client_id_, profile_->GetRequestContext(), + content::BrowserContext::GetDefaultStoragePartition(profile_) + ->GetURLLoaderFactoryForBrowserProcess(), base::Bind(&DiceTurnSyncOnHelper::OnPolicyFetchComplete, weak_pointer_factory_.GetWeakPtr())); } diff --git a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc index 11c0e0234ac1d9..3a8c253005f9a9 100644 --- a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc +++ b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc @@ -39,6 +39,7 @@ #include "content/public/test/test_browser_thread_bundle.h" #include "google_apis/gaia/google_service_auth_error.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "testing/gtest/include/gtest/gtest.h" using ::testing::AtLeast; @@ -133,6 +134,7 @@ class FakeUserPolicySigninService : public policy::UserPolicySigninService { nullptr, signin_manager, nullptr, + nullptr, oauth2_token_service) {} void set_dm_token(const std::string& dm_token) { dm_token_ = dm_token; } @@ -158,6 +160,7 @@ class FakeUserPolicySigninService : public policy::UserPolicySigninService { const std::string& dm_token, const std::string& client_id, scoped_refptr profile_request_context, + scoped_refptr test_shared_loader_factory, const PolicyFetchCallback& callback) override { callback.Run(true); } diff --git a/chromeos/BUILD.gn b/chromeos/BUILD.gn index 7ff4fdf4f74c20..9d7730d8c60159 100644 --- a/chromeos/BUILD.gn +++ b/chromeos/BUILD.gn @@ -51,6 +51,7 @@ component("chromeos") { "//google_apis", "//media/base:video_facing", "//net", + "//services/network/public/cpp", "//skia", # For components/user_manager "//third_party/icu", "//third_party/protobuf:protobuf_lite", diff --git a/chromeos/DEPS b/chromeos/DEPS index 2043a51427e255..a5f3f900137c71 100644 --- a/chromeos/DEPS +++ b/chromeos/DEPS @@ -12,6 +12,7 @@ include_rules = [ "+google_apis/gaia", "+media/base/video_facing.h", "+net", + "+services/network/public", "+third_party/cros_system_api", "+third_party/protobuf", diff --git a/chromeos/account_manager/account_manager.cc b/chromeos/account_manager/account_manager.cc index 1fb4eec6a0c73e..c2945833e6ce14 100644 --- a/chromeos/account_manager/account_manager.cc +++ b/chromeos/account_manager/account_manager.cc @@ -21,6 +21,7 @@ #include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #include "third_party/protobuf/src/google/protobuf/message_lite.h" namespace chromeos { @@ -355,7 +356,7 @@ net::URLRequestContextGetter* AccountManager::GetUrlRequestContext() { std::unique_ptr AccountManager::CreateAccessTokenFetcher( const AccountKey& account_key, - net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -364,8 +365,8 @@ AccountManager::CreateAccessTokenFetcher( return nullptr; } - return std::make_unique(consumer, getter, - it->second); + return std::make_unique( + consumer, url_loader_factory, it->second); } bool AccountManager::IsTokenAvailable(const AccountKey& account_key) const { diff --git a/chromeos/account_manager/account_manager.h b/chromeos/account_manager/account_manager.h index d5fcf4cf1a2a96..1cd8aafe5f0179 100644 --- a/chromeos/account_manager/account_manager.h +++ b/chromeos/account_manager/account_manager.h @@ -33,7 +33,11 @@ class ImportantFileWriter; namespace net { class URLRequestContextGetter; -} // namespace net +} + +namespace network { +class SharedURLLoaderFactory; +} namespace chromeos { @@ -132,7 +136,7 @@ class CHROMEOS_EXPORT AccountManager { // |account_key|, otherwise a |nullptr| is returned. std::unique_ptr CreateAccessTokenFetcher( const AccountKey& account_key, - net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) const; // Returns |true| if an LST is available for |account_key|. diff --git a/components/drive/DEPS b/components/drive/DEPS index 0341d6a2ba19e8..effd9116a97876 100644 --- a/components/drive/DEPS +++ b/components/drive/DEPS @@ -9,6 +9,7 @@ include_rules = [ "+services/device/public/mojom", "+third_party/leveldatabase", "+third_party/re2", + "+services/network/public/cpp", ] specific_include_rules = { diff --git a/components/drive/service/drive_api_service.cc b/components/drive/service/drive_api_service.cc index e5f65482e193d3..b00195c7d03afe 100644 --- a/components/drive/service/drive_api_service.cc +++ b/components/drive/service/drive_api_service.cc @@ -23,6 +23,7 @@ #include "google_apis/drive/request_sender.h" #include "google_apis/google_api_keys.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" using google_apis::AboutResourceCallback; using google_apis::AppList; @@ -267,6 +268,7 @@ void BatchRequestConfigurator::Commit() { DriveAPIService::DriveAPIService( OAuth2TokenService* oauth2_token_service, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, base::SequencedTaskRunner* blocking_task_runner, const GURL& base_url, const GURL& base_thumbnail_url, @@ -274,6 +276,7 @@ DriveAPIService::DriveAPIService( const net::NetworkTrafficAnnotationTag& traffic_annotation) : oauth2_token_service_(oauth2_token_service), url_request_context_getter_(url_request_context_getter), + url_loader_factory_(url_loader_factory), blocking_task_runner_(blocking_task_runner), url_generator_(base_url, base_thumbnail_url, @@ -302,7 +305,8 @@ void DriveAPIService::Initialize(const std::string& account_id) { sender_ = std::make_unique( new google_apis::AuthService(oauth2_token_service_, account_id, - url_request_context_getter_.get(), scopes), + url_request_context_getter_.get(), + url_loader_factory_, scopes), url_request_context_getter_.get(), blocking_task_runner_.get(), custom_user_agent_, traffic_annotation_); sender_->auth_service()->AddObserver(this); diff --git a/components/drive/service/drive_api_service.h b/components/drive/service/drive_api_service.h index 6a63be3fcf9131..3ec81ee89a3eac 100644 --- a/components/drive/service/drive_api_service.h +++ b/components/drive/service/drive_api_service.h @@ -41,6 +41,9 @@ class BatchUploadRequest; namespace net { class URLRequestContextGetter; } // namespace net +namespace network { +class SharedURLLoaderFactory; +} namespace drive { @@ -95,6 +98,8 @@ class DriveAPIService : public DriveServiceInterface, public: // |oauth2_token_service| is used for obtaining OAuth2 access tokens. // |url_request_context_getter| is used to initialize URLFetcher. + // |url_loader_factory| is used to create SimpleURLLoaders used to create + // OAuth tokens. // |blocking_task_runner| is used to run blocking tasks (like parsing JSON). // |base_url| is used to generate URLs for communication with the drive API. // |base_thumbnail_url| is used to generate URLs for downloading thumbnail @@ -103,13 +108,15 @@ class DriveAPIService : public DriveServiceInterface, // requests issues through the service if the value is not empty. // |traffic_annotation| will be used to annotate the network request that will // be created to perform this service. - DriveAPIService(OAuth2TokenService* oauth2_token_service, - net::URLRequestContextGetter* url_request_context_getter, - base::SequencedTaskRunner* blocking_task_runner, - const GURL& base_url, - const GURL& base_thumbnail_url, - const std::string& custom_user_agent, - const net::NetworkTrafficAnnotationTag& traffic_annotation); + DriveAPIService( + OAuth2TokenService* oauth2_token_service, + net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, + base::SequencedTaskRunner* blocking_task_runner, + const GURL& base_url, + const GURL& base_thumbnail_url, + const std::string& custom_user_agent, + const net::NetworkTrafficAnnotationTag& traffic_annotation); ~DriveAPIService() override; // DriveServiceInterface Overrides @@ -276,6 +283,7 @@ class DriveAPIService : public DriveServiceInterface, OAuth2TokenService* oauth2_token_service_; scoped_refptr url_request_context_getter_; + scoped_refptr url_loader_factory_; scoped_refptr blocking_task_runner_; std::unique_ptr sender_; std::unique_ptr diff --git a/components/policy/core/DEPS b/components/policy/core/DEPS index bf2973b36489bc..2afec685ef7655 100644 --- a/components/policy/core/DEPS +++ b/components/policy/core/DEPS @@ -6,4 +6,5 @@ include_rules = [ "+google_apis", "+net", "+third_party/re2", + "+services/network/public/cpp", ] diff --git a/components/policy/core/browser/browser_policy_connector.h b/components/policy/core/browser/browser_policy_connector.h index 158376a2162a44..d7e189c19e3e2e 100644 --- a/components/policy/core/browser/browser_policy_connector.h +++ b/components/policy/core/browser/browser_policy_connector.h @@ -10,6 +10,7 @@ #include #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "components/policy/core/browser/browser_policy_connector_base.h" #include "components/policy/policy_export.h" @@ -20,6 +21,10 @@ namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} + namespace policy { class DeviceManagementService; @@ -36,7 +41,8 @@ class POLICY_EXPORT BrowserPolicyConnector : public BrowserPolicyConnectorBase { // tests that don't require the full policy system running. virtual void Init( PrefService* local_state, - scoped_refptr request_context) = 0; + scoped_refptr request_context, + scoped_refptr url_loader_factory) = 0; // Checks whether this device is under any kind of enterprise management. virtual bool IsEnterpriseManaged() const = 0; diff --git a/components/policy/core/common/BUILD.gn b/components/policy/core/common/BUILD.gn index 922f57ef2615db..7fbc0f124ee783 100644 --- a/components/policy/core/common/BUILD.gn +++ b/components/policy/core/common/BUILD.gn @@ -158,6 +158,7 @@ source_set("internal") { "//extensions/buildflags", "//google_apis", "//net", + "//services/network/public/cpp", "//third_party/libxml", "//third_party/re2", "//url", @@ -289,6 +290,7 @@ static_library("test_support") { "//components/policy/proto", "//crypto", "//net", + "//services/network/public/cpp", "//testing/gmock", "//testing/gtest", ] diff --git a/components/policy/core/common/cloud/cloud_policy_client.cc b/components/policy/core/common/cloud/cloud_policy_client.cc index 4ac1d0058236dd..be4700ebb0bef8 100644 --- a/components/policy/core/common/cloud/cloud_policy_client.cc +++ b/components/policy/core/common/cloud/cloud_policy_client.cc @@ -21,6 +21,7 @@ #include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/gaia_urls.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace em = enterprise_management; @@ -100,6 +101,7 @@ CloudPolicyClient::CloudPolicyClient( const std::string& brand_code, DeviceManagementService* service, scoped_refptr request_context, + scoped_refptr url_loader_factory, SigningService* signing_service, DeviceDMTokenCallback device_dm_token_callback) : machine_id_(machine_id), @@ -109,6 +111,7 @@ CloudPolicyClient::CloudPolicyClient( signing_service_(signing_service), device_dm_token_callback_(device_dm_token_callback), request_context_(request_context), + url_loader_factory_(url_loader_factory), weak_ptr_factory_(this) {} CloudPolicyClient::~CloudPolicyClient() { @@ -682,6 +685,11 @@ CloudPolicyClient::GetRequestContext() { return request_context_; } +scoped_refptr +CloudPolicyClient::GetURLLoaderFactory() { + return url_loader_factory_; +} + int CloudPolicyClient::GetActiveRequestCountForTest() const { return request_jobs_.size(); } diff --git a/components/policy/core/common/cloud/cloud_policy_client.h b/components/policy/core/common/cloud/cloud_policy_client.h index 129becb08b7023..7562e66966cdb0 100644 --- a/components/policy/core/common/cloud/cloud_policy_client.h +++ b/components/policy/core/common/cloud/cloud_policy_client.h @@ -28,6 +28,9 @@ namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace policy { @@ -104,13 +107,15 @@ class POLICY_EXPORT CloudPolicyClient { // requests. |device_dm_token_callback| is used to retrieve device DMToken for // affiliated users. Could be null if it's not possible to use // device DMToken for user policy fetches. - CloudPolicyClient(const std::string& machine_id, - const std::string& machine_model, - const std::string& brand_code, - DeviceManagementService* service, - scoped_refptr request_context, - SigningService* signing_service, - DeviceDMTokenCallback device_dm_token_callback); + CloudPolicyClient( + const std::string& machine_id, + const std::string& machine_model, + const std::string& brand_code, + DeviceManagementService* service, + scoped_refptr request_context, + scoped_refptr url_loader_factory, + SigningService* signing_service, + DeviceDMTokenCallback device_dm_token_callback); virtual ~CloudPolicyClient(); // Sets the DMToken, thereby establishing a registration with the server. A @@ -349,6 +354,8 @@ class POLICY_EXPORT CloudPolicyClient { scoped_refptr GetRequestContext(); + scoped_refptr GetURLLoaderFactory(); + // Returns the number of active requests. int GetActiveRequestCountForTest() const; @@ -513,7 +520,9 @@ class POLICY_EXPORT CloudPolicyClient { DeviceDMTokenCallback device_dm_token_callback_; base::ObserverList observers_; + scoped_refptr request_context_; + scoped_refptr url_loader_factory_; private: void SetClientId(const std::string& client_id); diff --git a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc index e497788b75849d..ebc8e2685cf402 100644 --- a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc +++ b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc @@ -15,6 +15,7 @@ #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_token_service.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #if !defined(OS_ANDROID) #include "google_apis/gaia/oauth2_access_token_consumer.h" @@ -102,9 +103,11 @@ class CloudPolicyClientRegistrationHelper::LoginTokenHelper public: LoginTokenHelper(); - void FetchAccessToken(const std::string& login_refresh_token, - net::URLRequestContextGetter* context, - const StringCallback& callback); + void FetchAccessToken( + const std::string& login_refresh_token, + net::URLRequestContextGetter* context, + scoped_refptr url_loader_factory, + const StringCallback& callback); private: // OAuth2AccessTokenConsumer implementation: @@ -121,14 +124,15 @@ CloudPolicyClientRegistrationHelper::LoginTokenHelper::LoginTokenHelper() {} void CloudPolicyClientRegistrationHelper::LoginTokenHelper::FetchAccessToken( const std::string& login_refresh_token, net::URLRequestContextGetter* context, + scoped_refptr url_loader_factory, const StringCallback& callback) { DCHECK(!oauth2_access_token_fetcher_); callback_ = callback; // Start fetching an OAuth2 access token for the device management and // userinfo services. - oauth2_access_token_fetcher_.reset( - new OAuth2AccessTokenFetcherImpl(this, context, login_refresh_token)); + oauth2_access_token_fetcher_.reset(new OAuth2AccessTokenFetcherImpl( + this, url_loader_factory, login_refresh_token)); GaiaUrls* gaia_urls = GaiaUrls::GetInstance(); oauth2_access_token_fetcher_->Start( gaia_urls->oauth2_chrome_client_id(), @@ -153,9 +157,11 @@ CloudPolicyClientRegistrationHelper::CloudPolicyClientRegistrationHelper( CloudPolicyClient* client, enterprise_management::DeviceRegisterRequest::Type registration_type) : context_(client->GetRequestContext()), + url_loader_factory_(client->GetURLLoaderFactory()), client_(client), registration_type_(registration_type) { DCHECK(context_); + DCHECK(url_loader_factory_); DCHECK(client_); } @@ -207,8 +213,7 @@ void CloudPolicyClientRegistrationHelper::StartRegistrationWithLoginToken( login_token_helper_.reset( new CloudPolicyClientRegistrationHelper::LoginTokenHelper()); login_token_helper_->FetchAccessToken( - login_refresh_token, - context_.get(), + login_refresh_token, context_.get(), url_loader_factory_, base::Bind(&CloudPolicyClientRegistrationHelper::OnTokenFetched, base::Unretained(this))); } diff --git a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.h b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.h index 517c35d384e314..6301ca19b86dd1 100644 --- a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.h +++ b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.h @@ -25,6 +25,10 @@ namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} + namespace policy { // Helper class that registers a CloudPolicyClient. It fetches an OAuth2 token @@ -108,6 +112,7 @@ class POLICY_EXPORT CloudPolicyClientRegistrationHelper std::string oauth_access_token_; scoped_refptr context_; + scoped_refptr url_loader_factory_; CloudPolicyClient* client_; enterprise_management::DeviceRegisterRequest::Type registration_type_; base::Closure callback_; diff --git a/components/policy/core/common/cloud/cloud_policy_client_unittest.cc b/components/policy/core/common/cloud/cloud_policy_client_unittest.cc index bb9e3d1d83984e..48adb8a67fd709 100644 --- a/components/policy/core/common/cloud/cloud_policy_client_unittest.cc +++ b/components/policy/core/common/cloud/cloud_policy_client_unittest.cc @@ -25,6 +25,7 @@ #include "components/policy/proto/device_management_backend.pb.h" #include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_test_util.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" @@ -270,7 +271,7 @@ class CloudPolicyClientTest : public testing::Test { new net::TestURLRequestContextGetter(loop_.task_runner()); client_ = std::make_unique( kMachineID, kMachineModel, kBrandCode, &service_, request_context_, - &fake_signing_service_, + /*url_loader_factory_*/ nullptr, &fake_signing_service_, base::BindRepeating( &MockDeviceDMTokenCallbackObserver::OnDeviceDMTokenRequested, base::Unretained(&device_dmtoken_callback_observer_))); diff --git a/components/policy/core/common/cloud/mock_cloud_policy_client.cc b/components/policy/core/common/cloud/mock_cloud_policy_client.cc index 5f41eb4ddb8dee..22d1d6eb80e353 100644 --- a/components/policy/core/common/cloud/mock_cloud_policy_client.cc +++ b/components/policy/core/common/cloud/mock_cloud_policy_client.cc @@ -8,6 +8,7 @@ #include "components/policy/core/common/cloud/mock_cloud_policy_client.h" #include "components/policy/proto/device_management_backend.pb.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace em = enterprise_management; @@ -19,6 +20,7 @@ MockCloudPolicyClient::MockCloudPolicyClient() std::string() /* brand_code */, nullptr /* service */, nullptr /* request_context */, + nullptr /* url_loader_factory */, nullptr /* signing_service */, CloudPolicyClient::DeviceDMTokenCallback()) {} diff --git a/components/policy/core/common/cloud/user_cloud_policy_manager.cc b/components/policy/core/common/cloud/user_cloud_policy_manager.cc index 47232792e12fa1..868f03e11b8718 100644 --- a/components/policy/core/common/cloud/user_cloud_policy_manager.cc +++ b/components/policy/core/common/cloud/user_cloud_policy_manager.cc @@ -20,6 +20,7 @@ #include "components/policy/core/common/policy_types.h" #include "components/policy/policy_constants.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace em = enterprise_management; @@ -83,11 +84,12 @@ void UserCloudPolicyManager::Connect( std::unique_ptr UserCloudPolicyManager::CreateCloudPolicyClient( DeviceManagementService* device_management_service, - scoped_refptr request_context) { + scoped_refptr request_context, + scoped_refptr url_loader_factory) { return std::make_unique( std::string() /* machine_id */, std::string() /* machine_model */, std::string() /* brand_code */, device_management_service, - request_context, nullptr /* signing_service */, + request_context, url_loader_factory, nullptr /* signing_service */, CloudPolicyClient::DeviceDMTokenCallback()); } diff --git a/components/policy/core/common/cloud/user_cloud_policy_manager.h b/components/policy/core/common/cloud/user_cloud_policy_manager.h index 684cb7ad6d5d09..054cf9d9487535 100644 --- a/components/policy/core/common/cloud/user_cloud_policy_manager.h +++ b/components/policy/core/common/cloud/user_cloud_policy_manager.h @@ -25,6 +25,9 @@ class SequencedTaskRunner; namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace policy { @@ -75,7 +78,8 @@ class POLICY_EXPORT UserCloudPolicyManager : public CloudPolicyManager { // want to check if the user's domain requires policy). static std::unique_ptr CreateCloudPolicyClient( DeviceManagementService* device_management_service, - scoped_refptr request_context); + scoped_refptr request_context, + scoped_refptr url_loader_factory); private: // CloudPolicyManager: diff --git a/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc b/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc index 49cf9bef1bc47b..d6e44ba802b361 100644 --- a/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc +++ b/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc @@ -27,6 +27,7 @@ #include "components/policy/core/common/remote_commands/testing_remote_commands_server.h" #include "components/policy/proto/device_management_backend.pb.h" #include "net/url_request/url_request_context_getter.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" @@ -88,6 +89,7 @@ class TestingCloudPolicyClientForRemoteCommands : public CloudPolicyClient { std::string() /* brand_code */, nullptr /* service */, nullptr /* request_context */, + nullptr /* url_loader_factory */, nullptr /* signing_service */, CloudPolicyClient::DeviceDMTokenCallback()), server_(server) { diff --git a/components/signin/DEPS b/components/signin/DEPS index 20e335b0eba53c..813a64278b1769 100644 --- a/components/signin/DEPS +++ b/components/signin/DEPS @@ -13,3 +13,9 @@ include_rules = [ "+net", "+sql", ] + +specific_include_rules = { + "test_signin_client\.h": [ + "+services/network/test" + ] +} \ No newline at end of file diff --git a/components/signin/core/browser/fake_profile_oauth2_token_service.cc b/components/signin/core/browser/fake_profile_oauth2_token_service.cc index 9710636cf6e502..e7a6cc28dba928 100644 --- a/components/signin/core/browser/fake_profile_oauth2_token_service.cc +++ b/components/signin/core/browser/fake_profile_oauth2_token_service.cc @@ -123,6 +123,7 @@ void FakeProfileOAuth2TokenService::FetchOAuth2Token( RequestImpl* request, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const ScopeSet& scopes) { diff --git a/components/signin/core/browser/fake_profile_oauth2_token_service.h b/components/signin/core/browser/fake_profile_oauth2_token_service.h index 9edcd08607167f..f8f0d185e37321 100644 --- a/components/signin/core/browser/fake_profile_oauth2_token_service.h +++ b/components/signin/core/browser/fake_profile_oauth2_token_service.h @@ -87,12 +87,14 @@ class FakeProfileOAuth2TokenService : public ProfileOAuth2TokenService { protected: // OAuth2TokenService overrides. - void FetchOAuth2Token(RequestImpl* request, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes) override; + void FetchOAuth2Token( + RequestImpl* request, + const std::string& account_id, + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes) override; void InvalidateAccessTokenImpl(const std::string& account_id, const std::string& client_id, diff --git a/components/signin/core/browser/signin_client.h b/components/signin/core/browser/signin_client.h index fbaf5c89452475..00be28e435dfa1 100644 --- a/components/signin/core/browser/signin_client.h +++ b/components/signin/core/browser/signin_client.h @@ -29,6 +29,9 @@ class Observer; namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace network { class SharedURLLoaderFactory; diff --git a/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.h b/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.h index 79ae397f64a054..5a298deb8ec519 100644 --- a/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.h +++ b/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.h @@ -29,6 +29,7 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate { OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) override; // KeyedService diff --git a/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.mm b/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.mm index f3089c309b5a23..5d620896ed1495 100644 --- a/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.mm +++ b/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.mm @@ -301,6 +301,7 @@ void OnAccessTokenResponse(NSString* token, ProfileOAuth2TokenServiceIOSDelegate::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) { AccountInfo account_info = account_tracker_service_->GetAccountInfo(account_id); diff --git a/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate_unittest.mm b/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate_unittest.mm index 9131b9bb76bbd0..f14ece7c4db3b0 100644 --- a/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate_unittest.mm +++ b/components/signin/ios/browser/profile_oauth2_token_service_ios_delegate_unittest.mm @@ -261,7 +261,8 @@ void ResetObserverCounts() { scopes.push_back("scope"); std::unique_ptr fetcher1( oauth2_delegate_->CreateAccessTokenFetcher( - GetAccountId(account1), oauth2_delegate_->GetRequestContext(), this)); + GetAccountId(account1), oauth2_delegate_->GetRequestContext(), + oauth2_delegate_->GetURLLoaderFactory(), this)); fetcher1->Start("foo", "bar", scopes); EXPECT_EQ(0, access_token_success_); EXPECT_EQ(0, access_token_failure_); @@ -284,7 +285,8 @@ void ResetObserverCounts() { scopes.push_back("scope"); std::unique_ptr fetcher1( oauth2_delegate_->CreateAccessTokenFetcher( - GetAccountId(account1), oauth2_delegate_->GetRequestContext(), this)); + GetAccountId(account1), oauth2_delegate_->GetRequestContext(), + oauth2_delegate_->GetURLLoaderFactory(), this)); fetcher1->Start("foo", "bar", scopes); EXPECT_EQ(0, access_token_success_); EXPECT_EQ(0, access_token_failure_); diff --git a/extensions/shell/browser/shell_oauth2_token_service_delegate.cc b/extensions/shell/browser/shell_oauth2_token_service_delegate.cc index 759f3e2f24c829..c7f4a51a4b9201 100644 --- a/extensions/shell/browser/shell_oauth2_token_service_delegate.cc +++ b/extensions/shell/browser/shell_oauth2_token_service_delegate.cc @@ -34,10 +34,12 @@ OAuth2AccessTokenFetcher* ShellOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) { DCHECK_EQ(account_id, account_id_); DCHECK(!refresh_token_.empty()); - return new OAuth2AccessTokenFetcherImpl(consumer, getter, refresh_token_); + return new OAuth2AccessTokenFetcherImpl(consumer, url_loader_factory, + refresh_token_); } net::URLRequestContextGetter* diff --git a/extensions/shell/browser/shell_oauth2_token_service_delegate.h b/extensions/shell/browser/shell_oauth2_token_service_delegate.h index 3a87c07e0908db..de8167c9ce51ec 100644 --- a/extensions/shell/browser/shell_oauth2_token_service_delegate.h +++ b/extensions/shell/browser/shell_oauth2_token_service_delegate.h @@ -30,6 +30,7 @@ class ShellOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate { OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) override; net::URLRequestContextGetter* GetRequestContext() const override; diff --git a/google_apis/BUILD.gn b/google_apis/BUILD.gn index 0309ce0af02dc0..f679a5b5da6e4b 100644 --- a/google_apis/BUILD.gn +++ b/google_apis/BUILD.gn @@ -191,6 +191,7 @@ template("google_apis_tmpl") { google_apis_tmpl("google_apis") { deps = [ "//net", + "//services/network/public/cpp", ] } @@ -213,6 +214,8 @@ static_library("test_support") { "//base", "//base/test:test_support", "//net:test_support", + "//services/network:test_support", + "//services/network/public/cpp", ] if (enable_extensions) { @@ -254,6 +257,7 @@ test("google_apis_unittests") { ":test_support", "//base", "//base/test:run_all_unittests", + "//mojo/edk", "//testing/gmock", "//testing/gtest", ] diff --git a/google_apis/DEPS b/google_apis/DEPS index 54031e7ebeab68..926008a9865080 100644 --- a/google_apis/DEPS +++ b/google_apis/DEPS @@ -4,4 +4,5 @@ include_rules = [ "+crypto", "+net", "+third_party/ocmock", + "+services/network/public/cpp", ] diff --git a/google_apis/drive/auth_service.cc b/google_apis/drive/auth_service.cc index f1c7203576971d..983c7d31f30244 100644 --- a/google_apis/drive/auth_service.cc +++ b/google_apis/drive/auth_service.cc @@ -16,6 +16,7 @@ #include "google_apis/drive/auth_service_observer.h" #include "google_apis/gaia/google_service_auth_error.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" namespace google_apis { @@ -41,6 +42,7 @@ class AuthRequest : public OAuth2TokenService::Consumer { AuthRequest(OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, const AuthStatusCallback& callback, const std::vector& scopes); ~AuthRequest() override; @@ -64,17 +66,14 @@ AuthRequest::AuthRequest( OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, const AuthStatusCallback& callback, const std::vector& scopes) - : OAuth2TokenService::Consumer("auth_service"), - callback_(callback) { + : OAuth2TokenService::Consumer("auth_service"), callback_(callback) { DCHECK(!callback_.is_null()); - request_ = oauth2_token_service-> - StartRequestWithContext( - account_id, - url_request_context_getter, - OAuth2TokenService::ScopeSet(scopes.begin(), scopes.end()), - this); + request_ = oauth2_token_service->StartRequestWithContext( + account_id, url_request_context_getter, url_loader_factory, + OAuth2TokenService::ScopeSet(scopes.begin(), scopes.end()), this); } AuthRequest::~AuthRequest() {} @@ -122,10 +121,12 @@ AuthService::AuthService( OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, const std::vector& scopes) : oauth2_token_service_(oauth2_token_service), account_id_(account_id), url_request_context_getter_(url_request_context_getter), + url_loader_factory_(url_loader_factory), scopes_(scopes), weak_ptr_factory_(this) { DCHECK(oauth2_token_service); @@ -149,12 +150,10 @@ void AuthService::StartAuthentication(const AuthStatusCallback& callback) { FROM_HERE, base::Bind(callback, HTTP_SUCCESS, access_token_)); } else if (HasRefreshToken()) { // We have refresh token, let's get an access token. - new AuthRequest(oauth2_token_service_, - account_id_, - url_request_context_getter_.get(), + new AuthRequest(oauth2_token_service_, account_id_, + url_request_context_getter_.get(), url_loader_factory_, base::Bind(&AuthService::OnAuthCompleted, - weak_ptr_factory_.GetWeakPtr(), - callback), + weak_ptr_factory_.GetWeakPtr(), callback), scopes_); } else { base::ThreadTaskRunnerHandle::Get()->PostTask( diff --git a/google_apis/drive/auth_service.h b/google_apis/drive/auth_service.h index 55f112dc7c991c..cb36713206229a 100644 --- a/google_apis/drive/auth_service.h +++ b/google_apis/drive/auth_service.h @@ -18,6 +18,9 @@ namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} namespace google_apis { @@ -37,6 +40,7 @@ class AuthService : public AuthServiceInterface, AuthService(OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* url_request_context_getter, + scoped_refptr url_loader_factory, const std::vector& scopes); ~AuthService() override; @@ -67,6 +71,7 @@ class AuthService : public AuthServiceInterface, OAuth2TokenService* oauth2_token_service_; std::string account_id_; scoped_refptr url_request_context_getter_; + scoped_refptr url_loader_factory_; bool has_refresh_token_; std::string access_token_; std::vector scopes_; diff --git a/google_apis/gaia/DEPS b/google_apis/gaia/DEPS index f8f20b689240cb..d311b38405fdff 100644 --- a/google_apis/gaia/DEPS +++ b/google_apis/gaia/DEPS @@ -1,4 +1,15 @@ include_rules = [ + "+services/network/public/cpp", # Needed to support typemaps of core types for the Identity Service. "+mojo/public/cpp/bindings/struct_traits.h", ] + +specific_include_rules = { + r"(fake_oauth2_token_service_delegate\.h" + r"|oauth2_access_token_fetcher_impl_unittest\.cc" + r"|oauth2_token_service_unittest\.cc" + r")": [ + "+services/network/test/test_url_loader_factory.h", + "+mojo/edk", + ], +} \ No newline at end of file diff --git a/google_apis/gaia/fake_oauth2_token_service.cc b/google_apis/gaia/fake_oauth2_token_service.cc index 3ed5a89e6f0f4b..1d57b22118fa0e 100644 --- a/google_apis/gaia/fake_oauth2_token_service.cc +++ b/google_apis/gaia/fake_oauth2_token_service.cc @@ -26,6 +26,7 @@ void FakeOAuth2TokenService::FetchOAuth2Token( RequestImpl* request, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_factory, const std::string& client_id, const std::string& client_secret, const ScopeSet& scopes) { diff --git a/google_apis/gaia/fake_oauth2_token_service.h b/google_apis/gaia/fake_oauth2_token_service.h index 17a4e26b4dd637..70a8a96af9395b 100644 --- a/google_apis/gaia/fake_oauth2_token_service.h +++ b/google_apis/gaia/fake_oauth2_token_service.h @@ -6,12 +6,16 @@ #define GOOGLE_APIS_GAIA_FAKE_OAUTH2_TOKEN_SERVICE_H_ #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "google_apis/gaia/fake_oauth2_token_service_delegate.h" #include "google_apis/gaia/oauth2_token_service.h" namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} // Do-nothing implementation of OAuth2TokenService. class FakeOAuth2TokenService : public OAuth2TokenService { @@ -35,12 +39,14 @@ class FakeOAuth2TokenService : public OAuth2TokenService { protected: // OAuth2TokenService overrides. - void FetchOAuth2Token(RequestImpl* request, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes) override; + void FetchOAuth2Token( + RequestImpl* request, + const std::string& account_id, + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes) override; void InvalidateAccessTokenImpl(const std::string& account_id, const std::string& client_id, diff --git a/google_apis/gaia/fake_oauth2_token_service_delegate.cc b/google_apis/gaia/fake_oauth2_token_service_delegate.cc index 757bf253fb1f80..4090022f9b8900 100644 --- a/google_apis/gaia/fake_oauth2_token_service_delegate.cc +++ b/google_apis/gaia/fake_oauth2_token_service_delegate.cc @@ -12,8 +12,10 @@ FakeOAuth2TokenServiceDelegate::AccountInfo::AccountInfo( FakeOAuth2TokenServiceDelegate::FakeOAuth2TokenServiceDelegate( net::URLRequestContextGetter* request_context) - : request_context_(request_context) { -} + : request_context_(request_context), + shared_factory_( + base::MakeRefCounted( + &test_url_loader_factory_)) {} FakeOAuth2TokenServiceDelegate::~FakeOAuth2TokenServiceDelegate() { } @@ -22,10 +24,11 @@ OAuth2AccessTokenFetcher* FakeOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_factory, OAuth2AccessTokenConsumer* consumer) { AccountInfoMap::const_iterator it = refresh_tokens_.find(account_id); DCHECK(it != refresh_tokens_.end()); - return new OAuth2AccessTokenFetcherImpl(consumer, getter, + return new OAuth2AccessTokenFetcherImpl(consumer, url_factory, it->second->refresh_token); } @@ -110,6 +113,11 @@ FakeOAuth2TokenServiceDelegate::GetRequestContext() const { return request_context_.get(); } +scoped_refptr +FakeOAuth2TokenServiceDelegate::GetURLLoaderFactory() const { + return shared_factory_; +} + void FakeOAuth2TokenServiceDelegate::UpdateAuthError( const std::string& account_id, const GoogleServiceAuthError& error) { diff --git a/google_apis/gaia/fake_oauth2_token_service_delegate.h b/google_apis/gaia/fake_oauth2_token_service_delegate.h index 45a80def3504f8..6c4cd77a2beb70 100644 --- a/google_apis/gaia/fake_oauth2_token_service_delegate.h +++ b/google_apis/gaia/fake_oauth2_token_service_delegate.h @@ -7,9 +7,16 @@ #include "base/macros.h" #include "base/memory/linked_ptr.h" +#include "base/memory/ref_counted.h" #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_token_service_delegate.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" +#include "services/network/test/test_url_loader_factory.h" + +namespace network { +class SharedURLLoaderFactory; +} class FakeOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate { public: @@ -19,6 +26,7 @@ class FakeOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate { OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_factory, OAuth2AccessTokenConsumer* consumer) override; // Overriden to make sure it works on Android. @@ -38,6 +46,8 @@ class FakeOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate { void RevokeCredentials(const std::string& account_id) override; net::URLRequestContextGetter* GetRequestContext() const override; + scoped_refptr GetURLLoaderFactory() + const override; void set_request_context(net::URLRequestContextGetter* request_context) { request_context_ = request_context; @@ -45,6 +55,10 @@ class FakeOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate { std::string GetRefreshToken(const std::string& account_id) const; + network::TestURLLoaderFactory* test_url_loader_factory() { + return &test_url_loader_factory_; + } + private: struct AccountInfo { AccountInfo(const std::string& refresh_token); @@ -62,6 +76,9 @@ class FakeOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate { scoped_refptr request_context_; + network::TestURLLoaderFactory test_url_loader_factory_; + scoped_refptr shared_factory_; + DISALLOW_COPY_AND_ASSIGN(FakeOAuth2TokenServiceDelegate); }; #endif diff --git a/google_apis/gaia/oauth2_access_token_fetcher_impl.cc b/google_apis/gaia/oauth2_access_token_fetcher_impl.cc index 32d762aa61c15e..95dd85a7391cd0 100644 --- a/google_apis/gaia/oauth2_access_token_fetcher_impl.cc +++ b/google_apis/gaia/oauth2_access_token_fetcher_impl.cc @@ -23,32 +23,29 @@ #include "net/base/load_flags.h" #include "net/http/http_status_code.h" #include "net/traffic_annotation/network_traffic_annotation.h" -#include "net/url_request/url_fetcher.h" -#include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_status.h" - -using net::URLFetcher; -using net::URLFetcherDelegate; -using net::URLRequestContextGetter; -using net::URLRequestStatus; +#include "services/network/public/cpp/resource_request.h" +#include "services/network/public/cpp/resource_response.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" +#include "services/network/public/cpp/simple_url_loader.h" namespace { -static const char kGetAccessTokenBodyFormat[] = +constexpr char kGetAccessTokenBodyFormat[] = "client_id=%s&" "client_secret=%s&" "grant_type=refresh_token&" "refresh_token=%s"; -static const char kGetAccessTokenBodyWithScopeFormat[] = +constexpr char kGetAccessTokenBodyWithScopeFormat[] = "client_id=%s&" "client_secret=%s&" "grant_type=refresh_token&" "refresh_token=%s&" "scope=%s"; -static const char kAccessTokenKey[] = "access_token"; -static const char kExpiresInKey[] = "expires_in"; -static const char kErrorKey[] = "error"; +constexpr char kAccessTokenKey[] = "access_token"; +constexpr char kExpiresInKey[] = "expires_in"; +constexpr char kErrorKey[] = "error"; // Enumerated constants for logging server responses on 400 errors, matching // RFC 6749. @@ -81,23 +78,16 @@ OAuth2ErrorCodesForHistogram OAuth2ErrorToHistogramValue( return OAUTH2_ACCESS_ERROR_UNKNOWN; } -static GoogleServiceAuthError CreateAuthError(URLRequestStatus status) { - CHECK(!status.is_success()); - if (status.status() == URLRequestStatus::CANCELED) { - return GoogleServiceAuthError(GoogleServiceAuthError::REQUEST_CANCELED); - } else { - DLOG(WARNING) << "Could not reach Google Accounts servers: errno " - << status.error(); - return GoogleServiceAuthError::FromConnectionError(status.error()); - } +static GoogleServiceAuthError CreateAuthError(int net_error) { + CHECK_NE(net_error, net::OK); + DLOG(WARNING) << "Could not reach Google Accounts servers: errno " + << net_error; + return GoogleServiceAuthError::FromConnectionError(net_error); } -static std::unique_ptr CreateFetcher( - URLRequestContextGetter* getter, +static std::unique_ptr CreateURLLoader( const GURL& url, - const std::string& body, - URLFetcherDelegate* delegate) { - bool empty_body = body.empty(); + const std::string& body) { net::NetworkTrafficAnnotationTag traffic_annotation = net::DefineNetworkTrafficAnnotation("oauth2_access_token_fetcher", R"( semantics { @@ -125,33 +115,41 @@ static std::unique_ptr CreateFetcher( } } })"); - std::unique_ptr result = net::URLFetcher::Create( - 0, url, empty_body ? URLFetcher::GET : URLFetcher::POST, delegate, - traffic_annotation); - - gaia::MarkURLFetcherAsGaia(result.get()); - result->SetRequestContext(getter); - result->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES | - net::LOAD_DO_NOT_SAVE_COOKIES); + + auto resource_request = std::make_unique(); + resource_request->url = url; + resource_request->load_flags = + net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES; + if (!body.empty()) + resource_request->method = "POST"; + + auto url_loader = network::SimpleURLLoader::Create( + std::move(resource_request), traffic_annotation); + + if (!body.empty()) + url_loader->AttachStringForUpload(body, + "application/x-www-form-urlencoded"); + + // We want to receive the body even on error, as it may contain the reason for + // failure. + url_loader->SetAllowHttpErrorResults(true); + // Fetchers are sometimes cancelled because a network change was detected, // especially at startup and after sign-in on ChromeOS. Retrying once should // be enough in those cases; let the fetcher retry up to 3 times just in case. // http://crbug.com/163710 - result->SetAutomaticallyRetryOnNetworkChanges(3); - - if (!empty_body) - result->SetUploadData("application/x-www-form-urlencoded", body); + url_loader->SetRetryOptions( + 3, network::SimpleURLLoader::RETRY_ON_NETWORK_CHANGE); - return result; + return url_loader; } std::unique_ptr ParseGetAccessTokenResponse( - const net::URLFetcher* source) { - CHECK(source); + std::unique_ptr data) { + if (!data) + return nullptr; - std::string data; - source->GetResponseAsString(&data); - std::unique_ptr value = base::JSONReader::Read(data); + std::unique_ptr value = base::JSONReader::Read(*data); if (!value.get() || value->type() != base::Value::Type::DICTIONARY) value.reset(); @@ -163,16 +161,18 @@ std::unique_ptr ParseGetAccessTokenResponse( OAuth2AccessTokenFetcherImpl::OAuth2AccessTokenFetcherImpl( OAuth2AccessTokenConsumer* consumer, - net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& refresh_token) : OAuth2AccessTokenFetcher(consumer), - getter_(getter), + url_loader_factory_(url_loader_factory), refresh_token_(refresh_token), state_(INITIAL) {} OAuth2AccessTokenFetcherImpl::~OAuth2AccessTokenFetcherImpl() {} -void OAuth2AccessTokenFetcherImpl::CancelRequest() { fetcher_.reset(); } +void OAuth2AccessTokenFetcherImpl::CancelRequest() { + url_loader_.reset(); +} void OAuth2AccessTokenFetcherImpl::Start( const std::string& client_id, @@ -187,29 +187,42 @@ void OAuth2AccessTokenFetcherImpl::Start( void OAuth2AccessTokenFetcherImpl::StartGetAccessToken() { CHECK_EQ(INITIAL, state_); state_ = GET_ACCESS_TOKEN_STARTED; - fetcher_ = CreateFetcher(getter_, MakeGetAccessTokenUrl(), - MakeGetAccessTokenBody(client_id_, client_secret_, - refresh_token_, scopes_), - this); - fetcher_->Start(); // OnURLFetchComplete will be called. + url_loader_ = + CreateURLLoader(MakeGetAccessTokenUrl(), + MakeGetAccessTokenBody(client_id_, client_secret_, + refresh_token_, scopes_)); + // It's safe to use Unretained below as the |url_loader_| is owned by |this|. + url_loader_->DownloadToString( + url_loader_factory_.get(), + base::BindOnce(&OAuth2AccessTokenFetcherImpl::OnURLLoadComplete, + base::Unretained(this)), + 1024 * 1024); } void OAuth2AccessTokenFetcherImpl::EndGetAccessToken( - const net::URLFetcher* source) { + std::unique_ptr response_body) { CHECK_EQ(GET_ACCESS_TOKEN_STARTED, state_); state_ = GET_ACCESS_TOKEN_DONE; - URLRequestStatus status = source->GetStatus(); - int histogram_value = - status.is_success() ? source->GetResponseCode() : status.error(); + bool net_failure = false; + int histogram_value; + if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) { + // Note that the SimpleURLLoader reports net::ERR_FAILED for HTTP codes + // other than 200s. + histogram_value = url_loader_->ResponseInfo()->headers->response_code(); + } else { + histogram_value = url_loader_->NetError(); + net_failure = true; + } base::UmaHistogramSparse("Gaia.ResponseCodesForOAuth2AccessToken", histogram_value); - if (!status.is_success()) { - OnGetTokenFailure(CreateAuthError(status)); + if (net_failure) { + OnGetTokenFailure(CreateAuthError(histogram_value)); return; } - switch (source->GetResponseCode()) { + int response_code = url_loader_->ResponseInfo()->headers->response_code(); + switch (response_code) { case net::HTTP_OK: break; case net::HTTP_FORBIDDEN: @@ -222,7 +235,8 @@ void OAuth2AccessTokenFetcherImpl::EndGetAccessToken( // HTTP_BAD_REQUEST (400) usually contains error as per // http://tools.ietf.org/html/rfc6749#section-5.2. std::string gaia_error; - if (!ParseGetAccessTokenFailureResponse(source, &gaia_error)) { + if (!ParseGetAccessTokenFailureResponse(std::move(response_body), + &gaia_error)) { OnGetTokenFailure( GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_ERROR)); return; @@ -243,14 +257,14 @@ void OAuth2AccessTokenFetcherImpl::EndGetAccessToken( return; } default: { - if (source->GetResponseCode() >= net::HTTP_INTERNAL_SERVER_ERROR) { + if (response_code >= net::HTTP_INTERNAL_SERVER_ERROR) { // 5xx is always treated as transient. OnGetTokenFailure(GoogleServiceAuthError( GoogleServiceAuthError::SERVICE_UNAVAILABLE)); } else { // The other errors are treated as permanent error. DLOG(ERROR) << "Unexpected persistent error: http_status=" - << source->GetResponseCode(); + << response_code; OnGetTokenFailure( GoogleServiceAuthError::FromInvalidGaiaCredentialsReason( GoogleServiceAuthError::InvalidGaiaCredentialsReason:: @@ -264,7 +278,8 @@ void OAuth2AccessTokenFetcherImpl::EndGetAccessToken( // Parse out the access token and the expiration time. std::string access_token; int expires_in; - if (!ParseGetAccessTokenSuccessResponse(source, &access_token, &expires_in)) { + if (!ParseGetAccessTokenSuccessResponse(std::move(response_body), + &access_token, &expires_in)) { DLOG(WARNING) << "Response doesn't match expected format"; OnGetTokenFailure( GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_UNAVAILABLE)); @@ -289,11 +304,10 @@ void OAuth2AccessTokenFetcherImpl::OnGetTokenFailure( FireOnGetTokenFailure(error); } -void OAuth2AccessTokenFetcherImpl::OnURLFetchComplete( - const net::URLFetcher* source) { - CHECK(source); +void OAuth2AccessTokenFetcherImpl::OnURLLoadComplete( + std::unique_ptr response_body) { CHECK(state_ == GET_ACCESS_TOKEN_STARTED); - EndGetAccessToken(source); + EndGetAccessToken(std::move(response_body)); } // static @@ -330,27 +344,24 @@ std::string OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( // static bool OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( - const net::URLFetcher* source, + std::unique_ptr response_body, std::string* access_token, int* expires_in) { CHECK(access_token); std::unique_ptr value = - ParseGetAccessTokenResponse(source); - if (value.get() == NULL) + ParseGetAccessTokenResponse(std::move(response_body)); + if (!value) return false; - return value->GetString(kAccessTokenKey, access_token) && value->GetInteger(kExpiresInKey, expires_in); } // static bool OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenFailureResponse( - const net::URLFetcher* source, + std::unique_ptr response_body, std::string* error) { CHECK(error); std::unique_ptr value = - ParseGetAccessTokenResponse(source); - if (value.get() == NULL) - return false; - return value->GetString(kErrorKey, error); + ParseGetAccessTokenResponse(std::move(response_body)); + return value ? value->GetString(kErrorKey, error) : false; } diff --git a/google_apis/gaia/oauth2_access_token_fetcher_impl.h b/google_apis/gaia/oauth2_access_token_fetcher_impl.h index 9e8759682764dc..5003e0ddab86ed 100644 --- a/google_apis/gaia/oauth2_access_token_fetcher_impl.h +++ b/google_apis/gaia/oauth2_access_token_fetcher_impl.h @@ -11,9 +11,9 @@ #include "base/gtest_prod_util.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "google_apis/gaia/oauth2_access_token_consumer.h" #include "google_apis/gaia/oauth2_access_token_fetcher.h" -#include "net/url_request/url_fetcher_delegate.h" #include "url/gurl.h" class OAuth2AccessTokenFetcherImplTest; @@ -22,13 +22,12 @@ namespace base { class Time; } -namespace net { -class URLFetcher; -class URLRequestContextGetter; +namespace network { +class SimpleURLLoader; +class SharedURLLoaderFactory; } -// Abstracts the details to get OAuth2 access token token from -// OAuth2 refresh token. +// Abstracts the details to get OAuth2 access token from OAuth2 refresh token. // See "Using the Refresh Token" section in: // http://code.google.com/apis/accounts/docs/OAuth2WebServer.html // @@ -45,12 +44,12 @@ class URLRequestContextGetter; // // This class can handle one request at a time. To parallelize requests, // create multiple instances. -class OAuth2AccessTokenFetcherImpl : public OAuth2AccessTokenFetcher, - public net::URLFetcherDelegate { +class OAuth2AccessTokenFetcherImpl : public OAuth2AccessTokenFetcher { public: - OAuth2AccessTokenFetcherImpl(OAuth2AccessTokenConsumer* consumer, - net::URLRequestContextGetter* getter, - const std::string& refresh_token); + OAuth2AccessTokenFetcherImpl( + OAuth2AccessTokenConsumer* consumer, + scoped_refptr url_loader_factory, + const std::string& refresh_token); ~OAuth2AccessTokenFetcherImpl() override; // Implementation of OAuth2AccessTokenFetcher @@ -60,9 +59,6 @@ class OAuth2AccessTokenFetcherImpl : public OAuth2AccessTokenFetcher, void CancelRequest() override; - // Implementation of net::URLFetcherDelegate - void OnURLFetchComplete(const net::URLFetcher* source) override; - private: enum State { INITIAL, @@ -71,9 +67,11 @@ class OAuth2AccessTokenFetcherImpl : public OAuth2AccessTokenFetcher, ERROR_STATE, }; + void OnURLLoadComplete(std::unique_ptr response_body); + // Helper methods for the flow. void StartGetAccessToken(); - void EndGetAccessToken(const net::URLFetcher* source); + void EndGetAccessToken(std::unique_ptr response_body); // Helper mehtods for reporting back results. void OnGetTokenSuccess(const std::string& access_token, @@ -88,30 +86,45 @@ class OAuth2AccessTokenFetcherImpl : public OAuth2AccessTokenFetcher, const std::string& refresh_token, const std::vector& scopes); - static bool ParseGetAccessTokenSuccessResponse(const net::URLFetcher* source, - std::string* access_token, - int* expires_in); + static bool ParseGetAccessTokenSuccessResponse( + std::unique_ptr response_body, + std::string* access_token, + int* expires_in); - static bool ParseGetAccessTokenFailureResponse(const net::URLFetcher* source, - std::string* error); + static bool ParseGetAccessTokenFailureResponse( + std::unique_ptr response_body, + std::string* error); // State that is set during construction. - net::URLRequestContextGetter* const getter_; - std::string refresh_token_; + scoped_refptr url_loader_factory_; + const std::string refresh_token_; State state_; // While a fetch is in progress. - std::unique_ptr fetcher_; + std::unique_ptr url_loader_; std::string client_id_; std::string client_secret_; std::vector scopes_; friend class OAuth2AccessTokenFetcherImplTest; FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, - ParseGetAccessTokenResponse); + ParseGetAccessTokenResponseNoBody); FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, - MakeGetAccessTokenBody); - + MakeGetAccessTokenBodyNoScope); + FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, + MakeGetAccessTokenBodyOneScope); + FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, + MakeGetAccessTokenBodyMultipleScopes); + FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, + ParseGetAccessTokenResponseBadJson); + FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, + ParseGetAccessTokenResponseNoAccessToken); + FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, + ParseGetAccessTokenResponseSuccess); + FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, + ParseGetAccessTokenFailure); + FRIEND_TEST_ALL_PREFIXES(OAuth2AccessTokenFetcherImplTest, + ParseGetAccessTokenFailureInvalidError); DISALLOW_COPY_AND_ASSIGN(OAuth2AccessTokenFetcherImpl); }; diff --git a/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc b/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc index 9d3bf952681f4c..4c9c8e71a2544d 100644 --- a/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc +++ b/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc @@ -11,79 +11,42 @@ #include "base/run_loop.h" #include "base/test/scoped_task_environment.h" -#include "base/threading/thread_task_runner_handle.h" -#include "build/build_config.h" #include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_access_token_consumer.h" +#include "mojo/edk/embedder/embedder.h" #include "net/base/net_errors.h" #include "net/http/http_status_code.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" -#include "net/url_request/test_url_fetcher_factory.h" -#include "net/url_request/url_fetcher.h" -#include "net/url_request/url_fetcher_delegate.h" -#include "net/url_request/url_fetcher_factory.h" -#include "net/url_request/url_request.h" -#include "net/url_request/url_request_status.h" -#include "net/url_request/url_request_test_util.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/gtest/include/gtest/gtest.h" #include "url/gurl.h" -using net::ScopedURLFetcherFactory; -using net::TestURLFetcher; -using net::URLFetcher; -using net::URLFetcherDelegate; -using net::URLFetcherFactory; -using net::URLRequestStatus; using testing::_; -using testing::Return; namespace { -typedef std::vector ScopeList; +using ScopeList = std::vector; -static const char kValidTokenResponse[] = - "{" - " \"access_token\": \"at1\"," - " \"expires_in\": 3600," - " \"token_type\": \"Bearer\"" - "}"; -static const char kTokenResponseNoAccessToken[] = - "{" - " \"expires_in\": 3600," - " \"token_type\": \"Bearer\"" - "}"; +constexpr char kValidTokenResponse[] = R"( + { + "access_token": "at1", + "expires_in": 3600, + "token_type": "Bearer" + })"; -static const char kValidFailureTokenResponse[] = - "{" - " \"error\": \"invalid_grant\"" - "}"; +constexpr char kTokenResponseNoAccessToken[] = R"( + { + "expires_in": 3600, + "token_type": "Bearer" + })"; -class MockUrlFetcherFactory : public ScopedURLFetcherFactory, - public URLFetcherFactory { - public: - MockUrlFetcherFactory() : ScopedURLFetcherFactory(this) {} - ~MockUrlFetcherFactory() override {} - - MOCK_METHOD5( - CreateURLFetcherMock, - URLFetcher*(int id, - const GURL& url, - URLFetcher::RequestType request_type, - URLFetcherDelegate* d, - net::NetworkTrafficAnnotationTag traffic_annotation)); - - std::unique_ptr CreateURLFetcher( - int id, - const GURL& url, - URLFetcher::RequestType request_type, - URLFetcherDelegate* d, - net::NetworkTrafficAnnotationTag traffic_annotation) override { - return std::unique_ptr( - CreateURLFetcherMock(id, url, request_type, d, traffic_annotation)); - } -}; +constexpr char kValidFailureTokenResponse[] = R"( + { + "error": "invalid_grant" + })"; class MockOAuth2AccessTokenConsumer : public OAuth2AccessTokenConsumer { public: @@ -96,186 +59,168 @@ class MockOAuth2AccessTokenConsumer : public OAuth2AccessTokenConsumer { MOCK_METHOD1(OnGetTokenFailure, void(const GoogleServiceAuthError& error)); }; +class URLLoaderFactoryInterceptor { + public: + MOCK_METHOD1(Intercept, void(const network::ResourceRequest&)); +}; + +MATCHER_P(resourceRequestUrlEquals, url, "") { + return arg.url == url; +} + } // namespace class OAuth2AccessTokenFetcherImplTest : public testing::Test { public: OAuth2AccessTokenFetcherImplTest() - : request_context_getter_(new net::TestURLRequestContextGetter( - base::ThreadTaskRunnerHandle::Get())), - fetcher_(&consumer_, request_context_getter_.get(), "refresh_token") { + : shared_url_loader_factory_( + base::MakeRefCounted( + &url_loader_factory_)), + fetcher_(&consumer_, shared_url_loader_factory_, "refresh_token") { + url_loader_factory_.SetInterceptor(base::BindRepeating( + &URLLoaderFactoryInterceptor::Intercept, + base::Unretained(&url_loader_factory_interceptor_))); + mojo::edk::Init(); base::RunLoop().RunUntilIdle(); } - ~OAuth2AccessTokenFetcherImplTest() override {} + ~OAuth2AccessTokenFetcherImplTest() override { + shared_url_loader_factory_->Detach(); + } - virtual TestURLFetcher* SetupGetAccessToken(bool fetch_succeeds, - int response_code, - const std::string& body) { + void SetupGetAccessToken(int net_error_code, + net::HttpStatusCode http_response_code, + const std::string& body) { GURL url(GaiaUrls::GetInstance()->oauth2_token_url()); - TestURLFetcher* url_fetcher = new TestURLFetcher(0, url, &fetcher_); - net::Error error = fetch_succeeds ? net::OK : net::ERR_FAILED; - url_fetcher->set_status(URLRequestStatus::FromError(error)); - - if (response_code != 0) - url_fetcher->set_response_code(response_code); - - if (!body.empty()) - url_fetcher->SetResponseString(body); - - EXPECT_CALL(factory_, CreateURLFetcherMock(_, url, _, _, _)) - .WillOnce(Return(url_fetcher)); - return url_fetcher; + if (net_error_code == net::OK) { + url_loader_factory_.AddResponse(url.spec(), body, http_response_code); + } else { + url_loader_factory_.AddResponse( + url, network::ResourceResponseHead(), body, + network::URLLoaderCompletionStatus(net_error_code)); + } + + EXPECT_CALL(url_loader_factory_interceptor_, + Intercept(resourceRequestUrlEquals(url))); } protected: base::test::ScopedTaskEnvironment scoped_task_environment_; - MockUrlFetcherFactory factory_; MockOAuth2AccessTokenConsumer consumer_; - scoped_refptr request_context_getter_; + URLLoaderFactoryInterceptor url_loader_factory_interceptor_; + network::TestURLLoaderFactory url_loader_factory_; + scoped_refptr + shared_url_loader_factory_; OAuth2AccessTokenFetcherImpl fetcher_; }; // These four tests time out, see http://crbug.com/113446. -TEST_F(OAuth2AccessTokenFetcherImplTest, - DISABLED_GetAccessTokenRequestFailure) { - TestURLFetcher* url_fetcher = SetupGetAccessToken(false, 0, std::string()); +TEST_F(OAuth2AccessTokenFetcherImplTest, GetAccessTokenRequestFailure) { + SetupGetAccessToken(net::ERR_FAILED, net::HTTP_OK, std::string()); EXPECT_CALL(consumer_, OnGetTokenFailure(_)).Times(1); fetcher_.Start("client_id", "client_secret", ScopeList()); - fetcher_.OnURLFetchComplete(url_fetcher); + base::RunLoop().RunUntilIdle(); } -TEST_F(OAuth2AccessTokenFetcherImplTest, - DISABLED_GetAccessTokenResponseCodeFailure) { - TestURLFetcher* url_fetcher = - SetupGetAccessToken(true, net::HTTP_FORBIDDEN, std::string()); +TEST_F(OAuth2AccessTokenFetcherImplTest, GetAccessTokenResponseCodeFailure) { + SetupGetAccessToken(net::OK, net::HTTP_FORBIDDEN, std::string()); EXPECT_CALL(consumer_, OnGetTokenFailure(_)).Times(1); fetcher_.Start("client_id", "client_secret", ScopeList()); - fetcher_.OnURLFetchComplete(url_fetcher); + base::RunLoop().RunUntilIdle(); } -TEST_F(OAuth2AccessTokenFetcherImplTest, DISABLED_Success) { - TestURLFetcher* url_fetcher = - SetupGetAccessToken(true, net::HTTP_OK, kValidTokenResponse); +TEST_F(OAuth2AccessTokenFetcherImplTest, Success) { + SetupGetAccessToken(net::OK, net::HTTP_OK, kValidTokenResponse); EXPECT_CALL(consumer_, OnGetTokenSuccess("at1", _)).Times(1); fetcher_.Start("client_id", "client_secret", ScopeList()); - fetcher_.OnURLFetchComplete(url_fetcher); + base::RunLoop().RunUntilIdle(); } -TEST_F(OAuth2AccessTokenFetcherImplTest, DISABLED_MakeGetAccessTokenBody) { - { // No scope. - std::string body = - "client_id=cid1&" - "client_secret=cs1&" - "grant_type=refresh_token&" - "refresh_token=rt1"; - EXPECT_EQ(body, - OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( - "cid1", "cs1", "rt1", ScopeList())); - } - - { // One scope. - std::string body = - "client_id=cid1&" - "client_secret=cs1&" - "grant_type=refresh_token&" - "refresh_token=rt1&" - "scope=https://www.googleapis.com/foo"; - ScopeList scopes; - scopes.push_back("https://www.googleapis.com/foo"); - EXPECT_EQ(body, - OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( - "cid1", "cs1", "rt1", scopes)); - } +TEST_F(OAuth2AccessTokenFetcherImplTest, MakeGetAccessTokenBodyNoScope) { + std::string body = + "client_id=cid1&" + "client_secret=cs1&" + "grant_type=refresh_token&" + "refresh_token=rt1"; + EXPECT_EQ(body, OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( + "cid1", "cs1", "rt1", ScopeList())); +} - { // Multiple scopes. - std::string body = - "client_id=cid1&" - "client_secret=cs1&" - "grant_type=refresh_token&" - "refresh_token=rt1&" - "scope=https://www.googleapis.com/foo+" - "https://www.googleapis.com/bar+" - "https://www.googleapis.com/baz"; - ScopeList scopes; - scopes.push_back("https://www.googleapis.com/foo"); - scopes.push_back("https://www.googleapis.com/bar"); - scopes.push_back("https://www.googleapis.com/baz"); - EXPECT_EQ(body, - OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( - "cid1", "cs1", "rt1", scopes)); - } +TEST_F(OAuth2AccessTokenFetcherImplTest, MakeGetAccessTokenBodyOneScope) { + std::string body = + "client_id=cid1&" + "client_secret=cs1&" + "grant_type=refresh_token&" + "refresh_token=rt1&" + "scope=https://www.googleapis.com/foo"; + ScopeList scopes = {"https://www.googleapis.com/foo"}; + EXPECT_EQ(body, OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( + "cid1", "cs1", "rt1", scopes)); } -// http://crbug.com/114215 -#if defined(OS_WIN) -#define MAYBE_ParseGetAccessTokenResponse DISABLED_ParseGetAccessTokenResponse -#else -#define MAYBE_ParseGetAccessTokenResponse ParseGetAccessTokenResponse -#endif // defined(OS_WIN) -TEST_F(OAuth2AccessTokenFetcherImplTest, MAYBE_ParseGetAccessTokenResponse) { - { // No body. - TestURLFetcher url_fetcher(0, GURL("http://www.google.com"), NULL); +TEST_F(OAuth2AccessTokenFetcherImplTest, MakeGetAccessTokenBodyMultipleScopes) { + std::string body = + "client_id=cid1&" + "client_secret=cs1&" + "grant_type=refresh_token&" + "refresh_token=rt1&" + "scope=https://www.googleapis.com/foo+" + "https://www.googleapis.com/bar+" + "https://www.googleapis.com/baz"; + ScopeList scopes = {"https://www.googleapis.com/foo", + "https://www.googleapis.com/bar", + "https://www.googleapis.com/baz"}; + EXPECT_EQ(body, OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( + "cid1", "cs1", "rt1", scopes)); +} - std::string at; - int expires_in; - EXPECT_FALSE( - OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( - &url_fetcher, &at, &expires_in)); - EXPECT_TRUE(at.empty()); - } - { // Bad json. - TestURLFetcher url_fetcher(0, GURL("http://www.google.com"), NULL); - url_fetcher.SetResponseString("foo"); +TEST_F(OAuth2AccessTokenFetcherImplTest, ParseGetAccessTokenResponseNoBody) { + std::string at; + int expires_in; + auto empty_body = std::make_unique(""); + EXPECT_FALSE(OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( + std::move(empty_body), &at, &expires_in)); + EXPECT_TRUE(at.empty()); +} - std::string at; - int expires_in; - EXPECT_FALSE( - OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( - &url_fetcher, &at, &expires_in)); - EXPECT_TRUE(at.empty()); - } - { // Valid json: access token missing. - TestURLFetcher url_fetcher(0, GURL("http://www.google.com"), NULL); - url_fetcher.SetResponseString(kTokenResponseNoAccessToken); +TEST_F(OAuth2AccessTokenFetcherImplTest, ParseGetAccessTokenResponseBadJson) { + std::string at; + int expires_in; + EXPECT_FALSE(OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( + std::make_unique("foo"), &at, &expires_in)); + EXPECT_TRUE(at.empty()); +} - std::string at; - int expires_in; - EXPECT_FALSE( - OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( - &url_fetcher, &at, &expires_in)); - EXPECT_TRUE(at.empty()); - } - { // Valid json: all good. - TestURLFetcher url_fetcher(0, GURL("http://www.google.com"), NULL); - url_fetcher.SetResponseString(kValidTokenResponse); +TEST_F(OAuth2AccessTokenFetcherImplTest, + ParseGetAccessTokenResponseNoAccessToken) { + std::string at; + int expires_in; + EXPECT_FALSE(OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( + std::make_unique(kTokenResponseNoAccessToken), &at, + &expires_in)); + EXPECT_TRUE(at.empty()); +} - std::string at; - int expires_in; - EXPECT_TRUE( - OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( - &url_fetcher, &at, &expires_in)); - EXPECT_EQ("at1", at); - EXPECT_EQ(3600, expires_in); - } - { // Valid json: invalid error response. - TestURLFetcher url_fetcher(0, GURL("http://www.google.com"), NULL); - url_fetcher.SetResponseString(kTokenResponseNoAccessToken); +TEST_F(OAuth2AccessTokenFetcherImplTest, ParseGetAccessTokenResponseSuccess) { + std::string at; + int expires_in; + EXPECT_TRUE(OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenSuccessResponse( + std::make_unique(kValidTokenResponse), &at, &expires_in)); + EXPECT_EQ("at1", at); + EXPECT_EQ(3600, expires_in); +} - std::string error; - EXPECT_FALSE( - OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenFailureResponse( - &url_fetcher, &error)); - EXPECT_TRUE(error.empty()); - } - { // Valid json: error response. - TestURLFetcher url_fetcher(0, GURL("http://www.google.com"), NULL); - url_fetcher.SetResponseString(kValidFailureTokenResponse); +TEST_F(OAuth2AccessTokenFetcherImplTest, + ParseGetAccessTokenFailureInvalidError) { + std::string error; + EXPECT_FALSE(OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenFailureResponse( + std::make_unique(kTokenResponseNoAccessToken), &error)); + EXPECT_TRUE(error.empty()); +} - std::string error; - EXPECT_TRUE( - OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenFailureResponse( - &url_fetcher, &error)); - EXPECT_EQ("invalid_grant", error); - } +TEST_F(OAuth2AccessTokenFetcherImplTest, ParseGetAccessTokenFailure) { + std::string error; + EXPECT_TRUE(OAuth2AccessTokenFetcherImpl::ParseGetAccessTokenFailureResponse( + std::make_unique(kValidFailureTokenResponse), &error)); + EXPECT_EQ("invalid_grant", error); } diff --git a/google_apis/gaia/oauth2_token_service.cc b/google_apis/gaia/oauth2_token_service.cc index 542435cbb90a69..8b50814b9cb256 100644 --- a/google_apis/gaia/oauth2_token_service.cc +++ b/google_apis/gaia/oauth2_token_service.cc @@ -24,6 +24,7 @@ #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" #include "google_apis/gaia/oauth2_token_service_delegate.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" int OAuth2TokenService::max_fetch_retry_num_ = 5; @@ -124,6 +125,7 @@ class OAuth2TokenService::Fetcher : public OAuth2AccessTokenConsumer { OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const ScopeSet& scopes, @@ -159,6 +161,7 @@ class OAuth2TokenService::Fetcher : public OAuth2AccessTokenConsumer { Fetcher(OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const OAuth2TokenService::ScopeSet& scopes, @@ -175,6 +178,7 @@ class OAuth2TokenService::Fetcher : public OAuth2AccessTokenConsumer { // (whichever comes first). OAuth2TokenService* const oauth2_token_service_; scoped_refptr getter_; + scoped_refptr url_loader_factory_; const std::string account_id_; const ScopeSet scopes_; std::vector > waiting_requests_; @@ -203,13 +207,14 @@ OAuth2TokenService::Fetcher::CreateAndStart( OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const OAuth2TokenService::ScopeSet& scopes, base::WeakPtr waiting_request) { std::unique_ptr fetcher = base::WrapUnique( - new Fetcher(oauth2_token_service, account_id, getter, client_id, - client_secret, scopes, waiting_request)); + new Fetcher(oauth2_token_service, account_id, getter, url_loader_factory, + client_id, client_secret, scopes, waiting_request)); fetcher->Start(); return fetcher; @@ -219,12 +224,14 @@ OAuth2TokenService::Fetcher::Fetcher( OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const OAuth2TokenService::ScopeSet& scopes, base::WeakPtr waiting_request) : oauth2_token_service_(oauth2_token_service), getter_(getter), + url_loader_factory_(url_loader_factory), account_id_(account_id), scopes_(scopes), retry_number_(0), @@ -243,7 +250,7 @@ OAuth2TokenService::Fetcher::~Fetcher() { void OAuth2TokenService::Fetcher::Start() { fetcher_.reset(oauth2_token_service_->CreateAccessTokenFetcher( - account_id_, getter_.get(), this)); + account_id_, getter_.get(), url_loader_factory_, this)); DCHECK(fetcher_); // Stop the timer before starting the fetch, as defense in depth against the @@ -420,12 +427,9 @@ std::unique_ptr OAuth2TokenService::StartRequest( const OAuth2TokenService::ScopeSet& scopes, OAuth2TokenService::Consumer* consumer) { return StartRequestForClientWithContext( - account_id, - GetRequestContext(), + account_id, GetRequestContext(), delegate_->GetURLLoaderFactory(), GaiaUrls::GetInstance()->oauth2_chrome_client_id(), - GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), - scopes, - consumer); + GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), scopes, consumer); } std::unique_ptr @@ -436,12 +440,8 @@ OAuth2TokenService::StartRequestForClient( const OAuth2TokenService::ScopeSet& scopes, OAuth2TokenService::Consumer* consumer) { return StartRequestForClientWithContext( - account_id, - GetRequestContext(), - client_id, - client_secret, - scopes, - consumer); + account_id, GetRequestContext(), delegate_->GetURLLoaderFactory(), + client_id, client_secret, scopes, consumer); } net::URLRequestContextGetter* OAuth2TokenService::GetRequestContext() const { @@ -452,21 +452,20 @@ std::unique_ptr OAuth2TokenService::StartRequestWithContext( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const ScopeSet& scopes, Consumer* consumer) { return StartRequestForClientWithContext( - account_id, - getter, + account_id, getter, url_loader_factory, GaiaUrls::GetInstance()->oauth2_chrome_client_id(), - GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), - scopes, - consumer); + GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), scopes, consumer); } std::unique_ptr OAuth2TokenService::StartRequestForClientWithContext( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const ScopeSet& scopes, @@ -500,22 +499,20 @@ OAuth2TokenService::StartRequestForClientWithContext( InformConsumerWithCacheEntry(cache_entry, request.get(), request_parameters); } else { - FetchOAuth2Token(request.get(), - account_id, - getter, - client_id, - client_secret, - scopes); + FetchOAuth2Token(request.get(), account_id, getter, url_loader_factory, + client_id, client_secret, scopes); } return std::move(request); } -void OAuth2TokenService::FetchOAuth2Token(RequestImpl* request, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes) { +void OAuth2TokenService::FetchOAuth2Token( + RequestImpl* request, + const std::string& account_id, + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes) { // If there is already a pending fetcher for |scopes| and |account_id|, // simply register this |request| for those results rather than starting // a new fetcher. @@ -528,21 +525,18 @@ void OAuth2TokenService::FetchOAuth2Token(RequestImpl* request, return; } - pending_fetchers_[request_parameters] = - Fetcher::CreateAndStart(this, - account_id, - getter, - client_id, - client_secret, - scopes, - request->AsWeakPtr()); + pending_fetchers_[request_parameters] = Fetcher::CreateAndStart( + this, account_id, getter, url_loader_factory, client_id, client_secret, + scopes, request->AsWeakPtr()); } OAuth2AccessTokenFetcher* OAuth2TokenService::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer) { - return delegate_->CreateAccessTokenFetcher(account_id, getter, consumer); + return delegate_->CreateAccessTokenFetcher(account_id, getter, + url_loader_factory, consumer); } void OAuth2TokenService::InformConsumerWithCacheEntry( diff --git a/google_apis/gaia/oauth2_token_service.h b/google_apis/gaia/oauth2_token_service.h index 9f279c525021c1..4b8bc853c79981 100644 --- a/google_apis/gaia/oauth2_token_service.h +++ b/google_apis/gaia/oauth2_token_service.h @@ -14,6 +14,7 @@ #include "base/gtest_prod_util.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "base/sequence_checker.h" @@ -25,6 +26,9 @@ namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} class GoogleServiceAuthError; class OAuth2AccessTokenFetcher; @@ -180,6 +184,7 @@ class OAuth2TokenService { std::unique_ptr StartRequestWithContext( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const ScopeSet& scopes, Consumer* consumer); @@ -287,17 +292,20 @@ class OAuth2TokenService { // Fetches an OAuth token for the specified client/scopes. Virtual so it can // be overridden for tests and for platform-specific behavior. - virtual void FetchOAuth2Token(RequestImpl* request, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes); + virtual void FetchOAuth2Token( + RequestImpl* request, + const std::string& account_id, + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes); // Create an access token fetcher for the given account id. OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, OAuth2AccessTokenConsumer* consumer); // Invalidates the |access_token| issued for |account_id|, |client_id| and @@ -346,6 +354,7 @@ class OAuth2TokenService { std::unique_ptr StartRequestForClientWithContext( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const ScopeSet& scopes, diff --git a/google_apis/gaia/oauth2_token_service_delegate.cc b/google_apis/gaia/oauth2_token_service_delegate.cc index 9b77071cb95fef..aa0bce3cb38ac0 100644 --- a/google_apis/gaia/oauth2_token_service_delegate.cc +++ b/google_apis/gaia/oauth2_token_service_delegate.cc @@ -5,6 +5,7 @@ #include "google_apis/gaia/oauth2_token_service_delegate.h" #include "google_apis/gaia/oauth2_token_service.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" // static const char OAuth2TokenServiceDelegate::kInvalidRefreshToken[] = @@ -101,6 +102,11 @@ net::URLRequestContextGetter* OAuth2TokenServiceDelegate::GetRequestContext() return nullptr; } +scoped_refptr +OAuth2TokenServiceDelegate::GetURLLoaderFactory() const { + return nullptr; +} + GoogleServiceAuthError OAuth2TokenServiceDelegate::GetAuthError( const std::string& account_id) const { return GoogleServiceAuthError::AuthErrorNone(); diff --git a/google_apis/gaia/oauth2_token_service_delegate.h b/google_apis/gaia/oauth2_token_service_delegate.h index a65589bd4e2b20..d3f5132898db58 100644 --- a/google_apis/gaia/oauth2_token_service_delegate.h +++ b/google_apis/gaia/oauth2_token_service_delegate.h @@ -6,6 +6,7 @@ #define GOOGLE_APIS_GAIA_OAUTH2_TOKEN_SERVICE_DELEGATE_H_ #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/observer_list.h" #include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/oauth2_token_service.h" @@ -14,6 +15,9 @@ namespace net { class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +} // Abstract base class to fetch and maintain refresh tokens from various // entities. Concrete subclasses should implement RefreshTokenIsAvailable and @@ -41,6 +45,7 @@ class OAuth2TokenServiceDelegate { virtual OAuth2AccessTokenFetcher* CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_factory, OAuth2AccessTokenConsumer* consumer) = 0; virtual bool RefreshTokenIsAvailable(const std::string& account_id) const = 0; @@ -63,6 +68,8 @@ class OAuth2TokenServiceDelegate { const std::string& refresh_token) {} virtual void RevokeCredentials(const std::string& account_id) {} virtual net::URLRequestContextGetter* GetRequestContext() const; + virtual scoped_refptr GetURLLoaderFactory() + const; bool ValidateAccountId(const std::string& account_id) const; diff --git a/google_apis/gaia/oauth2_token_service_request_unittest.cc b/google_apis/gaia/oauth2_token_service_request_unittest.cc index 2c6f8280102aed..c49d30f0232265 100644 --- a/google_apis/gaia/oauth2_token_service_request_unittest.cc +++ b/google_apis/gaia/oauth2_token_service_request_unittest.cc @@ -87,12 +87,14 @@ class MockOAuth2TokenService : public FakeOAuth2TokenService { } protected: - void FetchOAuth2Token(RequestImpl* request, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes) override; + void FetchOAuth2Token( + RequestImpl* request, + const std::string& account_id, + net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes) override; void InvalidateAccessTokenImpl(const std::string& account_id, const std::string& client_id, @@ -129,6 +131,7 @@ void MockOAuth2TokenService::FetchOAuth2Token( RequestImpl* request, const std::string& account_id, net::URLRequestContextGetter* getter, + scoped_refptr url_loader_factory, const std::string& client_id, const std::string& client_secret, const ScopeSet& scopes) { diff --git a/google_apis/gaia/oauth2_token_service_unittest.cc b/google_apis/gaia/oauth2_token_service_unittest.cc index 324a5a33df0e69..0c73d1b34c372a 100644 --- a/google_apis/gaia/oauth2_token_service_unittest.cc +++ b/google_apis/gaia/oauth2_token_service_unittest.cc @@ -11,11 +11,13 @@ #include "base/run_loop.h" #include "google_apis/gaia/fake_oauth2_token_service_delegate.h" #include "google_apis/gaia/gaia_constants.h" +#include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_access_token_consumer.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" #include "google_apis/gaia/oauth2_token_service.h" #include "google_apis/gaia/oauth2_token_service_test_util.h" +#include "mojo/edk/embedder/embedder.h" #include "net/http/http_status_code.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher_delegate.h" @@ -35,11 +37,15 @@ class RetryingTestingOAuth2TokenServiceConsumer void OnGetTokenFailure(const OAuth2TokenService::Request* request, const GoogleServiceAuthError& error) override { + if (retry_counter_ <= 0) + return; + retry_counter_--; TestingOAuth2TokenServiceConsumer::OnGetTokenFailure(request, error); request_ = oauth2_service_->StartRequest( account_id_, OAuth2TokenService::ScopeSet(), this); } + int retry_counter_ = 2; OAuth2TokenService* oauth2_service_; std::string account_id_; std::unique_ptr request_; @@ -65,10 +71,12 @@ class TestOAuth2TokenService : public OAuth2TokenService { class OAuth2TokenServiceTest : public testing::Test { public: void SetUp() override { - oauth2_service_.reset(new TestOAuth2TokenService( - std::make_unique( - new net::TestURLRequestContextGetter( - message_loop_.task_runner())))); + mojo::edk::Init(); + auto delegate = std::make_unique( + new net::TestURLRequestContextGetter(message_loop_.task_runner())); + test_url_loader_factory_ = delegate->test_url_loader_factory(); + oauth2_service_ = + std::make_unique(std::move(delegate)); account_id_ = "test_user@gmail.com"; } @@ -77,9 +85,15 @@ class OAuth2TokenServiceTest : public testing::Test { base::RunLoop().RunUntilIdle(); } + void SimulateOAuthTokenResponse(const std::string& token, + net::HttpStatusCode status = net::HTTP_OK) { + test_url_loader_factory_->AddResponse( + GaiaUrls::GetInstance()->oauth2_token_url().spec(), token, status); + } + protected: base::MessageLoopForIO message_loop_; // net:: stuff needs IO message loop. - net::TestURLFetcherFactory factory_; + network::TestURLLoaderFactory* test_url_loader_factory_ = nullptr; std::unique_ptr oauth2_service_; std::string account_id_; TestingOAuth2TokenServiceConsumer consumer_; @@ -101,18 +115,14 @@ TEST_F(OAuth2TokenServiceTest, FailureShouldNotRetry) { std::unique_ptr request( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, consumer_.number_of_successful_tokens_); - EXPECT_EQ(0, consumer_.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_UNAUTHORIZED); - fetcher->SetResponseString(std::string()); - fetcher->delegate()->OnURLFetchComplete(fetcher); + SimulateOAuthTokenResponse("", net::HTTP_UNAUTHORIZED); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(1, consumer_.number_of_errors_); - EXPECT_EQ(fetcher, factory_.GetFetcherByID(0)); + EXPECT_EQ(0, test_url_loader_factory_->NumPending()); } TEST_F(OAuth2TokenServiceTest, SuccessWithoutCaching) { @@ -121,15 +131,12 @@ TEST_F(OAuth2TokenServiceTest, SuccessWithoutCaching) { std::unique_ptr request( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); - base::RunLoop().RunUntilIdle(); - EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); @@ -149,17 +156,13 @@ TEST_F(OAuth2TokenServiceTest, SuccessWithCaching) { account_id_, "refreshToken"); // First request. + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); std::unique_ptr request( oauth2_service_->StartRequest(account_id_, scopes1, &consumer_)); - base::RunLoop().RunUntilIdle(); - EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); @@ -176,16 +179,13 @@ TEST_F(OAuth2TokenServiceTest, SuccessWithCaching) { EXPECT_EQ("token", consumer_.last_token_); // Third request to a new set of scopes, should return another token. + SimulateOAuthTokenResponse(GetValidTokenResponse("token2", 3600)); std::unique_ptr request3( oauth2_service_->StartRequest(account_id_, scopes2, &consumer_)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(2, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token2", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(3, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token2", consumer_.last_token_); @@ -196,35 +196,27 @@ TEST_F(OAuth2TokenServiceTest, SuccessAndExpirationAndFailure) { account_id_, "refreshToken"); // First request. + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 0)); std::unique_ptr request( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 0)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); // Second request must try to access the network as the token has expired. + SimulateOAuthTokenResponse("", net::HTTP_UNAUTHORIZED); // Network failure. std::unique_ptr request2( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); + base::RunLoop().RunUntilIdle(); - // Network failure. - fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_UNAUTHORIZED); - fetcher->SetResponseString(std::string()); - fetcher->delegate()->OnURLFetchComplete(fetcher); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(1, consumer_.number_of_errors_); } @@ -234,34 +226,25 @@ TEST_F(OAuth2TokenServiceTest, SuccessAndExpirationAndSuccess) { account_id_, "refreshToken"); // First request. + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 0)); std::unique_ptr request( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 0)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); // Second request must try to access the network as the token has expired. + SimulateOAuthTokenResponse(GetValidTokenResponse("another token", 0)); std::unique_ptr request2( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - - fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("another token", 0)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + base::RunLoop().RunUntilIdle(); EXPECT_EQ(2, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("another token", consumer_.last_token_); @@ -277,14 +260,12 @@ TEST_F(OAuth2TokenServiceTest, RequestDeletedBeforeCompletion) { base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); + EXPECT_EQ(1, test_url_loader_factory_->NumPending()); request.reset(); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); + EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); } @@ -293,15 +274,11 @@ TEST_F(OAuth2TokenServiceTest, RequestDeletedAfterCompletion) { oauth2_service_->GetFakeOAuth2TokenServiceDelegate()->UpdateCredentials( account_id_, "refreshToken"); + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); std::unique_ptr request( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); base::RunLoop().RunUntilIdle(); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); @@ -328,11 +305,9 @@ TEST_F(OAuth2TokenServiceTest, MultipleRequestsForTheSameScopesWithOneDeleted) { request.reset(); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); } @@ -344,12 +319,8 @@ TEST_F(OAuth2TokenServiceTest, ClearedRefreshTokenFailsSubsequentRequests) { std::unique_ptr request( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer_)); + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); base::RunLoop().RunUntilIdle(); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); @@ -378,8 +349,7 @@ TEST_F(OAuth2TokenServiceTest, std::unique_ptr request( oauth2_service_->StartRequest(account_id_, scopes, &consumer_)); base::RunLoop().RunUntilIdle(); - net::TestURLFetcher* fetcher1 = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher1); + ASSERT_EQ(1, test_url_loader_factory_->NumPending()); // Note |request| is still pending when the refresh token changes. oauth2_service_->GetFakeOAuth2TokenServiceDelegate()->UpdateCredentials( @@ -392,17 +362,25 @@ TEST_F(OAuth2TokenServiceTest, oauth2_service_->StartRequest(account_id_, scopes1, &consumer2)); base::RunLoop().RunUntilIdle(); - net::TestURLFetcher* fetcher2 = factory_.GetFetcherByID(0); - fetcher2->set_response_code(net::HTTP_OK); - fetcher2->SetResponseString(GetValidTokenResponse("second token", 3600)); - fetcher2->delegate()->OnURLFetchComplete(fetcher2); + std::vector* pending_requests = + test_url_loader_factory_->pending_requests(); + ASSERT_EQ(2U, pending_requests->size()); + network::TestURLLoaderFactory::PendingRequest second_request = + std::move((*pending_requests)[1]); + network::TestURLLoaderFactory::PendingRequest first_request = + std::move((*pending_requests)[0]); + pending_requests->clear(); + + network::TestURLLoaderFactory::SimulateResponse( + std::move(second_request), GetValidTokenResponse("second token", 3600)); + base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, consumer2.number_of_successful_tokens_); EXPECT_EQ(0, consumer2.number_of_errors_); EXPECT_EQ("second token", consumer2.last_token_); - fetcher1->set_response_code(net::HTTP_OK); - fetcher1->SetResponseString(GetValidTokenResponse("first token", 3600)); - fetcher1->delegate()->OnURLFetchComplete(fetcher1); + network::TestURLLoaderFactory::SimulateResponse( + std::move(first_request), GetValidTokenResponse("first token", 3600)); + base::RunLoop().RunUntilIdle(); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("first token", consumer_.last_token_); @@ -433,23 +411,11 @@ TEST_F(OAuth2TokenServiceTest, RetryingConsumer) { std::unique_ptr request( oauth2_service_->StartRequest(account_id_, OAuth2TokenService::ScopeSet(), &consumer)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, consumer.number_of_successful_tokens_); EXPECT_EQ(0, consumer.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_UNAUTHORIZED); - fetcher->SetResponseString(std::string()); - fetcher->delegate()->OnURLFetchComplete(fetcher); - EXPECT_EQ(0, consumer.number_of_successful_tokens_); - EXPECT_EQ(1, consumer.number_of_errors_); - - fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_UNAUTHORIZED); - fetcher->SetResponseString(std::string()); - fetcher->delegate()->OnURLFetchComplete(fetcher); + SimulateOAuthTokenResponse("", net::HTTP_UNAUTHORIZED); + base::RunLoop().RunUntilIdle(); EXPECT_EQ(0, consumer.number_of_successful_tokens_); EXPECT_EQ(2, consumer.number_of_errors_); } @@ -466,11 +432,10 @@ TEST_F(OAuth2TokenServiceTest, InvalidateToken) { EXPECT_EQ(0, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); @@ -486,19 +451,23 @@ TEST_F(OAuth2TokenServiceTest, InvalidateToken) { EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); + // Clear previous response so the token request will be pending and we can + // simulate a response after it started. + test_url_loader_factory_->ClearResponses(); + // Invalidating the token should return a new token on the next request. oauth2_service_->InvalidateAccessToken(account_id_, scopes, consumer_.last_token_); std::unique_ptr request3( oauth2_service_->StartRequest(account_id_, scopes, &consumer_)); base::RunLoop().RunUntilIdle(); + EXPECT_EQ(2, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); - fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token2", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + + SimulateOAuthTokenResponse(GetValidTokenResponse("token2", 3600)); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(3, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token2", consumer_.last_token_); @@ -640,12 +609,9 @@ TEST_F(OAuth2TokenServiceTest, UpdateClearsCache) { kEmail, "refreshToken"); std::unique_ptr request( oauth2_service_->StartRequest(kEmail, scope_list, &consumer_)); + SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600)); base::RunLoop().RunUntilIdle(); - net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); + EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("token", consumer_.last_token_); @@ -657,13 +623,9 @@ TEST_F(OAuth2TokenServiceTest, UpdateClearsCache) { EXPECT_EQ(0, (int)oauth2_service_->token_cache_.size()); oauth2_service_->GetFakeOAuth2TokenServiceDelegate()->UpdateCredentials( kEmail, "refreshToken"); + SimulateOAuthTokenResponse(GetValidTokenResponse("another token", 3600)); request = oauth2_service_->StartRequest(kEmail, scope_list, &consumer_); base::RunLoop().RunUntilIdle(); - fetcher = factory_.GetFetcherByID(0); - // ASSERT_TRUE(fetcher); - fetcher->set_response_code(net::HTTP_OK); - fetcher->SetResponseString(GetValidTokenResponse("another token", 3600)); - fetcher->delegate()->OnURLFetchComplete(fetcher); EXPECT_EQ(2, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); EXPECT_EQ("another token", consumer_.last_token_); diff --git a/ios/web_view/internal/signin/ios_web_view_signin_client.mm b/ios/web_view/internal/signin/ios_web_view_signin_client.mm index 2c4d469f19dae1..f2861221d9e4eb 100644 --- a/ios/web_view/internal/signin/ios_web_view_signin_client.mm +++ b/ios/web_view/internal/signin/ios_web_view_signin_client.mm @@ -7,6 +7,7 @@ #include "components/signin/core/browser/cookie_settings_util.h" #include "components/signin/core/browser/signin_cookie_change_subscription.h" #include "google_apis/gaia/gaia_auth_fetcher.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." diff --git a/services/network/test/test_url_loader_factory.cc b/services/network/test/test_url_loader_factory.cc index 679c0bff9ddf56..aaa3c993fde80c 100644 --- a/services/network/test/test_url_loader_factory.cc +++ b/services/network/test/test_url_loader_factory.cc @@ -14,6 +14,21 @@ namespace network { +namespace { + +ResourceResponseHead CreateResourceResponseHead( + net::HttpStatusCode http_status) { + ResourceResponseHead head; + std::string headers(base::StringPrintf( + "HTTP/1.1 %d %s\nContent-type: text/html\n\n", + static_cast(http_status), net::GetHttpReasonPhrase(http_status))); + head.headers = new net::HttpResponseHeaders( + net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size())); + return head; +} + +} // namespace + TestURLLoaderFactory::PendingRequest::PendingRequest() = default; TestURLLoaderFactory::PendingRequest::~PendingRequest() = default; @@ -55,12 +70,7 @@ void TestURLLoaderFactory::AddResponse(const GURL& url, void TestURLLoaderFactory::AddResponse(const std::string& url, const std::string& content, net::HttpStatusCode http_status) { - ResourceResponseHead head; - std::string headers(base::StringPrintf( - "HTTP/1.1 %d %s\nContent-type: text/html\n\n", - static_cast(http_status), GetHttpReasonPhrase(http_status))); - head.headers = new net::HttpResponseHeaders( - net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size())); + ResourceResponseHead head = CreateResourceResponseHead(http_status); head.mime_type = "text/html"; URLLoaderCompletionStatus status; status.decoded_body_length = content.size(); @@ -131,20 +141,44 @@ bool TestURLLoaderFactory::CreateLoaderAndStartInternal( if (it == responses_.end()) return false; - for (const auto& redirect : it->second.redirects) + SimulateResponseImpl(client, it->second.redirects, it->second.head, + it->second.content, it->second.status); + return true; +} + +// static +void TestURLLoaderFactory::SimulateResponse( + TestURLLoaderFactory::PendingRequest request, + std::string content, + int net_error) { + network::URLLoaderCompletionStatus status(net::OK); + ResourceResponseHead head = CreateResourceResponseHead(net::HTTP_OK); + status.decoded_body_length = content.size(); + SimulateResponseImpl(request.client.get(), TestURLLoaderFactory::Redirects(), + head, content, status); + base::RunLoop().RunUntilIdle(); +} + +// static +void TestURLLoaderFactory::SimulateResponseImpl( + mojom::URLLoaderClient* client, + TestURLLoaderFactory::Redirects redirects, + ResourceResponseHead head, + std::string content, + URLLoaderCompletionStatus status) { + for (const auto& redirect : redirects) client->OnReceiveRedirect(redirect.first, redirect.second); - if (it->second.status.error_code == net::OK) { - client->OnReceiveResponse(it->second.head); - mojo::DataPipe data_pipe(it->second.content.size()); - uint32_t bytes_written = it->second.content.size(); + if (status.error_code == net::OK) { + client->OnReceiveResponse(head); + mojo::DataPipe data_pipe(content.size()); + uint32_t bytes_written = content.size(); CHECK_EQ(MOJO_RESULT_OK, data_pipe.producer_handle->WriteData( - it->second.content.data(), &bytes_written, + content.data(), &bytes_written, MOJO_WRITE_DATA_FLAG_ALL_OR_NONE)); client->OnStartLoadingResponseBody(std::move(data_pipe.consumer_handle)); } - client->OnComplete(it->second.status); - return true; + client->OnComplete(status); } } // namespace network diff --git a/services/network/test/test_url_loader_factory.h b/services/network/test/test_url_loader_factory.h index f36a6ce421cd76..5aca2f66201311 100644 --- a/services/network/test/test_url_loader_factory.h +++ b/services/network/test/test_url_loader_factory.h @@ -77,6 +77,12 @@ class TestURLLoaderFactory : public mojom::URLLoaderFactory { // servicing requests themselves, whenever possible. std::vector* pending_requests() { return &pending_requests_; } + // Sends a response for |request| that can be retrieved from + // pending_requests(). Prefer using AddResponse. + static void SimulateResponse(PendingRequest request, + std::string content, + int net_error = net::OK); + // mojom::URLLoaderFactory implementation. void CreateLoaderAndStart(mojom::URLLoaderRequest request, int32_t routing_id, @@ -92,6 +98,12 @@ class TestURLLoaderFactory : public mojom::URLLoaderFactory { bool CreateLoaderAndStartInternal(const GURL& url, mojom::URLLoaderClient* client); + static void SimulateResponseImpl(mojom::URLLoaderClient* client, + Redirects redirects, + ResourceResponseHead head, + std::string content, + URLLoaderCompletionStatus status); + struct Response { Response(); ~Response();