bluetooth: Change state to kTransitioning when SetPowered gets called

Bug: 896113
Change-Id: I60f8e082a01dd1a7007004ae26ff275c87eaa8bf
Reviewed-on: https://chromium-review.googlesource.com/c/1290576Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Ovidio Henriquez <odejesush@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601462}

services/device/bluetooth/bluetooth_system.cc

@@ -103,6 +103,13 @@ void BluetoothSystem::SetPowered(bool powered, SetPoweredCallback callback) {
     return;
   }
 
+  // Update the BluetoothSystem state to kTransitioning if a previous call to
+  // SetPowered() has not done so already.
+  if (state_ != State::kTransitioning) {
+    state_ = State::kTransitioning;
+    client_ptr_->OnStateChanged(state_);
+  }
+
   GetBluetoothAdapterClient()
       ->GetProperties(active_adapter_.value())
       ->powered.Set(powered,
@@ -134,6 +141,13 @@ void BluetoothSystem::UpdateStateAndNotifyIfNecessary() {
 
 void BluetoothSystem::OnSetPoweredFinished(SetPoweredCallback callback,
                                            bool succeeded) {
+  if (!succeeded) {
+    // We change |state_| to `kTransitioning` before trying to set 'powered'. If
+    // the call to set 'powered' fails, then we need to change it back to
+    // `kPoweredOn` or `kPoweredOff` depending on the `active_adapter_` state.
+    UpdateStateAndNotifyIfNecessary();
+  }
+
   std::move(callback).Run(succeeded ? SetPoweredResult::kSuccess
                                     : SetPoweredResult::kFailedUnknownReason);
 }

services/device/bluetooth/bluetooth_system_unittest.cc

@@ -606,16 +606,18 @@ TEST_F(BluetoothSystemTest, SetPoweredOff_SucceedsAdapterInitiallyOn) {
             SetPoweredAndWait(system, false));
   EXPECT_EQ(1u, test_bluetooth_adapter_client_->GetSetPoweredCallCount(
                     kFooObjectPathStr));
-  // TODO(ortuno): Change to kTransitioning once implemented.
-  EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOn, GetStateAndWait(system));
-  EXPECT_TRUE(on_state_changed_states_.empty());
+  EXPECT_EQ(mojom::BluetoothSystem::State::kTransitioning,
+            GetStateAndWait(system));
+  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning}),
+            on_state_changed_states_);
 
   test_bluetooth_adapter_client_->SimulateSetPoweredCompleted(
       kFooObjectPathStr);
 
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOff,
             GetStateAndWait(system));
-  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kPoweredOff}),
+  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning,
+                         mojom::BluetoothSystem::State::kPoweredOff}),
             on_state_changed_states_);
 }
 
@@ -632,16 +634,17 @@ TEST_F(BluetoothSystemTest, SetPoweredOn_SucceedsAdapterInitiallyOff) {
             SetPoweredAndWait(system, true));
   EXPECT_EQ(1u, test_bluetooth_adapter_client_->GetSetPoweredCallCount(
                     kFooObjectPathStr));
-  // TODO(ortuno): Change to kTransitioning once implemented.
-  EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOff,
+  EXPECT_EQ(mojom::BluetoothSystem::State::kTransitioning,
             GetStateAndWait(system));
-  EXPECT_TRUE(on_state_changed_states_.empty());
+  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning}),
+            on_state_changed_states_);
 
   test_bluetooth_adapter_client_->SimulateSetPoweredCompleted(
       kFooObjectPathStr);
 
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOn, GetStateAndWait(system));
-  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kPoweredOn}),
+  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning,
+                         mojom::BluetoothSystem::State::kPoweredOn}),
             on_state_changed_states_);
 }
 
@@ -661,9 +664,9 @@ TEST_F(BluetoothSystemTest, SetPoweredOff_FailsAdapterInitiallyOn) {
   EXPECT_EQ(1u, test_bluetooth_adapter_client_->GetSetPoweredCallCount(
                     kFooObjectPathStr));
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOn, GetStateAndWait(system));
-  // TODO(ortuno): Test that the state change to kTransitioning and then back
-  // to kPoweredOff after the call failed.
-  EXPECT_TRUE(on_state_changed_states_.empty());
+  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning,
+                         mojom::BluetoothSystem::State::kPoweredOn}),
+            on_state_changed_states_);
 }
 
 // Tests failing to set powered to "On" when the adapter is "Off".
@@ -681,9 +684,9 @@ TEST_F(BluetoothSystemTest, SetPoweredOn_FailsAdapterInitiallyOff) {
                     kFooObjectPathStr));
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOff,
             GetStateAndWait(system));
-  // TODO(ortuno): Test that the state change to kTransitioning and then back
-  // to kPoweredOff after the call failed.
-  EXPECT_TRUE(on_state_changed_states_.empty());
+  EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning,
+                         mojom::BluetoothSystem::State::kPoweredOff}),
+            on_state_changed_states_);
 }
 
 }  // namespace device

services/device/public/mojom/bluetooth_system.mojom

@@ -48,8 +48,9 @@ interface BluetoothSystem {
 
   // Attempts to change the state of the Bluetooth to `kPoweredOn` if |powered|
   // and `kPoweredOff` otherwise . Callback is run with `kSuccess if the command
-  // was successfully sent to the BT Radio. Once the BT radio actually changes
-  // state BluetoothSystemClient::OnStateChanged will be called.
+  // was successfully sent to the BT Radio. The state immediately changes to
+  // kTransitioning and once the BT radio actually changes state
+  // BluetoothSystemClient::OnStateChanged will be called.
   // TODO(https://crbug.com/896113): This function is missing two features:
   //  1. The new state should be saved in the user's pref so that the next time
   //     the machine turns off the state matches the user pref.