Skip to content

Commit

Permalink
Tether: Persist if first-time setup is required to HostScanCache.
Browse files Browse the repository at this point in the history
BUG=672263

Review-Url: https://codereview.chromium.org/2913313002
Cr-Commit-Position: refs/heads/master@{#476140}
  • Loading branch information
hansberry authored and Commit Bot committed Jun 1, 2017
1 parent 92cdde0 commit 387dc45
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 18 deletions.
19 changes: 15 additions & 4 deletions chromeos/components/tether/fake_host_scan_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,23 @@ 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.
it->second.device_name = device_name;
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(
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion chromeos/components/tether/fake_host_scan_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class FakeHostScanCache : public HostScanCache {
std::string carrier;
int battery_percentage;
int signal_strength;
bool setup_required;
};

FakeHostScanCache();
Expand Down Expand Up @@ -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:
Expand Down
15 changes: 14 additions & 1 deletion chromeos/components/tether/host_scan_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion chromeos/components/tether/host_scan_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

Expand All @@ -99,6 +107,7 @@ class HostScanCache : public TetherHostResponseRecorder::Observer {
// host).
std::unordered_map<std::string, std::unique_ptr<base::Timer>>
tether_guid_to_timer_map_;
std::unordered_set<std::string> setup_required_tether_guids_;
base::WeakPtrFactory<HostScanCache> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(HostScanCache);
Expand Down
31 changes: 23 additions & 8 deletions chromeos/components/tether/host_scan_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -205,6 +210,7 @@ class HostScanCacheTest : public NetworkStateTest {
std::string carrier;
int battery_percentage;
int signal_strength;
bool setup_required;

switch (index) {
case 0:
Expand All @@ -213,55 +219,61 @@ class HostScanCacheTest : public NetworkStateTest {
carrier = kTetherCarrier0;
battery_percentage = kTetherBatteryPercentage0;
signal_strength = kTetherSignalStrength0;
setup_required = kTetherSetupRequired0;
break;
case 1:
tether_network_guid = kTetherGuid1;
device_name = kTetherDeviceName1;
carrier = kTetherCarrier1;
battery_percentage = kTetherBatteryPercentage1;
signal_strength = kTetherSignalStrength1;
setup_required = kTetherSetupRequired1;
break;
case 2:
tether_network_guid = kTetherGuid2;
device_name = kTetherDeviceName2;
carrier = kTetherCarrier2;
battery_percentage = kTetherBatteryPercentage2;
signal_strength = kTetherSignalStrength2;
setup_required = kTetherSetupRequired2;
break;
case 3:
tether_network_guid = kTetherGuid3;
device_name = kTetherDeviceName3;
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) {
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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();

Expand Down
3 changes: 2 additions & 1 deletion chromeos/components/tether/host_scanner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions chromeos/components/tether/host_scanner_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<cryptauth::RemoteDevice> test_devices_;
Expand Down

0 comments on commit 387dc45

Please sign in to comment.