Tether: Display a 'setup required' notification when appropriate.

BUG=672263

Review-Url: https://codereview.chromium.org/2917803002
Cr-Commit-Position: refs/heads/master@{#476495}

chromeos/components/tether/initializer.cc

@@ -193,11 +193,24 @@ void Initializer::OnBluetoothAdapterAdvertisingIntervalSet(
                                                       network_state_handler_);
   device_id_tether_network_guid_map_ =
       base::MakeUnique<DeviceIdTetherNetworkGuidMap>();
+  host_scan_cache_ = base::MakeUnique<HostScanCache>(
+      network_state_handler_, active_host_.get(),
+      tether_host_response_recorder_.get(),
+      device_id_tether_network_guid_map_.get());
+  clock_ = base::MakeUnique<base::DefaultClock>();
+  host_scanner_ = base::MakeUnique<HostScanner>(
+      tether_host_fetcher_.get(), ble_connection_manager_.get(),
+      host_scan_device_prioritizer_.get(), tether_host_response_recorder_.get(),
+      notification_presenter_.get(), device_id_tether_network_guid_map_.get(),
+      host_scan_cache_.get(), clock_.get());
+  host_scan_scheduler_ = base::MakeUnique<HostScanScheduler>(
+      network_state_handler_, host_scanner_.get());
   tether_connector_ = base::MakeUnique<TetherConnector>(
       network_state_handler_, wifi_hotspot_connector_.get(), active_host_.get(),
       tether_host_fetcher_.get(), ble_connection_manager_.get(),
       tether_host_response_recorder_.get(),
-      device_id_tether_network_guid_map_.get());
+      device_id_tether_network_guid_map_.get(), host_scan_cache_.get(),
+      notification_presenter_.get());
   network_configuration_remover_ =
       base::MakeUnique<NetworkConfigurationRemover>(
           network_state_handler_, managed_network_configuration_handler_);
@@ -214,18 +227,6 @@ void Initializer::OnBluetoothAdapterAdvertisingIntervalSet(
       base::MakeUnique<NetworkConnectionHandlerTetherDelegate>(
           network_connection_handler_, tether_connector_.get(),
           tether_disconnector_.get());
-  host_scan_cache_ = base::MakeUnique<HostScanCache>(
-      network_state_handler_, active_host_.get(),
-      tether_host_response_recorder_.get(),
-      device_id_tether_network_guid_map_.get());
-  clock_ = base::MakeUnique<base::DefaultClock>();
-  host_scanner_ = base::MakeUnique<HostScanner>(
-      tether_host_fetcher_.get(), ble_connection_manager_.get(),
-      host_scan_device_prioritizer_.get(), tether_host_response_recorder_.get(),
-      notification_presenter_.get(), device_id_tether_network_guid_map_.get(),
-      host_scan_cache_.get(), clock_.get());
-  host_scan_scheduler_ = base::MakeUnique<HostScanScheduler>(
-      network_state_handler_, host_scanner_.get());
 
   // Because Initializer is created on each user log in, it's appropriate to
   // call this method now.

chromeos/components/tether/initializer.h

@@ -124,6 +124,10 @@ class Initializer : public OAuth2TokenService::Observer {
       active_host_network_state_updater_;
   std::unique_ptr<DeviceIdTetherNetworkGuidMap>
       device_id_tether_network_guid_map_;
+  std::unique_ptr<HostScanCache> host_scan_cache_;
+  std::unique_ptr<base::DefaultClock> clock_;
+  std::unique_ptr<HostScanner> host_scanner_;
+  std::unique_ptr<HostScanScheduler> host_scan_scheduler_;
   std::unique_ptr<TetherConnector> tether_connector_;
   std::unique_ptr<TetherDisconnector> tether_disconnector_;
   std::unique_ptr<NetworkConfigurationRemover> network_configuration_remover_;
@@ -131,10 +135,6 @@ class Initializer : public OAuth2TokenService::Observer {
       network_connection_handler_tether_delegate_;
   std::unique_ptr<TetherNetworkDisconnectionHandler>
       tether_network_disconnection_handler_;
-  std::unique_ptr<HostScanCache> host_scan_cache_;
-  std::unique_ptr<base::DefaultClock> clock_;
-  std::unique_ptr<HostScanner> host_scanner_;
-  std::unique_ptr<HostScanScheduler> host_scan_scheduler_;
 
   base::WeakPtrFactory<Initializer> weak_ptr_factory_;
 

chromeos/components/tether/network_connection_handler_tether_delegate_unittest.cc

@@ -73,7 +73,9 @@ class MockTetherConnector : public TetherConnector {
                         nullptr /* tether_host_fetcher */,
                         nullptr /* connection_manager */,
                         nullptr /* tether_host_response_recorder */,
-                        nullptr /* device_id_tether_network_guid_map */) {}
+                        nullptr /* device_id_tether_network_guid_map */,
+                        nullptr /* host_scan_cache */,
+                        nullptr /* notification_presenter */) {}
   ~MockTetherConnector() override {}
 
   MOCK_METHOD3(
@@ -149,4 +151,4 @@ TEST_F(NetworkConnectionHandlerTetherDelegateTest, TestDisconnect) {
 
 }  // namespace tether
 
-}  // namespace cryptauth
+}  // namespace chromeos

chromeos/components/tether/tether_connector.cc

@@ -7,9 +7,12 @@
 #include "base/bind.h"
 #include "chromeos/components/tether/active_host.h"
 #include "chromeos/components/tether/device_id_tether_network_guid_map.h"
+#include "chromeos/components/tether/host_scan_cache.h"
+#include "chromeos/components/tether/notification_presenter.h"
 #include "chromeos/components/tether/tether_host_fetcher.h"
 #include "chromeos/components/tether/wifi_hotspot_connector.h"
 #include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "components/proximity_auth/logging/logging.h"
 
@@ -24,7 +27,9 @@ TetherConnector::TetherConnector(
     TetherHostFetcher* tether_host_fetcher,
     BleConnectionManager* connection_manager,
     TetherHostResponseRecorder* tether_host_response_recorder,
-    DeviceIdTetherNetworkGuidMap* device_id_tether_network_guid_map)
+    DeviceIdTetherNetworkGuidMap* device_id_tether_network_guid_map,
+    HostScanCache* host_scan_cache,
+    NotificationPresenter* notification_presenter)
     : network_state_handler_(network_state_handler),
       wifi_hotspot_connector_(wifi_hotspot_connector),
       active_host_(active_host),
