Instant Tethering: Remove kMultiDeviceApi flagging from HostScanSchedulerImpl.

R=hansberry@chromium.org

Bug: 903991
Test: none
Change-Id: I85c5d0efcdf3c954d7756d2ca2952c06c56dd509
Reviewed-on: https://chromium-review.googlesource.com/c/1333899
Commit-Queue: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607711}

chromeos/components/tether/host_scan_scheduler_impl.cc

@@ -33,13 +33,6 @@ namespace {
 // seconds apart.
 const int64_t kMaxNumSecondsBetweenBatchScans = 60;
 
-// If Tether and Smart Lock use their own BLE channel logic, instead of the
-// shared SecureChannel API (i.e. |chromeos::features::kMultiDeviceApi| is
-// disabled), scanning immediately after the device is unlocked may cause
-// unwanted interactions with Smart Lock 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;
 
@@ -59,7 +52,6 @@ HostScanSchedulerImpl::HostScanSchedulerImpl(
       host_scanner_(host_scanner),
       session_manager_(session_manager),
       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()),
       ignore_wired_networks_(false),
@@ -144,8 +136,6 @@ void HostScanSchedulerImpl::OnSessionStateChanged() {
   if (is_screen_locked_) {
     // If the screen is now locked, stop any ongoing scan.
     host_scanner_->StopScan();
-    if (!base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
-      delay_scan_after_unlock_timer_->Stop();
     return;
   }
 
@@ -154,25 +144,14 @@ void HostScanSchedulerImpl::OnSessionStateChanged() {
 
   // If the device was just unlocked, start a scan if not already connected to
   // a network.
-  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
-    AttemptScanIfOffline();
-  } else {
-    delay_scan_after_unlock_timer_->Start(
-        FROM_HERE,
-        base::TimeDelta::FromSeconds(kNumSecondsToDelayScanAfterUnlock),
-        base::BindRepeating(&HostScanSchedulerImpl::AttemptScanIfOffline,
-                            weak_ptr_factory_.GetWeakPtr()));
-  }
+  AttemptScanIfOffline();
 }
 
 void HostScanSchedulerImpl::SetTestDoubles(
     std::unique_ptr<base::OneShotTimer> test_host_scan_batch_timer,
-    std::unique_ptr<base::OneShotTimer> test_delay_scan_after_unlock_timer,
     base::Clock* test_clock,
     scoped_refptr<base::TaskRunner> test_task_runner) {
   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;
 }
@@ -197,9 +176,6 @@ void HostScanSchedulerImpl::AttemptScan() {
   else
     last_scan_batch_start_timestamp_ = clock_->Now();
 
-  if (!base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
-    delay_scan_after_unlock_timer_->Stop();
-
   host_scanner_->StartScan();
   network_state_handler_->SetTetherScanState(true);
 }

chromeos/components/tether/host_scan_scheduler_impl.h

@@ -71,7 +71,6 @@ class HostScanSchedulerImpl : public HostScanScheduler,
 
   void SetTestDoubles(
       std::unique_ptr<base::OneShotTimer> test_host_scan_batch_timer,
-      std::unique_ptr<base::OneShotTimer> test_delay_scan_after_unlock_timer,
       base::Clock* test_clock,
       scoped_refptr<base::TaskRunner> test_task_runner);
 
@@ -80,7 +79,6 @@ class HostScanSchedulerImpl : public HostScanScheduler,
   session_manager::SessionManager* session_manager_;
 
   std::unique_ptr<base::OneShotTimer> host_scan_batch_timer_;
-  std::unique_ptr<base::OneShotTimer> delay_scan_after_unlock_timer_;
   base::Clock* clock_;
   scoped_refptr<base::TaskRunner> task_runner_;
 

chromeos/components/tether/host_scan_scheduler_impl_unittest.cc

@@ -57,18 +57,6 @@ class HostScanSchedulerImplTest : public NetworkStateTest {
   void SetUp() override {
     DBusThreadManager::Initialize();
     NetworkStateTest::SetUp();
-  }
-
-  void TearDown() override {
-    host_scan_scheduler_.reset();
-
-    ShutdownNetworkState();
-    NetworkStateTest::TearDown();
-    DBusThreadManager::Shutdown();
-  }
-
-  void InitializeTest(bool multidevice_flags_enabled) {
-    SetMultiDeviceApi(multidevice_flags_enabled);
 
     histogram_tester_ = std::make_unique<base::HistogramTester>();
 
@@ -83,28 +71,22 @@ class HostScanSchedulerImplTest : public NetworkStateTest {
         session_manager_.get());
 
     mock_host_scan_batch_timer_ = new base::MockOneShotTimer();
-    mock_delay_scan_after_unlock_timer_ = new base::MockOneShotTimer();
 
     // 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_host_scan_batch_timer_),
-        base::WrapUnique(mock_delay_scan_after_unlock_timer_), &test_clock_,
+        base::WrapUnique(mock_host_scan_batch_timer_), &test_clock_,
         test_task_runner_);
   }
 
-  void SetMultiDeviceApi(bool enabled) {
-    static const std::vector<base::Feature> kFeatures{
-        chromeos::features::kMultiDeviceApi,
-        chromeos::features::kEnableUnifiedMultiDeviceSetup};
+  void TearDown() override {
+    host_scan_scheduler_.reset();
 
-    scoped_feature_list_.InitWithFeatures(
-        (enabled ? kFeatures
-                 : std::vector<base::Feature>() /* enable_features */),
-        (enabled ? std::vector<base::Feature>()
-                 : kFeatures /* disable_features */));
+    ShutdownNetworkState();
+    NetworkStateTest::TearDown();
+    DBusThreadManager::Shutdown();
   }
 
   void RequestScan(const NetworkTypePattern& type) {
@@ -172,54 +154,6 @@ class HostScanSchedulerImplTest : public NetworkStateTest {
                   : session_manager::SessionState::LOGIN_PRIMARY);
   }
 
-  void TestDeviceLockAndUnlock(bool is_online) {
-    if (is_online)
-      InitializeEthernet();
-
-    // Lock the screen. This should never 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());
-
-    // Try to start a scan. Because the screen is locked, this should not
-    // cause a scan to be started.
-    host_scan_scheduler_->AttemptScanIfOffline();
-    EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
-    EXPECT_FALSE(mock_delay_scan_after_unlock_timer_->IsRunning());
-
-    // Unlock the screen. A scan should not yet have been scheduled yet, 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 only start the scan if the device is offline.
-    mock_delay_scan_after_unlock_timer_->Fire();
-    EXPECT_EQ(is_online ? 0u : 1u, fake_host_scanner_->num_scans_started());
-  }
-
-  void TestDeviceLockAndUnlock_MultiDeviceApiEnabled(bool is_online) {
-    if (is_online)
-      InitializeEthernet();
-
-    // Lock the screen. This should never 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());
-
-    // Try to start a scan. Because the screen is locked, this should not
-    // cause a scan to be started.
-    host_scan_scheduler_->AttemptScanIfOffline();
-    EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
-    EXPECT_FALSE(mock_delay_scan_after_unlock_timer_->IsRunning());
-
-    // Unlock the screen. If the device is offline, a new scan should have
-    // started. The timer should be untouched.
-    SetScreenLockedState(false /* is_locked */);
-    EXPECT_EQ(is_online ? 0u : 1u, fake_host_scanner_->num_scans_started());
-    EXPECT_FALSE(mock_delay_scan_after_unlock_timer_->IsRunning());
-  }
-
   void VerifyScanDuration(size_t expected_num_seconds) {
     histogram_tester_->ExpectTimeBucketCount(
         "InstantTethering.HostScanBatchDuration",
@@ -233,7 +167,6 @@ class HostScanSchedulerImplTest : public NetworkStateTest {
   std::unique_ptr<session_manager::SessionManager> session_manager_;
 
   base::MockOneShotTimer* mock_host_scan_batch_timer_;
-  base::MockOneShotTimer* mock_delay_scan_after_unlock_timer_;
   base::SimpleTestClock test_clock_;
   scoped_refptr<base::TestSimpleTaskRunner> test_task_runner_;
 
@@ -245,7 +178,6 @@ class HostScanSchedulerImplTest : public NetworkStateTest {
 };
 
 TEST_F(HostScanSchedulerImplTest, AttemptScanIfOffline) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   host_scan_scheduler_->AttemptScanIfOffline();
   EXPECT_EQ(1u, fake_host_scanner_->num_scans_started());
   EXPECT_TRUE(
@@ -263,30 +195,41 @@ TEST_F(HostScanSchedulerImplTest, AttemptScanIfOffline) {
 }
 
 TEST_F(HostScanSchedulerImplTest, TestDeviceLockAndUnlock_Offline) {
-  InitializeTest(false /* multidevice_flags_enabled */);
-  TestDeviceLockAndUnlock(false /* is_online */);
+  // Lock the screen. This should never trigger a scan.
+  SetScreenLockedState(true /* is_locked */);
+  EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
+
+  // Try to start a scan. Because the screen is locked, this should not
+  // cause a scan to be started.
+  host_scan_scheduler_->AttemptScanIfOffline();
+  EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
+
+  // Unlock the screen. Because the device is offline, a new scan should have
+  // started.
+  SetScreenLockedState(false /* is_locked */);
+  EXPECT_EQ(1u, fake_host_scanner_->num_scans_started());
 }
 
 TEST_F(HostScanSchedulerImplTest, TestDeviceLockAndUnlock_Online) {
-  InitializeTest(false /* multidevice_flags_enabled */);
-  TestDeviceLockAndUnlock(true /* is_online */);
-}
+  // Simulate the device being online.
+  InitializeEthernet();
 
-TEST_F(HostScanSchedulerImplTest,
-       TestDeviceLockAndUnlock_MultiDeviceApiEnabled_Offline) {
-  InitializeTest(true /* multidevice_flags_enabled */);
-  TestDeviceLockAndUnlock_MultiDeviceApiEnabled(false /* is_online */);
-}
+  // Lock the screen. This should never trigger a scan.
+  SetScreenLockedState(true /* is_locked */);
+  EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
+
+  // Try to start a scan. Because the screen is locked, this should not
+  // cause a scan to be started.
+  host_scan_scheduler_->AttemptScanIfOffline();
+  EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
 
-TEST_F(HostScanSchedulerImplTest,
-       TestDeviceLockAndUnlock_MultiDeviceApiEnabled_Online) {
-  InitializeTest(true /* multidevice_flags_enabled */);
-  TestDeviceLockAndUnlock_MultiDeviceApiEnabled(true /* is_online */);
+  // Unlock the screen. Because the device is online, a new scan should not have
+  // started.
+  SetScreenLockedState(false /* is_locked */);
+  EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
 }
 
 TEST_F(HostScanSchedulerImplTest, ScanRequested) {
-  InitializeTest(false /* multidevice_flags_enabled */);
-
   // Begin scanning.
   RequestScan(NetworkTypePattern::Tether());
   EXPECT_EQ(1u, fake_host_scanner_->num_scans_started());
@@ -315,7 +258,6 @@ TEST_F(HostScanSchedulerImplTest, ScanRequested) {
 }
 
 TEST_F(HostScanSchedulerImplTest, ScanRequested_NonMatchingNetworkTypePattern) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   RequestScan(NetworkTypePattern::WiFi());
   EXPECT_EQ(0u, fake_host_scanner_->num_scans_started());
   EXPECT_FALSE(
@@ -323,8 +265,6 @@ TEST_F(HostScanSchedulerImplTest, ScanRequested_NonMatchingNetworkTypePattern) {
 }
 
 TEST_F(HostScanSchedulerImplTest, HostScanSchedulerDestroyed) {
-  InitializeTest(false /* multidevice_flags_enabled */);
-
   host_scan_scheduler_->AttemptScanIfOffline();
   EXPECT_TRUE(
       network_state_handler()->GetScanningByType(NetworkTypePattern::Tether()));
@@ -339,8 +279,6 @@ TEST_F(HostScanSchedulerImplTest, HostScanSchedulerDestroyed) {
 }
 
 TEST_F(HostScanSchedulerImplTest, HostScanBatchMetric) {
-  InitializeTest(false /* multidevice_flags_enabled */);
-
   // The first scan takes 5 seconds. After stopping, the timer should be
   // running.
   host_scan_scheduler_->AttemptScanIfOffline();
@@ -401,7 +339,6 @@ TEST_F(HostScanSchedulerImplTest, HostScanBatchMetric) {
 }
 
 TEST_F(HostScanSchedulerImplTest, DefaultNetworkChanged) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   InitializeEthernet();
 
   // When no Tether network is present, a scan should start when the default