From 387dc454ec4618108b33925bde37c083ae3e8d55 Mon Sep 17 00:00:00 2001 From: hansberry Date: Wed, 31 May 2017 18:49:59 -0700 Subject: [PATCH] Tether: Persist if first-time setup is required to HostScanCache. BUG=672263 Review-Url: https://codereview.chromium.org/2913313002 Cr-Commit-Position: refs/heads/master@{#476140} --- .../components/tether/fake_host_scan_cache.cc | 19 +++++++++--- .../components/tether/fake_host_scan_cache.h | 5 ++- chromeos/components/tether/host_scan_cache.cc | 15 ++++++++- chromeos/components/tether/host_scan_cache.h | 11 ++++++- .../tether/host_scan_cache_unittest.cc | 31 ++++++++++++++----- chromeos/components/tether/host_scanner.cc | 3 +- .../tether/host_scanner_unittest.cc | 6 ++-- 7 files changed, 72 insertions(+), 18 deletions(-) diff --git a/chromeos/components/tether/fake_host_scan_cache.cc b/chromeos/components/tether/fake_host_scan_cache.cc index 917a13a0f0a445..fae2ff673e2d9c 100644 --- a/chromeos/components/tether/fake_host_scan_cache.cc +++ b/chromeos/components/tether/fake_host_scan_cache.cc @@ -51,7 +51,8 @@ void FakeHostScanCache::SetHostScanResult( const std::string& device_name, const std::string& carrier, int battery_percentage, - int signal_strength) { + int signal_strength, + bool setup_required) { auto it = cache_.find(tether_network_guid); if (it != cache_.end()) { // If already in the cache, update the cache with new values. @@ -59,13 +60,14 @@ void FakeHostScanCache::SetHostScanResult( it->second.carrier = carrier; it->second.battery_percentage = battery_percentage; it->second.signal_strength = signal_strength; + it->second.setup_required = setup_required; return; } // Otherwise, add a new entry. - cache_.emplace( - tether_network_guid, - CacheEntry{device_name, carrier, battery_percentage, signal_strength}); + cache_.emplace(tether_network_guid, + CacheEntry{device_name, carrier, battery_percentage, + signal_strength, setup_required}); } bool FakeHostScanCache::RemoveHostScanResult( @@ -86,6 +88,15 @@ void FakeHostScanCache::ClearCacheExceptForActiveHost() { } } +bool FakeHostScanCache::DoesHostRequireSetup( + const std::string& tether_network_guid) { + auto it = cache_.find(tether_network_guid); + if (it != cache_.end()) + return it->second.setup_required; + + return false; +} + void FakeHostScanCache::OnPreviouslyConnectedHostIdsChanged() {} } // namespace tether diff --git a/chromeos/components/tether/fake_host_scan_cache.h b/chromeos/components/tether/fake_host_scan_cache.h index 1b390187e164cc..e63c3fb8724a71 100644 --- a/chromeos/components/tether/fake_host_scan_cache.h +++ b/chromeos/components/tether/fake_host_scan_cache.h @@ -23,6 +23,7 @@ class FakeHostScanCache : public HostScanCache { std::string carrier; int battery_percentage; int signal_strength; + bool setup_required; }; FakeHostScanCache(); @@ -52,9 +53,11 @@ class FakeHostScanCache : public HostScanCache { const std::string& device_name, const std::string& carrier, int battery_percentage, - int signal_strength) override; + int signal_strength, + bool setup_required) override; bool RemoveHostScanResult(const std::string& tether_network_guid) override; void ClearCacheExceptForActiveHost() override; + bool DoesHostRequireSetup(const std::string& tether_network_guid) override; void OnPreviouslyConnectedHostIdsChanged() override; private: diff --git a/chromeos/components/tether/host_scan_cache.cc b/chromeos/components/tether/host_scan_cache.cc index c059aebe095dd9..51696431e1996a 100644 --- a/chromeos/components/tether/host_scan_cache.cc +++ b/chromeos/components/tether/host_scan_cache.cc @@ -41,7 +41,8 @@ void HostScanCache::SetHostScanResult(const std::string& tether_network_guid, const std::string& device_name, const std::string& carrier, int battery_percentage, - int signal_strength) { + int signal_strength, + bool setup_required) { DCHECK(!tether_network_guid.empty()); auto found_iter = tether_guid_to_timer_map_.find(tether_network_guid); @@ -72,6 +73,11 @@ void HostScanCache::SetHostScanResult(const std::string& tether_network_guid, << "new signal strength: " << signal_strength; } + if (setup_required) + setup_required_tether_guids_.insert(tether_network_guid); + else + setup_required_tether_guids_.erase(tether_network_guid); + StartTimer(tether_network_guid); } @@ -95,6 +101,7 @@ bool HostScanCache::RemoveHostScanResult( } tether_guid_to_timer_map_.erase(it); + setup_required_tether_guids_.erase(tether_network_guid); return network_state_handler_->RemoveTetherNetworkState(tether_network_guid); } @@ -133,6 +140,12 @@ void HostScanCache::ClearCacheExceptForActiveHost() { } } +bool HostScanCache::DoesHostRequireSetup( + const std::string& tether_network_guid) { + return setup_required_tether_guids_.find(tether_network_guid) != + setup_required_tether_guids_.end(); +} + void HostScanCache::OnPreviouslyConnectedHostIdsChanged() { for (auto& map_entry : tether_guid_to_timer_map_) { const std::string& tether_network_guid = map_entry.first; diff --git a/chromeos/components/tether/host_scan_cache.h b/chromeos/components/tether/host_scan_cache.h index 3d705e48887bef..762f3ef9715cfe 100644 --- a/chromeos/components/tether/host_scan_cache.h +++ b/chromeos/components/tether/host_scan_cache.h @@ -58,11 +58,15 @@ class HostScanCache : public TetherHostResponseRecorder::Observer { // Note: |signal_strength| should be in the range [0, 100]. This is different // from the |connection_strength| field received in ConnectTetheringResponse // and KeepAliveTickleResponse messages (the range is [0, 4] in those cases). + // |battery_percentage| should also be in the range [0, 100]. + // |setup_required| indicates that the host device requires first-time setup, + // i.e., user interaction to allow tethering. virtual void SetHostScanResult(const std::string& tether_network_guid, const std::string& device_name, const std::string& carrier, int battery_percentage, - int signal_strength); + int signal_strength, + bool setup_required); // Removes the scan result with GUID |tether_network_guid| from the cache. If // no cache result with that GUID was present in the cache, this function is @@ -74,6 +78,10 @@ class HostScanCache : public TetherHostResponseRecorder::Observer { // connecting/connected to ensure the UI is up to date. virtual void ClearCacheExceptForActiveHost(); + // Returns true if the host device requires first-time setup, i.e., user + // interaction to allow tethering. + virtual bool DoesHostRequireSetup(const std::string& tether_network_guid); + // TetherHostResponseRecorder::Observer: void OnPreviouslyConnectedHostIdsChanged() override; @@ -99,6 +107,7 @@ class HostScanCache : public TetherHostResponseRecorder::Observer { // host). std::unordered_map> tether_guid_to_timer_map_; + std::unordered_set setup_required_tether_guids_; base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(HostScanCache); diff --git a/chromeos/components/tether/host_scan_cache_unittest.cc b/chromeos/components/tether/host_scan_cache_unittest.cc index d0e3640012dbe1..ae09f5ec82f280 100644 --- a/chromeos/components/tether/host_scan_cache_unittest.cc +++ b/chromeos/components/tether/host_scan_cache_unittest.cc @@ -58,6 +58,11 @@ const int kTetherSignalStrength1 = 50; const int kTetherSignalStrength2 = 75; const int kTetherSignalStrength3 = 100; +const bool kTetherSetupRequired0 = true; +const bool kTetherSetupRequired1 = false; +const bool kTetherSetupRequired2 = true; +const bool kTetherSetupRequired3 = false; + // MockTimer which invokes a callback in its destructor. class ExtendedMockTimer : public base::MockTimer { public: @@ -205,6 +210,7 @@ class HostScanCacheTest : public NetworkStateTest { std::string carrier; int battery_percentage; int signal_strength; + bool setup_required; switch (index) { case 0: @@ -213,6 +219,7 @@ class HostScanCacheTest : public NetworkStateTest { carrier = kTetherCarrier0; battery_percentage = kTetherBatteryPercentage0; signal_strength = kTetherSignalStrength0; + setup_required = kTetherSetupRequired0; break; case 1: tether_network_guid = kTetherGuid1; @@ -220,6 +227,7 @@ class HostScanCacheTest : public NetworkStateTest { carrier = kTetherCarrier1; battery_percentage = kTetherBatteryPercentage1; signal_strength = kTetherSignalStrength1; + setup_required = kTetherSetupRequired1; break; case 2: tether_network_guid = kTetherGuid2; @@ -227,6 +235,7 @@ class HostScanCacheTest : public NetworkStateTest { carrier = kTetherCarrier2; battery_percentage = kTetherBatteryPercentage2; signal_strength = kTetherSignalStrength2; + setup_required = kTetherSetupRequired2; break; case 3: tether_network_guid = kTetherGuid3; @@ -234,34 +243,37 @@ class HostScanCacheTest : public NetworkStateTest { carrier = kTetherCarrier3; battery_percentage = kTetherBatteryPercentage3; signal_strength = kTetherSignalStrength3; + setup_required = kTetherSetupRequired3; break; default: NOTREACHED(); - // Set values for |battery_percentage| and |signal_strength| here to - // prevent a compiler warning which says that they may be unset at this - // point. + // Set values for |battery_percentage|, |signal_strength| and + // |setup_required| here to prevent a compiler warning which says that + // they may be unset at this point. battery_percentage = 0; signal_strength = 0; + setup_required = false; break; } SetHostScanResult(tether_network_guid, device_name, carrier, - battery_percentage, signal_strength); + battery_percentage, signal_strength, setup_required); } void SetHostScanResult(const std::string& tether_network_guid, const std::string& device_name, const std::string& carrier, int battery_percentage, - int signal_strength) { + int signal_strength, + bool setup_required) { test_timer_factory_->set_tether_network_guid_for_next_timer( tether_network_guid); host_scan_cache_->SetHostScanResult(tether_network_guid, device_name, carrier, battery_percentage, - signal_strength); + signal_strength, setup_required); expected_cache_->SetHostScanResult(tether_network_guid, device_name, carrier, battery_percentage, - signal_strength); + signal_strength, setup_required); } void RemoveHostScanResult(const std::string& tether_network_guid) { @@ -300,6 +312,8 @@ class HostScanCacheTest : public NetworkStateTest { tether_network_state->battery_percentage()); EXPECT_EQ(cache_entry.signal_strength, tether_network_state->signal_strength()); + EXPECT_EQ(cache_entry.setup_required, + host_scan_cache_->DoesHostRequireSetup(tether_network_guid)); EXPECT_EQ(HasConnectedToHost(tether_network_guid), tether_network_state->tether_has_connected_to_host()); @@ -373,7 +387,8 @@ TEST_F(HostScanCacheTest, TestSetScanResultThenUpdateAndRemove) { // Change the fields for tether network with GUID |kTetherGuid0| to the // fields corresponding to |kTetherGuid1|. SetHostScanResult(kTetherGuid0, kTetherDeviceName0, kTetherCarrier1, - kTetherBatteryPercentage1, kTetherSignalStrength1); + kTetherBatteryPercentage1, kTetherSignalStrength1, + kTetherSetupRequired1); EXPECT_EQ(1u, expected_cache_->size()); VerifyCacheMatchesNetworkStack(); diff --git a/chromeos/components/tether/host_scanner.cc b/chromeos/components/tether/host_scanner.cc index a8616ef385623c..31308e88862077 100644 --- a/chromeos/components/tether/host_scanner.cc +++ b/chromeos/components/tether/host_scanner.cc @@ -167,7 +167,8 @@ void HostScanner::SetCacheEntry( host_scan_cache_->SetHostScanResult( device_id_tether_network_guid_map_->GetTetherNetworkGuidForDeviceId( remote_device.GetDeviceId()), - remote_device.name, carrier, battery_percentage, signal_strength); + remote_device.name, carrier, battery_percentage, signal_strength, + scanned_device_info.setup_required); } } // namespace tether diff --git a/chromeos/components/tether/host_scanner_unittest.cc b/chromeos/components/tether/host_scanner_unittest.cc index e793b40ced80fe..b954d3cffce1a4 100644 --- a/chromeos/components/tether/host_scanner_unittest.cc +++ b/chromeos/components/tether/host_scanner_unittest.cc @@ -196,10 +196,10 @@ CreateFakeScannedDeviceInfos( cell_provider_name, battery_percentage, connection_strength); // Require set-up for odd-numbered device indices. - bool set_up_required = i % 2 == 0; + bool setup_required = i % 2 == 0; scanned_device_infos.push_back(HostScannerOperation::ScannedDeviceInfo( - remote_devices[i], device_status, set_up_required)); + remote_devices[i], device_status, setup_required)); } return scanned_device_infos; @@ -332,6 +332,8 @@ class HostScannerTest : public testing::Test { EXPECT_EQ(0, cache_item.signal_strength); else EXPECT_EQ(status.connection_strength() * 25, cache_item.signal_strength); + + EXPECT_EQ(scanned_device_info.setup_required, cache_item.setup_required); } const std::vector test_devices_;