@@ -32,6 +37,8 @@ TetherConnector::TetherConnector(
       connection_manager_(connection_manager),
       tether_host_response_recorder_(tether_host_response_recorder),
       device_id_tether_network_guid_map_(device_id_tether_network_guid_map),
+      host_scan_cache_(host_scan_cache),
+      notification_presenter_(notification_presenter),
       weak_ptr_factory_(this) {}
 
 TetherConnector::~TetherConnector() {
@@ -71,6 +78,13 @@ void TetherConnector::ConnectToNetwork(
             device_id_pending_connection_));
   }
 
+  if (host_scan_cache_->DoesHostRequireSetup(tether_network_guid)) {
+    const std::string& device_name =
+        network_state_handler_->GetNetworkStateFromGuid(tether_network_guid)
+            ->name();
+    notification_presenter_->NotifySetupRequired(device_name);
+  }
+
   device_id_pending_connection_ = device_id;
   success_callback_ = success_callback;
   error_callback_ = error_callback;
@@ -185,6 +199,8 @@ void TetherConnector::OnTetherHostToConnectFetched(
 
   DCHECK(device_id == tether_host_to_connect->GetDeviceId());
 
+  // TODO (hansberry): Indicate to ConnectTetheringOperation if first-time setup
+  // is required, so that it can adjust its timeout duration.
   connect_tethering_operation_ =
       ConnectTetheringOperation::Factory::NewInstance(
           *tether_host_to_connect, connection_manager_,
@@ -197,6 +213,8 @@ void TetherConnector::SetConnectionFailed(const std::string& error_name) {
   DCHECK(!device_id_pending_connection_.empty());
   DCHECK(!error_callback_.is_null());
 
+  notification_presenter_->RemoveSetupRequiredNotification();
+
   // Save a copy of the callback before resetting it below.
   network_handler::StringResultCallback error_callback = error_callback_;
 
@@ -215,6 +233,8 @@ void TetherConnector::SetConnectionSucceeded(
   DCHECK(device_id_pending_connection_ == device_id);
   DCHECK(!success_callback_.is_null());
 
+  notification_presenter_->RemoveSetupRequiredNotification();
+
   // Save a copy of the callback before resetting it below.
   base::Closure success_callback = success_callback_;
 

chromeos/components/tether/tether_connector.h

@@ -19,6 +19,8 @@ namespace tether {
 class ActiveHost;
 class BleConnectionManager;
 class DeviceIdTetherNetworkGuidMap;
+class HostScanCache;
+class NotificationPresenter;
 class TetherHostFetcher;
 class TetherHostResponseRecorder;
 class WifiHotspotConnector;
@@ -37,7 +39,9 @@ class TetherConnector : public ConnectTetheringOperation::Observer {
       TetherHostFetcher* tether_host_fetcher,
       BleConnectionManager* connection_manager,
       TetherHostResponseRecorder* tether_host_response_recorder,
-      DeviceIdTetherNetworkGuidMap* device_id_tether_network_guid_map);
+      DeviceIdTetherNetworkGuidMap* device_id_tether_network_guid_map,
+      HostScanCache* host_scan_cache,
+      NotificationPresenter* notification_presenter);
   virtual ~TetherConnector();
 
   virtual void ConnectToNetwork(
@@ -78,6 +82,8 @@ class TetherConnector : public ConnectTetheringOperation::Observer {
   BleConnectionManager* connection_manager_;
   TetherHostResponseRecorder* tether_host_response_recorder_;
   DeviceIdTetherNetworkGuidMap* device_id_tether_network_guid_map_;
+  HostScanCache* host_scan_cache_;
+  NotificationPresenter* notification_presenter_;
 
   std::string device_id_pending_connection_;
   base::Closure success_callback_;

chromeos/components/tether/tether_connector_unittest.cc

@@ -10,6 +10,8 @@
 #include "chromeos/components/tether/device_id_tether_network_guid_map.h"
 #include "chromeos/components/tether/fake_active_host.h"
 #include "chromeos/components/tether/fake_ble_connection_manager.h"
+#include "chromeos/components/tether/fake_host_scan_cache.h"
+#include "chromeos/components/tether/fake_notification_presenter.h"
 #include "chromeos/components/tether/fake_tether_host_fetcher.h"
 #include "chromeos/components/tether/fake_wifi_hotspot_connector.h"
 #include "chromeos/components/tether/mock_tether_host_response_recorder.h"
@@ -131,6 +133,9 @@ class TetherConnectorTest : public NetworkStateTest {
         base::MakeUnique<MockTetherHostResponseRecorder>();
     device_id_tether_network_guid_map_ =
         base::MakeUnique<DeviceIdTetherNetworkGuidMap>();
+    fake_host_scan_cache_ = base::MakeUnique<FakeHostScanCache>();
+    fake_notification_presenter_ =
+        base::MakeUnique<FakeNotificationPresenter>();
 
     result_.clear();
 
@@ -139,7 +144,8 @@ class TetherConnectorTest : public NetworkStateTest {
         fake_active_host_.get(), fake_tether_host_fetcher_.get(),
         fake_ble_connection_manager_.get(),
         mock_tether_host_response_recorder_.get(),
-        device_id_tether_network_guid_map_.get()));
+        device_id_tether_network_guid_map_.get(), fake_host_scan_cache_.get(),
+        fake_notification_presenter_.get()));
 
     SetUpTetherNetworks();
   }
@@ -164,16 +170,31 @@ class TetherConnectorTest : public NetworkStateTest {
     // Add a tether network corresponding to both of the test devices. These
     // networks are expected to be added already before
     // TetherConnector::ConnectToNetwork is called.
+    AddTetherNetwork(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()),
+                     "TetherNetworkName1", "TetherNetworkCarrier1",
+                     85 /* battery_percentage */, 75 /* signal_strength */,
+                     true /* has_connected_to_host */,
+                     false /* setup_required */);
+    AddTetherNetwork(GetTetherNetworkGuid(test_devices_[1].GetDeviceId()),
+                     "TetherNetworkName2", "TetherNetworkCarrier2",
+                     90 /* battery_percentage */, 50 /* signal_strength */,
+                     true /* has_connected_to_host */,
+                     true /* setup_required */);
+  }
+
+  virtual void AddTetherNetwork(const std::string& tether_network_guid,
+                                const std::string& device_name,
+                                const std::string& carrier,
+                                int battery_percentage,
+                                int signal_strength,
+                                bool has_connected_to_host,
+                                bool setup_required) {
     network_state_handler()->AddTetherNetworkState(
-        GetTetherNetworkGuid(test_devices_[0].GetDeviceId()),
-        "TetherNetworkName1", "TetherNetworkCarrier1",
-        85 /* battery_percentage */, 75 /* signal_strength */,
-        true /* has_connected_to_host */);
-    network_state_handler()->AddTetherNetworkState(
-        GetTetherNetworkGuid(test_devices_[1].GetDeviceId()),
-        "TetherNetworkName2", "TetherNetworkCarrier2",
-        90 /* battery_percentage */, 50 /* signal_strength */,
-        true /* has_connected_to_host */);
+        tether_network_guid, device_name, carrier, battery_percentage,
+        signal_strength, has_connected_to_host);
+    fake_host_scan_cache_->SetHostScanResult(tether_network_guid, device_name,
+                                             carrier, battery_percentage,
+                                             signal_strength, setup_required);
   }
 
   void SuccessfullyJoinWifiNetwork() {
@@ -213,6 +234,8 @@ class TetherConnectorTest : public NetworkStateTest {
   // TODO(hansberry): Use a fake for this when a real mapping scheme is created.
   std::unique_ptr<DeviceIdTetherNetworkGuidMap>
       device_id_tether_network_guid_map_;
+  std::unique_ptr<FakeHostScanCache> fake_host_scan_cache_;
+  std::unique_ptr<FakeNotificationPresenter> fake_notification_presenter_;
 
   std::string result_;
 
@@ -292,6 +315,30 @@ TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails) {
   EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
 }
 
+TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails_SetupRequired) {
+  EXPECT_FALSE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  CallConnect(GetTetherNetworkGuid(test_devices_[1].GetDeviceId()));
+
+  EXPECT_TRUE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  fake_tether_host_fetcher_->InvokePendingCallbacks();
+
+  EXPECT_TRUE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  fake_operation_factory_->created_operations()[0]->SendFailedResponse(
+      ConnectTetheringResponse_ResponseCode::
+          ConnectTetheringResponse_ResponseCode_UNKNOWN_ERROR);
+
+  EXPECT_FALSE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
+}
+
 TEST_F(TetherConnectorTest, TestConnectingToWifiFails) {
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
@@ -363,6 +410,8 @@ TEST_F(TetherConnectorTest, TestSuccessfulConnection) {
   EXPECT_EQ(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()),
             fake_active_host_->GetTetherNetworkGuid());
   EXPECT_TRUE(fake_active_host_->GetWifiNetworkGuid().empty());
+  EXPECT_FALSE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
 
   fake_tether_host_fetcher_->InvokePendingCallbacks();
 
@@ -394,6 +443,34 @@ TEST_F(TetherConnectorTest, TestSuccessfulConnection) {
   EXPECT_EQ(kSuccessResult, GetResultAndReset());
 }
 
+TEST_F(TetherConnectorTest, TestSuccessfulConnection_SetupRequired) {
+  EXPECT_FALSE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  CallConnect(GetTetherNetworkGuid(test_devices_[1].GetDeviceId()));
+
+  EXPECT_TRUE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  fake_tether_host_fetcher_->InvokePendingCallbacks();
+
+  EXPECT_TRUE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  fake_operation_factory_->created_operations()[0]->SendSuccessfulResponse(
+      kSsid, kPassword);
+
+  EXPECT_TRUE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  SuccessfullyJoinWifiNetwork();
+
+  EXPECT_FALSE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  EXPECT_EQ(kSuccessResult, GetResultAndReset());
+}
+
 TEST_F(TetherConnectorTest,
        TestNewConnectionAttemptDuringFetch_DifferentDevice) {
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));

chromeos/components/tether/tether_disconnector_unittest.cc

@@ -48,7 +48,7 @@ std::string CreateConnectedWifiConfigurationJsonString() {
 
 class TestNetworkConnectionHandler : public NetworkConnectionHandler {
  public:
-  TestNetworkConnectionHandler(base::Closure disconnect_callback)
+  explicit TestNetworkConnectionHandler(base::Closure disconnect_callback)
       : disconnect_callback_(disconnect_callback) {}
   ~TestNetworkConnectionHandler() override {}
 
@@ -105,7 +105,9 @@ class TestTetherConnector : public TetherConnector {
                         nullptr /* tether_host_fetcher */,
                         nullptr /* connection_manager */,
                         nullptr /* tether_host_response_recorder */,
-                        nullptr /* device_id_tether_network_guid_map */),
+                        nullptr /* device_id_tether_network_guid_map */,
+                        nullptr /* host_scan_cache */,
+                        nullptr /* notification_presenter */),
         should_cancel_successfully_(true) {}
   ~TestTetherConnector() override {}