Instant Tethering: Remove obsolete BLE advertising functionality.

R=hansberry@chromium.org

Bug: 902378
Change-Id: I62b8d80f2e8d41e5f2c63d78cfaccbe16d577da6
Tests: existing
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2288756Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: James Hawkins <jhawkins@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786890}

chrome/browser/chromeos/tether/tether_service.cc

@@ -32,9 +32,6 @@
 
 namespace {
 
-constexpr int64_t kMinAdvertisingIntervalMilliseconds = 100;
-constexpr int64_t kMaxAdvertisingIntervalMilliseconds = 100;
-
 constexpr int64_t kMetricFalsePositiveSeconds = 2;
 
 }  // namespace
@@ -57,21 +54,6 @@ TetherService* TetherService::Get(Profile* profile) {
 // static
 void TetherService::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
-  // If we initially assume that BLE advertising is not supported, it will
-  // result in Tether's Settings and Quick Settings sections not being visible
-  // when the user logs in with Bluetooth disabled (because the TechnologyState
-  // will be UNAVAILABLE, instead of the desired UNINITIALIZED).
-  //
-  // Initially assuming that BLE advertising *is* supported works well for most
-  // devices, but if a user first logs into a device without BLE advertising
-  // support and with Bluetooth disabled, Tether will be visible in Settings and
-  // Quick Settings, but disappear upon enabling Bluetooth. This is an
-  // acceptable edge case, and likely rare because Bluetooth is enabled by
-  // default on new logins. Additionally, through this pref, we will record if
-  // BLE advertising is not supported and remember that for future logins.
-  registry->RegisterBooleanPref(prefs::kInstantTetheringBleAdvertisingSupported,
-                                true);
-
   chromeos::tether::TetherComponentImpl::RegisterProfilePrefs(registry);
 }
 
