Commit a88ec4a6 authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Commit Bot

[CrOS Tether] Eliminate token fetch from Initializer.

This code was unnecessary, since the CryptAuth component is the one that
needs to listen for these tokens. Additionally, the listener in this
class was set up improperly, which made it possible to get "stuck"
without a token.

Bug: 763506, 672263
Change-Id: I3e02f10aa201b129591028a8606b6fb4eefbaf11
Reviewed-on: https://chromium-review.googlesource.com/663978
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarRyan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501486}
parent 26327d27
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include "chrome/browser/chromeos/tether/tether_service_factory.h" #include "chrome/browser/chromeos/tether/tether_service_factory.h"
#include "chrome/browser/cryptauth/chrome_cryptauth_service_factory.h" #include "chrome/browser/cryptauth/chrome_cryptauth_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chromeos/chromeos_switches.h" #include "chromeos/chromeos_switches.h"
...@@ -159,7 +158,6 @@ void TetherService::StartTetherIfPossible() { ...@@ -159,7 +158,6 @@ void TetherService::StartTetherIfPossible() {
PA_LOG(INFO) << "Starting up Tether component."; PA_LOG(INFO) << "Starting up Tether component.";
initializer_ = chromeos::tether::InitializerImpl::Factory::NewInstance( initializer_ = chromeos::tether::InitializerImpl::Factory::NewInstance(
cryptauth_service_, notification_presenter_.get(), profile_->GetPrefs(), cryptauth_service_, notification_presenter_.get(), profile_->GetPrefs(),
ProfileOAuth2TokenServiceFactory::GetForProfile(profile_),
network_state_handler_, network_state_handler_,
chromeos::NetworkHandler::Get()->managed_network_configuration_handler(), chromeos::NetworkHandler::Get()->managed_network_configuration_handler(),
chromeos::NetworkConnect::Get(), chromeos::NetworkConnect::Get(),
......
...@@ -157,7 +157,6 @@ class TestInitializerFactory final ...@@ -157,7 +157,6 @@ class TestInitializerFactory final
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
chromeos::tether::NotificationPresenter* notification_presenter, chromeos::tether::NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
chromeos::NetworkStateHandler* network_state_handler, chromeos::NetworkStateHandler* network_state_handler,
chromeos::ManagedNetworkConfigurationHandler* chromeos::ManagedNetworkConfigurationHandler*
managed_network_configuration_handler, managed_network_configuration_handler,
......
...@@ -64,7 +64,6 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::NewInstance( ...@@ -64,7 +64,6 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::NewInstance(
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter, NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler, NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler, ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect, NetworkConnect* network_connect,
...@@ -74,7 +73,7 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::NewInstance( ...@@ -74,7 +73,7 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::NewInstance(
factory_instance_ = new Factory(); factory_instance_ = new Factory();
} }
return factory_instance_->BuildInstance( return factory_instance_->BuildInstance(
cryptauth_service, notification_presenter, pref_service, token_service, cryptauth_service, notification_presenter, pref_service,
network_state_handler, managed_network_configuration_handler, network_state_handler, managed_network_configuration_handler,
network_connect, network_connection_handler, adapter); network_connect, network_connection_handler, adapter);
} }
...@@ -96,14 +95,13 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::BuildInstance( ...@@ -96,14 +95,13 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::BuildInstance(
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter, NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler, NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler, ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect, NetworkConnect* network_connect,
NetworkConnectionHandler* network_connection_handler, NetworkConnectionHandler* network_connection_handler,
scoped_refptr<device::BluetoothAdapter> adapter) { scoped_refptr<device::BluetoothAdapter> adapter) {
return base::WrapUnique(new InitializerImpl( return base::WrapUnique(new InitializerImpl(
cryptauth_service, notification_presenter, pref_service, token_service, cryptauth_service, notification_presenter, pref_service,
network_state_handler, managed_network_configuration_handler, network_state_handler, managed_network_configuration_handler,
network_connect, network_connection_handler, adapter)); network_connect, network_connection_handler, adapter));
} }
...@@ -112,7 +110,6 @@ InitializerImpl::InitializerImpl( ...@@ -112,7 +110,6 @@ InitializerImpl::InitializerImpl(
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter, NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler, NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler, ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect, NetworkConnect* network_connect,
...@@ -121,7 +118,6 @@ InitializerImpl::InitializerImpl( ...@@ -121,7 +118,6 @@ InitializerImpl::InitializerImpl(
: cryptauth_service_(cryptauth_service), : cryptauth_service_(cryptauth_service),
notification_presenter_(notification_presenter), notification_presenter_(notification_presenter),
pref_service_(pref_service), pref_service_(pref_service),
token_service_(token_service),
network_state_handler_(network_state_handler), network_state_handler_(network_state_handler),
managed_network_configuration_handler_( managed_network_configuration_handler_(
managed_network_configuration_handler), managed_network_configuration_handler),
...@@ -129,20 +125,10 @@ InitializerImpl::InitializerImpl( ...@@ -129,20 +125,10 @@ InitializerImpl::InitializerImpl(
network_connection_handler_(network_connection_handler), network_connection_handler_(network_connection_handler),
adapter_(adapter), adapter_(adapter),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
if (!token_service_->RefreshTokenIsAvailable(
cryptauth_service_->GetAccountId())) {
PA_LOG(INFO) << "Refresh token not yet available; "
<< "waiting for valid token to initializing tether feature.";
token_service_->AddObserver(this);
return;
}
PA_LOG(INFO) << "Refresh token is available; initializing tether feature.";
CreateComponent(); CreateComponent();
} }
InitializerImpl::~InitializerImpl() { InitializerImpl::~InitializerImpl() {
token_service_->RemoveObserver(this);
network_state_handler_->set_tether_sort_delegate(nullptr); network_state_handler_->set_tether_sort_delegate(nullptr);
if (disconnect_tethering_request_sender_) if (disconnect_tethering_request_sender_)
...@@ -187,20 +173,6 @@ void InitializerImpl::RequestShutdown() { ...@@ -187,20 +173,6 @@ void InitializerImpl::RequestShutdown() {
StartAsynchronousShutdown(); StartAsynchronousShutdown();
} }
void InitializerImpl::OnRefreshTokensLoaded() {
if (!token_service_->RefreshTokenIsAvailable(
cryptauth_service_->GetAccountId())) {
// If a token for the active account is still not available, continue
// waiting for a new token.
return;
}
PA_LOG(INFO) << "Refresh token has loaded; initializing tether feature.";
token_service_->RemoveObserver(this);
CreateComponent();
}
void InitializerImpl::OnPendingDisconnectRequestsComplete() { void InitializerImpl::OnPendingDisconnectRequestsComplete() {
DCHECK(status() == Initializer::Status::SHUTTING_DOWN); DCHECK(status() == Initializer::Status::SHUTTING_DOWN);
disconnect_tethering_request_sender_->RemoveObserver(this); disconnect_tethering_request_sender_->RemoveObserver(this);
...@@ -217,9 +189,6 @@ void InitializerImpl::OnPendingDisconnectRequestsComplete() { ...@@ -217,9 +189,6 @@ void InitializerImpl::OnPendingDisconnectRequestsComplete() {
} }
void InitializerImpl::CreateComponent() { void InitializerImpl::CreateComponent() {
PA_LOG(INFO) << "Successfully set Bluetooth advertisement interval. "
<< "Initializing tether feature.";
network_list_sorter_ = base::MakeUnique<NetworkListSorter>(); network_list_sorter_ = base::MakeUnique<NetworkListSorter>();
network_state_handler_->set_tether_sort_delegate(network_list_sorter_.get()); network_state_handler_->set_tether_sort_delegate(network_list_sorter_.get());
tether_host_fetcher_ = tether_host_fetcher_ =
......
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "chromeos/components/tether/disconnect_tethering_request_sender.h" #include "chromeos/components/tether/disconnect_tethering_request_sender.h"
#include "chromeos/components/tether/initializer.h" #include "chromeos/components/tether/initializer.h"
#include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_registry_simple.h"
#include "components/signin/core/browser/profile_oauth2_token_service.h"
#include "device/bluetooth/bluetooth_adapter.h" #include "device/bluetooth/bluetooth_adapter.h"
#include "device/bluetooth/bluetooth_advertisement.h" #include "device/bluetooth/bluetooth_advertisement.h"
...@@ -64,7 +63,6 @@ class WifiHotspotDisconnector; ...@@ -64,7 +63,6 @@ class WifiHotspotDisconnector;
// Initializes the Tether Chrome OS component. // Initializes the Tether Chrome OS component.
class InitializerImpl : public Initializer, class InitializerImpl : public Initializer,
public OAuth2TokenService::Observer,
public DisconnectTetheringRequestSender::Observer { public DisconnectTetheringRequestSender::Observer {
public: public:
static void RegisterProfilePrefs(PrefRegistrySimple* registry); static void RegisterProfilePrefs(PrefRegistrySimple* registry);
...@@ -75,7 +73,6 @@ class InitializerImpl : public Initializer, ...@@ -75,7 +73,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter, NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler, NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* ManagedNetworkConfigurationHandler*
managed_network_configuration_handler, managed_network_configuration_handler,
...@@ -90,7 +87,6 @@ class InitializerImpl : public Initializer, ...@@ -90,7 +87,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter, NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler, NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* ManagedNetworkConfigurationHandler*
managed_network_configuration_handler, managed_network_configuration_handler,
...@@ -106,7 +102,6 @@ class InitializerImpl : public Initializer, ...@@ -106,7 +102,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter, NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler, NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler, ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect, NetworkConnect* network_connect,
...@@ -118,9 +113,6 @@ class InitializerImpl : public Initializer, ...@@ -118,9 +113,6 @@ class InitializerImpl : public Initializer,
// Initializer: // Initializer:
void RequestShutdown() override; void RequestShutdown() override;
// OAuth2TokenService::Observer:
void OnRefreshTokensLoaded() override;
// DisconnectTetheringRequestSender::Observer: // DisconnectTetheringRequestSender::Observer:
void OnPendingDisconnectRequestsComplete() override; void OnPendingDisconnectRequestsComplete() override;
...@@ -134,7 +126,6 @@ class InitializerImpl : public Initializer, ...@@ -134,7 +126,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service_; cryptauth::CryptAuthService* cryptauth_service_;
NotificationPresenter* notification_presenter_; NotificationPresenter* notification_presenter_;
PrefService* pref_service_; PrefService* pref_service_;
ProfileOAuth2TokenService* token_service_;
NetworkStateHandler* network_state_handler_; NetworkStateHandler* network_state_handler_;
ManagedNetworkConfigurationHandler* managed_network_configuration_handler_; ManagedNetworkConfigurationHandler* managed_network_configuration_handler_;
NetworkConnect* network_connect_; NetworkConnect* network_connect_;
......
...@@ -27,7 +27,6 @@ ...@@ -27,7 +27,6 @@
#include "components/cryptauth/proto/cryptauth_api.pb.h" #include "components/cryptauth/proto/cryptauth_api.pb.h"
#include "components/cryptauth/secure_message_delegate.h" #include "components/cryptauth/secure_message_delegate.h"
#include "components/prefs/testing_pref_service.h" #include "components/prefs/testing_pref_service.h"
#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
#include "device/bluetooth/test/mock_bluetooth_adapter.h" #include "device/bluetooth/test/mock_bluetooth_adapter.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -142,14 +141,13 @@ class InitializerTest : public NetworkStateTest { ...@@ -142,14 +141,13 @@ class InitializerTest : public NetworkStateTest {
cryptauth::CryptAuthService* cryptauth_service, cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter, NotificationPresenter* notification_presenter,
PrefService* pref_service, PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler, NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler, ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect, NetworkConnect* network_connect,
NetworkConnectionHandler* network_connection_handler, NetworkConnectionHandler* network_connection_handler,
scoped_refptr<device::BluetoothAdapter> adapter) { scoped_refptr<device::BluetoothAdapter> adapter) {
Initializer* initializer = new InitializerImpl( Initializer* initializer = new InitializerImpl(
cryptauth_service, notification_presenter, pref_service, token_service, cryptauth_service, notification_presenter, pref_service,
network_state_handler, managed_network_configuration_handler, network_state_handler, managed_network_configuration_handler,
network_connect, network_connection_handler, adapter); network_connect, network_connection_handler, adapter);
delete initializer; delete initializer;
...@@ -193,9 +191,6 @@ TEST_F(InitializerTest, TestCreateAndDestroy) { ...@@ -193,9 +191,6 @@ TEST_F(InitializerTest, TestCreateAndDestroy) {
std::unique_ptr<TestingPrefServiceSimple> test_pref_service = std::unique_ptr<TestingPrefServiceSimple> test_pref_service =
base::MakeUnique<TestingPrefServiceSimple>(); base::MakeUnique<TestingPrefServiceSimple>();
std::unique_ptr<FakeProfileOAuth2TokenService> fake_token_service =
base::MakeUnique<FakeProfileOAuth2TokenService>();
std::unique_ptr<ManagedNetworkConfigurationHandler> std::unique_ptr<ManagedNetworkConfigurationHandler>
managed_network_configuration_handler = base::WrapUnique( managed_network_configuration_handler = base::WrapUnique(
new NiceMock<MockManagedNetworkConfigurationHandler>); new NiceMock<MockManagedNetworkConfigurationHandler>);
...@@ -214,10 +209,9 @@ TEST_F(InitializerTest, TestCreateAndDestroy) { ...@@ -214,10 +209,9 @@ TEST_F(InitializerTest, TestCreateAndDestroy) {
// InitializerTest only applies to the class itself, not these test functions. // InitializerTest only applies to the class itself, not these test functions.
InitializeAndDestroy( InitializeAndDestroy(
fake_cryptauth_service.get(), fake_notification_presenter.get(), fake_cryptauth_service.get(), fake_notification_presenter.get(),
test_pref_service_.get(), fake_token_service.get(), test_pref_service_.get(), network_state_handler(),
network_state_handler(), managed_network_configuration_handler.get(), managed_network_configuration_handler.get(), mock_network_connect.get(),
mock_network_connect.get(), network_connection_handler_.get(), network_connection_handler_.get(), mock_adapter);
mock_adapter);
} }
} // namespace tether } // namespace tether
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment