[CrOS Tether] Delay host scans slightly after unlock.

This helps to avoid bad interactions between Instant Tethering and
EasyUnlock Bluetooth channels.

Bug: 763604, 672263
Change-Id: I6ecca447d8f7719c6c9c7263950c2c20799eeac7
Reviewed-on: https://chromium-review.googlesource.com/941583
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539939}

chromeos/components/tether/host_scan_scheduler_impl.cc

@@ -31,6 +31,11 @@ namespace {
 // seconds apart.
 const int64_t kMaxNumSecondsBetweenBatchScans = 60;
 
+// Scanning immediately after the device is unlocked may cause unwanted
+// interactions with EasyUnlock BLE channels. The scan is delayed slightly in
+// order to circumvent this issue.
+const int64_t kNumSecondsToDelayScanAfterUnlock = 3;
+
 // Minimum value for the scan length metric.
 const int64_t kMinScanMetricSeconds = 1;
 
@@ -49,7 +54,8 @@ HostScanSchedulerImpl::HostScanSchedulerImpl(
     : network_state_handler_(network_state_handler),
       host_scanner_(host_scanner),
       session_manager_(session_manager),
-      timer_(std::make_unique<base::OneShotTimer>()),
+      host_scan_batch_timer_(std::make_unique<base::OneShotTimer>()),
+      delay_scan_after_unlock_timer_(std::make_unique<base::OneShotTimer>()),
       clock_(base::DefaultClock::GetInstance()),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       is_screen_locked_(session_manager_->IsScreenLocked()),
@@ -67,7 +73,7 @@ HostScanSchedulerImpl::~HostScanSchedulerImpl() {
 
   // If the most recent batch of host scans has already been logged, return
   // early.
-  if (!host_scanner_->IsScanActive() && !timer_->IsRunning())
+  if (!host_scanner_->IsScanActive() && !host_scan_batch_timer_->IsRunning())
     return;
 
   // If a scan is still active during shutdown, there is not enough time to wait
@@ -108,10 +114,10 @@ void HostScanSchedulerImpl::ScanFinished() {
   network_state_handler_->SetTetherScanState(false);
 
   last_scan_end_timestamp_ = clock_->Now();
-  timer_->Start(FROM_HERE,
-                base::TimeDelta::FromSeconds(kMaxNumSecondsBetweenBatchScans),
-                base::Bind(&HostScanSchedulerImpl::LogHostScanBatchMetric,
-                           weak_ptr_factory_.GetWeakPtr()));
+  host_scan_batch_timer_->Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(kMaxNumSecondsBetweenBatchScans),
+      base::Bind(&HostScanSchedulerImpl::LogHostScanBatchMetric,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void HostScanSchedulerImpl::OnSessionStateChanged() {
@@ -125,6 +131,7 @@ void HostScanSchedulerImpl::OnSessionStateChanged() {
     // Note: Once the SecureChannel API is in use, the scan will no longer have
     //       to stop.
     host_scanner_->StopScan();
+    delay_scan_after_unlock_timer_->Stop();
     return;
   }
 
@@ -132,14 +139,21 @@ void HostScanSchedulerImpl::OnSessionStateChanged() {
     return;
 
   // If the device was just unlocked, start a scan.
-  EnsureScan();
+  delay_scan_after_unlock_timer_->Start(
+      FROM_HERE,
+      base::TimeDelta::FromSeconds(kNumSecondsToDelayScanAfterUnlock),
+      base::Bind(&HostScanSchedulerImpl::EnsureScan,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void HostScanSchedulerImpl::SetTestDoubles(
-    std::unique_ptr<base::Timer> test_timer,
+    std::unique_ptr<base::Timer> test_host_scan_batch_timer,
+    std::unique_ptr<base::Timer> test_delay_scan_after_unlock_timer,
     base::Clock* test_clock,
     scoped_refptr<base::TaskRunner> test_task_runner) {
-  timer_ = std::move(test_timer);
+  host_scan_batch_timer_ = std::move(test_host_scan_batch_timer);
+  delay_scan_after_unlock_timer_ =
+      std::move(test_delay_scan_after_unlock_timer);
   clock_ = test_clock;
   task_runner_ = test_task_runner;
 }
@@ -152,11 +166,12 @@ void HostScanSchedulerImpl::EnsureScan() {
   // previous scan, so the timer should be stopped (it will be restarted after
   // the new scan finishes). If the timer is not running, the new scan is part
   // of a new batch, so the start timestamp should be recorded.
-  if (timer_->IsRunning())
-    timer_->Stop();
+  if (host_scan_batch_timer_->IsRunning())
+    host_scan_batch_timer_->Stop();
   else
     last_scan_batch_start_timestamp_ = clock_->Now();
 
+  delay_scan_after_unlock_timer_->Stop();
   host_scanner_->StartScan();
   network_state_handler_->SetTetherScanState(true);
 }

chromeos/components/tether/host_scan_scheduler_impl.h

@@ -67,15 +67,18 @@ class HostScanSchedulerImpl : public HostScanScheduler,
   bool IsTetherNetworkConnectingOrConnected();
   void LogHostScanBatchMetric();
 
-  void SetTestDoubles(std::unique_ptr<base::Timer> test_timer,
-                      base::Clock* test_clock,
-                      scoped_refptr<base::TaskRunner> test_task_runner);
+  void SetTestDoubles(
+      std::unique_ptr<base::Timer> test_host_scan_batch_timer,
+      std::unique_ptr<base::Timer> test_delay_scan_after_unlock_timer,
+      base::Clock* test_clock,
+      scoped_refptr<base::TaskRunner> test_task_runner);
 
   NetworkStateHandler* network_state_handler_;
   HostScanner* host_scanner_;
   session_manager::SessionManager* session_manager_;
 
-  std::unique_ptr<base::Timer> timer_;
+  std::unique_ptr<base::Timer> host_scan_batch_timer_;
+  std::unique_ptr<base::Timer> delay_scan_after_unlock_timer_;
   base::Clock* clock_;
   scoped_refptr<base::TaskRunner> task_runner_;
 

chromeos/components/tether/host_scan_scheduler_impl_unittest.cc

@@ -73,14 +73,19 @@ class HostScanSchedulerImplTest : public NetworkStateTest {
         network_state_handler(), fake_host_scanner_.get(),
         session_manager_.get());
 
-    mock_timer_ = new base::MockTimer(true /* retain_user_task */,
-                                      false /* is_repeating */);
+    mock_host_scan_batch_timer_ = new base::MockTimer(
+        true /* retain_user_task */, false /* is_repeating */);
+    mock_delay_scan_after_unlock_timer_ = new base::MockTimer(
+        true /* retain_user_task */, false /* is_repeating */);
+
     // Advance the clock by an arbitrary value to ensure that when Now() is
     // called, the Unix epoch will not be returned.
     test_clock_.Advance(base::TimeDelta::FromSeconds(10));
     test_task_runner_ = base::MakeRefCounted<base::TestSimpleTaskRunner>();
-    host_scan_scheduler_->SetTestDoubles(base::WrapUnique(mock_timer_),
-                                         &test_clock_, test_task_runner_);
+    host_scan_scheduler_->SetTestDoubles(
+        base::WrapUnique(mock_host_scan_batch_timer_),
+        base::WrapUnique(mock_delay_scan_after_unlock_timer_), &test_clock_,
+        test_task_runner_);
   }
 
   void TearDown() override {
@@ -159,7 +164,8 @@ class HostScanSchedulerImplTest : public NetworkStateTest {
   std::unique_ptr<FakeHostScanner> fake_host_scanner_;
   std::unique_ptr<session_manager::SessionManager> session_manager_;
 
-  base::MockTimer* mock_timer_;
+  base::MockTimer* mock_host_scan_batch_timer_;
+  base::MockTimer* mock_delay_scan_after_unlock_timer_;
   base::SimpleTestClock test_clock_;
   scoped_refptr<base::TestSimpleTaskRunner> test_task_runner_;
 
@@ -181,7 +187,7 @@ TEST_F(HostScanSchedulerImplTest, ScheduleScan) {
       network_state_handler()->GetScanningByType(NetworkTypePattern::Tether()));
 
   // Fire the timer; the duration should be recorded.
-  mock_timer_->Fire();
+  mock_host_scan_batch_timer_->Fire();
   VerifyScanDuration(5u /* expected_num_sections */);
 }
 
@@ -189,9 +195,16 @@ TEST_F(HostScanSchedulerImplTest, ScansWhenDeviceUnlocked) {
   // Lock the screen. This should not trigger a scan.
   SetScreenLockedState(true /* is_locked */);
   EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
+  EXPECT_FALSE(mock_delay_scan_after_unlock_timer_->IsRunning());
 
-  // Unlock the screen. A scan should start.
+  // Unlock the screen. A scan should not yet have been started, but the timer
+  // should have.
   SetScreenLockedState(false /* is_locked */);
+  EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
+  ASSERT_TRUE(mock_delay_scan_after_unlock_timer_->IsRunning());
+
+  // Fire the timer; this should start the scan.
+  mock_delay_scan_after_unlock_timer_->Fire();
   EXPECT_EQ(1u, fake_host_scanner_->num_scans_started());
 }
 
@@ -213,7 +226,7 @@ TEST_F(HostScanSchedulerImplTest, ScanRequested) {
   EXPECT_EQ(1u, fake_host_scanner_->num_scans_started());
   EXPECT_FALSE(
       network_state_handler()->GetScanningByType(NetworkTypePattern::Tether()));
-  mock_timer_->Fire();
+  mock_host_scan_batch_timer_->Fire();
   VerifyScanDuration(5u /* expected_num_sections */);
 
   // A new scan should be allowed once a scan is not active.
@@ -243,39 +256,42 @@ TEST_F(HostScanSchedulerImplTest, HostScanBatchMetric) {
   host_scan_scheduler_->ScheduleScan();
   test_clock_.Advance(base::TimeDelta::FromSeconds(5));
   fake_host_scanner_->StopScan();
-  EXPECT_TRUE(mock_timer_->IsRunning());
+  EXPECT_TRUE(mock_host_scan_batch_timer_->IsRunning());
 
   // Advance the clock by 1 second and start another scan. The timer should have
   // been stopped.
   test_clock_.Advance(base::TimeDelta::FromSeconds(1));
-  EXPECT_LT(base::TimeDelta::FromSeconds(1), mock_timer_->GetCurrentDelay());
+  EXPECT_LT(base::TimeDelta::FromSeconds(1),
+            mock_host_scan_batch_timer_->GetCurrentDelay());
   host_scan_scheduler_->ScheduleScan();
-  EXPECT_FALSE(mock_timer_->IsRunning());
+  EXPECT_FALSE(mock_host_scan_batch_timer_->IsRunning());
 
   // Stop the scan; the duration should not have been recorded, and the timer
   // should be running again.
   test_clock_.Advance(base::TimeDelta::FromSeconds(5));
   fake_host_scanner_->StopScan();
-  EXPECT_TRUE(mock_timer_->IsRunning());
+  EXPECT_TRUE(mock_host_scan_batch_timer_->IsRunning());
 
   // Advance the clock by 59 seconds and start another scan. The timer should
   // have been stopped.
   test_clock_.Advance(base::TimeDelta::FromSeconds(59));
-  EXPECT_LT(base::TimeDelta::FromSeconds(59), mock_timer_->GetCurrentDelay());
+  EXPECT_LT(base::TimeDelta::FromSeconds(59),
+            mock_host_scan_batch_timer_->GetCurrentDelay());
   host_scan_scheduler_->ScheduleScan();
-  EXPECT_FALSE(mock_timer_->IsRunning());
+  EXPECT_FALSE(mock_host_scan_batch_timer_->IsRunning());
 
   // Stop the scan; the duration should not have been recorded, and the timer
   // should be running again.
   test_clock_.Advance(base::TimeDelta::FromSeconds(5));
   fake_host_scanner_->StopScan();
-  EXPECT_TRUE(mock_timer_->IsRunning());
+  EXPECT_TRUE(mock_host_scan_batch_timer_->IsRunning());
 
   // Advance the clock by 60 seconds, which should be equal to the timer's
   // delay. Since this is a MockTimer, we need to manually fire the timer.
   test_clock_.Advance(base::TimeDelta::FromSeconds(60));
-  EXPECT_EQ(base::TimeDelta::FromSeconds(60), mock_timer_->GetCurrentDelay());
-  mock_timer_->Fire();
+  EXPECT_EQ(base::TimeDelta::FromSeconds(60),
+            mock_host_scan_batch_timer_->GetCurrentDelay());
+  mock_host_scan_batch_timer_->Fire();
 
   // The scan duration should be equal to the three 5-second scans as well as
   // the 1-second and 59-second breaks between the three scans.
@@ -286,10 +302,11 @@ TEST_F(HostScanSchedulerImplTest, HostScanBatchMetric) {
   host_scan_scheduler_->ScheduleScan();
   test_clock_.Advance(base::TimeDelta::FromSeconds(5));
   fake_host_scanner_->StopScan();
-  EXPECT_TRUE(mock_timer_->IsRunning());
+  EXPECT_TRUE(mock_host_scan_batch_timer_->IsRunning());
   test_clock_.Advance(base::TimeDelta::FromSeconds(60));
-  EXPECT_EQ(base::TimeDelta::FromSeconds(60), mock_timer_->GetCurrentDelay());
-  mock_timer_->Fire();
+  EXPECT_EQ(base::TimeDelta::FromSeconds(60),
+            mock_host_scan_batch_timer_->GetCurrentDelay());
+  mock_host_scan_batch_timer_->Fire();
   VerifyScanDuration(5u /* expected_num_sections */);
 }