Skip to content

Commit

Permalink
[CrOS Tether] Eliminate token fetch from Initializer.
Browse files Browse the repository at this point in the history
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.

[email protected]

(cherry picked from commit a88ec4a)

Bug: 763506, 672263
Change-Id: I3e02f10aa201b129591028a8606b6fb4eefbaf11
Reviewed-on: https://chromium-review.googlesource.com/663978
Commit-Queue: Kyle Horimoto <[email protected]>
Reviewed-by: Ryan Hansberry <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#501486}
Reviewed-on: https://chromium-review.googlesource.com/663982
Reviewed-by: Kyle Horimoto <[email protected]>
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#193}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
  • Loading branch information
Kyle Horimoto authored and Kyle Horimoto committed Sep 13, 2017
1 parent 7b954f1 commit 79da5a0
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 55 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/chromeos/tether/tether_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "chrome/browser/chromeos/tether/tether_service_factory.h"
#include "chrome/browser/cryptauth/chrome_cryptauth_service_factory.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/pref_names.h"
#include "chromeos/chromeos_switches.h"
Expand Down Expand Up @@ -159,7 +158,6 @@ void TetherService::StartTetherIfPossible() {
PA_LOG(INFO) << "Starting up Tether component.";
initializer_ = chromeos::tether::InitializerImpl::Factory::NewInstance(
cryptauth_service_, notification_presenter_.get(), profile_->GetPrefs(),
ProfileOAuth2TokenServiceFactory::GetForProfile(profile_),
network_state_handler_,
chromeos::NetworkHandler::Get()->managed_network_configuration_handler(),
chromeos::NetworkConnect::Get(),
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/chromeos/tether/tether_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ class TestInitializerFactory
cryptauth::CryptAuthService* cryptauth_service,
chromeos::tether::NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
chromeos::NetworkStateHandler* network_state_handler,
chromeos::ManagedNetworkConfigurationHandler*
managed_network_configuration_handler,
Expand Down
35 changes: 2 additions & 33 deletions chromeos/components/tether/initializer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::NewInstance(
cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect,
Expand All @@ -74,7 +73,7 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::NewInstance(
factory_instance_ = new Factory();
}
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_connect, network_connection_handler, adapter);
}
Expand All @@ -96,14 +95,13 @@ std::unique_ptr<Initializer> InitializerImpl::Factory::BuildInstance(
cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect,
NetworkConnectionHandler* network_connection_handler,
scoped_refptr<device::BluetoothAdapter> adapter) {
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_connect, network_connection_handler, adapter));
}
Expand All @@ -112,7 +110,6 @@ InitializerImpl::InitializerImpl(
cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect,
Expand All @@ -121,28 +118,17 @@ InitializerImpl::InitializerImpl(
: cryptauth_service_(cryptauth_service),
notification_presenter_(notification_presenter),
pref_service_(pref_service),
token_service_(token_service),
network_state_handler_(network_state_handler),
managed_network_configuration_handler_(
managed_network_configuration_handler),
network_connect_(network_connect),
network_connection_handler_(network_connection_handler),
adapter_(adapter),
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();
}

InitializerImpl::~InitializerImpl() {
token_service_->RemoveObserver(this);
network_state_handler_->set_tether_sort_delegate(nullptr);

if (disconnect_tethering_request_sender_)
Expand Down Expand Up @@ -187,20 +173,6 @@ void InitializerImpl::RequestShutdown() {
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() {
DCHECK(status() == Initializer::Status::SHUTTING_DOWN);
disconnect_tethering_request_sender_->RemoveObserver(this);
Expand All @@ -217,9 +189,6 @@ void InitializerImpl::OnPendingDisconnectRequestsComplete() {
}

void InitializerImpl::CreateComponent() {
PA_LOG(INFO) << "Successfully set Bluetooth advertisement interval. "
<< "Initializing tether feature.";

network_list_sorter_ = base::MakeUnique<NetworkListSorter>();
network_state_handler_->set_tether_sort_delegate(network_list_sorter_.get());
tether_host_fetcher_ =
Expand Down
9 changes: 0 additions & 9 deletions chromeos/components/tether/initializer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "chromeos/components/tether/disconnect_tethering_request_sender.h"
#include "chromeos/components/tether/initializer.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_advertisement.h"

Expand Down Expand Up @@ -64,7 +63,6 @@ class WifiHotspotDisconnector;

// Initializes the Tether Chrome OS component.
class InitializerImpl : public Initializer,
public OAuth2TokenService::Observer,
public DisconnectTetheringRequestSender::Observer {
public:
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
Expand All @@ -75,7 +73,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler*
managed_network_configuration_handler,
Expand All @@ -90,7 +87,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler*
managed_network_configuration_handler,
Expand All @@ -106,7 +102,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect,
Expand All @@ -118,9 +113,6 @@ class InitializerImpl : public Initializer,
// Initializer:
void RequestShutdown() override;

// OAuth2TokenService::Observer:
void OnRefreshTokensLoaded() override;

// DisconnectTetheringRequestSender::Observer:
void OnPendingDisconnectRequestsComplete() override;

Expand All @@ -134,7 +126,6 @@ class InitializerImpl : public Initializer,
cryptauth::CryptAuthService* cryptauth_service_;
NotificationPresenter* notification_presenter_;
PrefService* pref_service_;
ProfileOAuth2TokenService* token_service_;
NetworkStateHandler* network_state_handler_;
ManagedNetworkConfigurationHandler* managed_network_configuration_handler_;
NetworkConnect* network_connect_;
Expand Down
14 changes: 4 additions & 10 deletions chromeos/components/tether/initializer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "components/cryptauth/proto/cryptauth_api.pb.h"
#include "components/cryptauth/secure_message_delegate.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 "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -142,14 +141,13 @@ class InitializerTest : public NetworkStateTest {
cryptauth::CryptAuthService* cryptauth_service,
NotificationPresenter* notification_presenter,
PrefService* pref_service,
ProfileOAuth2TokenService* token_service,
NetworkStateHandler* network_state_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler,
NetworkConnect* network_connect,
NetworkConnectionHandler* network_connection_handler,
scoped_refptr<device::BluetoothAdapter> adapter) {
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_connect, network_connection_handler, adapter);
delete initializer;
Expand Down Expand Up @@ -193,9 +191,6 @@ TEST_F(InitializerTest, TestCreateAndDestroy) {
std::unique_ptr<TestingPrefServiceSimple> test_pref_service =
base::MakeUnique<TestingPrefServiceSimple>();

std::unique_ptr<FakeProfileOAuth2TokenService> fake_token_service =
base::MakeUnique<FakeProfileOAuth2TokenService>();

std::unique_ptr<ManagedNetworkConfigurationHandler>
managed_network_configuration_handler = base::WrapUnique(
new NiceMock<MockManagedNetworkConfigurationHandler>);
Expand All @@ -214,10 +209,9 @@ TEST_F(InitializerTest, TestCreateAndDestroy) {
// InitializerTest only applies to the class itself, not these test functions.
InitializeAndDestroy(
fake_cryptauth_service.get(), fake_notification_presenter.get(),
test_pref_service_.get(), fake_token_service.get(),
network_state_handler(), managed_network_configuration_handler.get(),
mock_network_connect.get(), network_connection_handler_.get(),
mock_adapter);
test_pref_service_.get(), network_state_handler(),
managed_network_configuration_handler.get(), mock_network_connect.get(),
network_connection_handler_.get(), mock_adapter);
}

} // namespace tether
Expand Down

0 comments on commit 79da5a0

Please sign in to comment.