@@ -86,8 +68,6 @@ std::string TetherService::TetherFeatureStateToString(
   switch (state) {
     case (TetherFeatureState::SHUT_DOWN):
       return "[TetherService shut down]";
-    case (TetherFeatureState::BLE_ADVERTISING_NOT_SUPPORTED):
-      return "[BLE advertising not supported]";
     case (TetherFeatureState::NO_AVAILABLE_HOSTS):
       return "[no potential Tether hosts]";
     case (TetherFeatureState::CELLULAR_DISABLED):
@@ -308,18 +288,7 @@ void TetherService::OnTetherHostsUpdated() {
 
 void TetherService::AdapterPoweredChanged(device::BluetoothAdapter* adapter,
                                           bool powered) {
-  // Once the BLE advertising interval has been set (regardless of if BLE
-  // advertising is supported), simply update the TechnologyState.
-  if (has_attempted_to_set_ble_advertising_interval_) {
-    UpdateTetherTechnologyState();
-    return;
-  }
-
-  // If the BluetoothAdapter was not powered when first fetched (see
-  // OnBluetoothAdapterFetched()), now attempt to set the BLE advertising
-  // interval.
-  if (powered)
-    SetBleAdvertisingInterval();
+  UpdateTetherTechnologyState();
 }
 
 void TetherService::DeviceListChanged() {
@@ -459,7 +428,6 @@ TetherService::GetTetherTechnologyState() {
     case SHUT_DOWN:
     case SUSPENDED:
     case BLE_NOT_PRESENT:
-    case BLE_ADVERTISING_NOT_SUPPORTED:
     case WIFI_NOT_PRESENT:
     case NO_AVAILABLE_HOSTS:
     case CELLULAR_DISABLED:
@@ -520,49 +488,6 @@ void TetherService::OnBluetoothAdapterFetched(
   // Update TechnologyState in case Tether is otherwise available but Bluetooth
   // is off.
   UpdateTetherTechnologyState();
-
-  // If |adapter_| is not powered, wait until it is to call
-  // SetBleAdvertisingInterval(). See AdapterPoweredChanged().
-  if (IsBluetoothPowered())
-    SetBleAdvertisingInterval();
-}
-
-void TetherService::OnBluetoothAdapterAdvertisingIntervalSet() {
-  has_attempted_to_set_ble_advertising_interval_ = true;
-  SetIsBleAdvertisingSupportedPref(true);
-
-  UpdateTetherTechnologyState();
-}
-
-void TetherService::OnBluetoothAdapterAdvertisingIntervalError(
-    device::BluetoothAdvertisement::ErrorCode status) {
-  has_attempted_to_set_ble_advertising_interval_ = true;
-  SetIsBleAdvertisingSupportedPref(false);
-
-  UpdateTetherTechnologyState();
-}
-
-void TetherService::SetBleAdvertisingInterval() {
-  DCHECK(IsBluetoothPowered());
-  adapter_->SetAdvertisingInterval(
-      base::TimeDelta::FromMilliseconds(kMinAdvertisingIntervalMilliseconds),
-      base::TimeDelta::FromMilliseconds(kMaxAdvertisingIntervalMilliseconds),
-      base::BindOnce(&TetherService::OnBluetoothAdapterAdvertisingIntervalSet,
-                     weak_ptr_factory_.GetWeakPtr()),
-      base::BindOnce(&TetherService::OnBluetoothAdapterAdvertisingIntervalError,
-                     weak_ptr_factory_.GetWeakPtr()));
-}
-
-bool TetherService::GetIsBleAdvertisingSupportedPref() {
-  return profile_->GetPrefs()->GetBoolean(
-      prefs::kInstantTetheringBleAdvertisingSupported);
-}
-
-void TetherService::SetIsBleAdvertisingSupportedPref(
-    bool is_ble_advertising_supported) {
-  profile_->GetPrefs()->SetBoolean(
-      prefs::kInstantTetheringBleAdvertisingSupported,
-      is_ble_advertising_supported);
 }
 
 bool TetherService::IsBluetoothPresent() const {
@@ -608,9 +533,6 @@ TetherService::TetherFeatureState TetherService::GetTetherFeatureState() {
   if (!IsWifiPresent())
     return WIFI_NOT_PRESENT;
 
-  if (!GetIsBleAdvertisingSupportedPref())
-    return BLE_ADVERTISING_NOT_SUPPORTED;
-
   if (!HasSyncedTetherHosts())
     return NO_AVAILABLE_HOSTS;
 

chrome/browser/chromeos/tether/tether_service.h

@@ -147,15 +147,6 @@ class TetherService
                            TestBetterTogetherSuiteInitiallyDisabled);
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest,
                            TestBetterTogetherSuiteBecomesDisabled);
-  FRIEND_TEST_ALL_PREFIXES(TetherServiceTest, TestBleAdvertisingNotSupported);
-  FRIEND_TEST_ALL_PREFIXES(
-      TetherServiceTest,
-      TestBleAdvertisingNotSupported_BluetoothIsInitiallyNotPowered);
-  FRIEND_TEST_ALL_PREFIXES(
-      TetherServiceTest,
-      TestBleAdvertisingNotSupportedAndRecorded_BluetoothIsInitiallyNotPowered);
-  FRIEND_TEST_ALL_PREFIXES(TetherServiceTest,
-                           TestBleAdvertisingSupportedButIncorrectlyRecorded);
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest,
                            TestGet_PrimaryUser_FeatureFlagEnabled);
   FRIEND_TEST_ALL_PREFIXES(
@@ -185,7 +176,8 @@ class TetherService
     // Note: Value 0 was previously OTHER_OR_UNKNOWN, but this was a vague
     // description.
     SHUT_DOWN = 0,
-    BLE_ADVERTISING_NOT_SUPPORTED = 1,
+    // Note: Value 1 was previously BLE_ADVERTISING_NOT_SUPPORTED, but this
+    // value is obsolete and should no longer be used.
     // Note: Value 2 was previously SCREEN_LOCKED, but this value is obsolete
     // and should no longer be used.
     NO_AVAILABLE_HOSTS = 3,
@@ -208,18 +200,6 @@ class TetherService
   void GetBluetoothAdapter();
   void OnBluetoothAdapterFetched(
       scoped_refptr<device::BluetoothAdapter> adapter);
-  void OnBluetoothAdapterAdvertisingIntervalSet();
-  void OnBluetoothAdapterAdvertisingIntervalError(
-      device::BluetoothAdvertisement::ErrorCode status);
-
-  void SetBleAdvertisingInterval();
-
-  // Whether BLE advertising is supported on this device. This should only
-  // return true if a call to BluetoothAdapter::SetAdvertisingInterval() during
-  // TetherService construction succeeds. That method will fail in cases like
-  // those captured in crbug.com/738222.
-  bool GetIsBleAdvertisingSupportedPref();
-  void SetIsBleAdvertisingSupportedPref(bool is_ble_advertising_supported);
 
   bool IsBluetoothPresent() const;
   bool IsBluetoothPowered() const;
@@ -270,10 +250,6 @@ class TetherService
   chromeos::multidevice_setup::mojom::HostStatus host_status_ =
       chromeos::multidevice_setup::mojom::HostStatus::kNoEligibleHosts;
 
-  // Whether the BLE advertising interval has attempted to be set during this
-  // session.
-  bool has_attempted_to_set_ble_advertising_interval_ = false;
-
   // The first report of TetherFeatureState::BLE_NOT_PRESENT is usually
   // incorrect and hence is a false positive. This property tracks if the first
   // report has been hit yet.

chrome/browser/chromeos/tether/tether_service_unittest.cc

@@ -86,33 +86,6 @@ chromeos::multidevice::RemoteDeviceRefList CreateTestDevices() {
   return list;
 }
 
-class MockExtendedBluetoothAdapter : public device::MockBluetoothAdapter {
- public:
-  void SetAdvertisingInterval(
-      const base::TimeDelta& min,
-      const base::TimeDelta& max,
-      base::OnceClosure callback,
-      AdvertisementErrorCallback error_callback) override {
-    if (is_ble_advertising_supported_) {
-      std::move(callback).Run();
-    } else {
-      std::move(error_callback)
-          .Run(device::BluetoothAdvertisement::ErrorCode::
-                   ERROR_INVALID_ADVERTISEMENT_INTERVAL);
-    }
-  }
-
-  void set_is_ble_advertising_supported(bool is_ble_advertising_supported) {
-    is_ble_advertising_supported_ = is_ble_advertising_supported;
-  }
-
- protected:
-  ~MockExtendedBluetoothAdapter() override {}
-
- private:
-  bool is_ble_advertising_supported_ = true;
-};
-
 class TestTetherService : public TetherService {
  public:
   TestTetherService(
@@ -380,7 +353,7 @@ class TetherServiceTest : public testing::Test {
     fake_enrollment_manager_->set_user_private_key(kTestUserPrivateKey);
 
     mock_adapter_ =
-        base::MakeRefCounted<NiceMock<MockExtendedBluetoothAdapter>>();
+        base::MakeRefCounted<NiceMock<device::MockBluetoothAdapter>>();
     SetIsBluetoothPowered(true);
     is_adapter_present_ = true;
     ON_CALL(*mock_adapter_, IsPresent())
@@ -595,7 +568,7 @@ class TetherServiceTest : public testing::Test {
 
   chromeos::multidevice_setup::mojom::FeatureState initial_feature_state_;
 
-  scoped_refptr<MockExtendedBluetoothAdapter> mock_adapter_;
+  scoped_refptr<device::MockBluetoothAdapter> mock_adapter_;
   bool is_adapter_present_;
   bool is_adapter_powered_;
   bool shutdown_reason_verified_;
@@ -829,114 +802,6 @@ TEST_F(TetherServiceTest, TestBetterTogetherSuiteBecomesDisabled) {
                                BETTER_TOGETHER_SUITE_DISABLED);
 }
 
-TEST_F(TetherServiceTest, TestBleAdvertisingNotSupported) {
-  mock_adapter_->set_is_ble_advertising_supported(false);
-
-  CreateTetherService();
-
-  EXPECT_EQ(
-      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNAVAILABLE,
-      network_state_handler()->GetTechnologyState(
-          chromeos::NetworkTypePattern::Tether()));
-  VerifyTetherActiveStatus(false /* expected_active */);
-
-  VerifyTetherFeatureStateRecorded(
-      TetherService::TetherFeatureState::BLE_ADVERTISING_NOT_SUPPORTED,
-      1 /* expected_count */);
-  VerifyLastShutdownReason(
-      chromeos::tether::TetherComponent::ShutdownReason::OTHER);
-}
-
-TEST_F(TetherServiceTest,
-       TestBleAdvertisingNotSupported_BluetoothIsInitiallyNotPowered) {
-  SetIsBluetoothPowered(false);
-
-  mock_adapter_->set_is_ble_advertising_supported(false);
-
-  CreateTetherService();
-
-  // TetherService has not yet been able to find out that BLE advertising is not
-  // supported.
-  EXPECT_EQ(
-      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNINITIALIZED,
-      network_state_handler()->GetTechnologyState(
-          chromeos::NetworkTypePattern::Tether()));
-  VerifyTetherActiveStatus(false /* expected_active */);
-  EXPECT_TRUE(profile_->GetPrefs()->GetBoolean(
-      prefs::kInstantTetheringBleAdvertisingSupported));
-
-  SetIsBluetoothPowered(true);
-
-  EXPECT_EQ(
-      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNAVAILABLE,
-      network_state_handler()->GetTechnologyState(
-          chromeos::NetworkTypePattern::Tether()));
-  VerifyTetherActiveStatus(false /* expected_active */);
-  EXPECT_FALSE(profile_->GetPrefs()->GetBoolean(
-      prefs::kInstantTetheringBleAdvertisingSupported));
-
-  VerifyTetherFeatureStateRecorded(
-      TetherService::TetherFeatureState::BLE_ADVERTISING_NOT_SUPPORTED,
-      1 /* expected_count */);
-}
-
-TEST_F(
-    TetherServiceTest,
-    TestBleAdvertisingNotSupportedAndRecorded_BluetoothIsInitiallyNotPowered) {
-  SetIsBluetoothPowered(false);
-
-  mock_adapter_->set_is_ble_advertising_supported(false);
-
-  // Simulate a login after we determined that BLE advertising is not supported.
-  profile_->GetPrefs()->SetBoolean(
-      prefs::kInstantTetheringBleAdvertisingSupported, false);
-
-  CreateTetherService();
-
-  EXPECT_EQ(
-      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNAVAILABLE,
-      network_state_handler()->GetTechnologyState(
-          chromeos::NetworkTypePattern::Tether()));
-  VerifyTetherActiveStatus(false /* expected_active */);
-
-  SetIsBluetoothPowered(true);
-
-  EXPECT_EQ(
-      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNAVAILABLE,
-      network_state_handler()->GetTechnologyState(
-          chromeos::NetworkTypePattern::Tether()));
-  VerifyTetherActiveStatus(false /* expected_active */);
-
-  VerifyTetherFeatureStateRecorded(
-      TetherService::TetherFeatureState::BLE_ADVERTISING_NOT_SUPPORTED,
-      1 /* expected_count */);
-}
-
-TEST_F(TetherServiceTest, TestBleAdvertisingSupportedButIncorrectlyRecorded) {
-  // Simulate a login after we incorrectly determined that BLE advertising is
-  // not supported (this is not an expected case, but may have happened if
-  // BluetoothAdapter::SetAdvertisingInterval() failed for a weird, one-off
-  // reason).
-  profile_->GetPrefs()->SetBoolean(
-      prefs::kInstantTetheringBleAdvertisingSupported, false);
-
-  CreateTetherService();
-
-  EXPECT_EQ(chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_ENABLED,
-            network_state_handler()->GetTechnologyState(
-                chromeos::NetworkTypePattern::Tether()));
-  VerifyTetherActiveStatus(true /* expected_active */);
-  EXPECT_TRUE(profile_->GetPrefs()->GetBoolean(
-      prefs::kInstantTetheringBleAdvertisingSupported));
-
-  VerifyTetherFeatureStateRecorded(TetherService::TetherFeatureState::ENABLED,
-                                   1 /* expected_count */);
-
-  ShutdownTetherService();
-  VerifyLastShutdownReason(
-      chromeos::tether::TetherComponent::ShutdownReason::USER_LOGGED_OUT);
-}
-
 TEST_F(TetherServiceTest, TestGet_NotPrimaryUser_FeatureFlagDisabled) {
   EXPECT_FALSE(TetherService::Get(profile_.get()));
 }

chrome/common/pref_names.cc

@@ -818,10 +818,6 @@ const char kPinUnlockWeakPinsAllowed[] = "pin_unlock_weak_pins_allowed";
 // digits are necessary to unlock the device. Can be recommended.
 const char kPinUnlockAutosubmitEnabled[] = "pin_unlock_autosubmit_enabled";
 
-// Boolean pref indicating whether this device supports BLE advertising.
-const char kInstantTetheringBleAdvertisingSupported[] =
-    "tether.ble_advertising_supported";
-
 // Boolean pref indicating whether someone can cast to the device.
 const char kCastReceiverEnabled[] = "cast_receiver.enabled";
 

chrome/common/pref_names.h

@@ -289,7 +289,6 @@ extern const char kPinUnlockMinimumLength[];
 extern const char kPinUnlockMaximumLength[];
 extern const char kPinUnlockWeakPinsAllowed[];
 extern const char kPinUnlockAutosubmitEnabled[];
-extern const char kInstantTetheringBleAdvertisingSupported[];
 extern const char kCastReceiverEnabled[];
 extern const char kMinimumAllowedChromeVersion[];
 extern const char kShowArcSettingsOnSessionStart[];