bluetooth: Check the correct value was set when powering on/off

Part of a bigger refactor to change how tests work. Currently tests
perform the following steps:

1. Set the next response to a BlueZ method call
2. Call a method that will call the BlueZ method
3. Check that the BlueZ method was called the right number of times
4. Apply the values that were passed to the BlueZ method

With this approach, we have no control over when BlueZ replies and
we don't know what values were passed to BlueZ until after we apply
them.

This CL address the second problem by exposing a GetSetValueForTesting
method in dbus::Property and adding a GetLastSetPoweredValue() method
to tests that use GetSetValueForTesting to test that the BlueZ method
was called with the right value. Now that we don't need to apply the
`set_value` to know what it was, we can remove
SimulateSetPoweredCompleted.

A follow up CL will address the first problem. In the end tests should
perform the following steps:

1. Call a method that will call a BlueZ method
2. Check that the BlueZ method was called the right number of times
3. Check that the BlueZ method was called with the right value
4. Simulate the BlueZ method replying

An added benefit of this approach is that we can simulate other
operations, like the adapter getting removed, between steps 1.
and 4., something that happens often in real devices.

Bug: 870192
Change-Id: I0c2cfc90a84d6d79b158804264c0e93664310a08
Reviewed-on: https://chromium-review.googlesource.com/c/1298818Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Ovidio Henriquez <odejesush@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603697}

dbus/property.h

@@ -441,6 +441,10 @@ class CHROME_DBUS_EXPORT Property : public PropertyBase {
   // |set_value_| of a property.
   void ReplaceSetValueForTesting(const T& value) { set_value_ = value; }
 
+  // Method used by test and stub implementations to retrieve the |set_value|
+  // of a property.
+  const T& GetSetValueForTesting() const { return set_value_; }
+
  private:
   // Current cached value of the property.
   T value_;

services/device/bluetooth/bluetooth_system_unittest.cc

@@ -47,13 +47,18 @@ class DEVICE_BLUETOOTH_EXPORT TestBluetoothAdapterClient
               callback) {}
     ~Properties() override = default;
 
-    void ResetCallCount() { set_powered_call_count_ = 0; }
+    void ResetCallCount() {
+      set_powered_call_count_ = 0;
+      last_set_powered_value_.reset();
+    }
 
     void SetNextSetPoweredResponse(bool next_response) {
       DCHECK(!next_set_powered_response_);
       next_set_powered_response_ = next_response;
     }
 
+    bool GetLastSetPoweredValue() { return last_set_powered_value_.value(); }
+
     // dbus::PropertySet override
     void Get(dbus::PropertyBase* property,
              dbus::PropertySet::GetCallback callback) override {
@@ -71,6 +76,7 @@ class DEVICE_BLUETOOTH_EXPORT TestBluetoothAdapterClient
       DVLOG(1) << "Set " << property->name();
       if (property->name() == powered.name()) {
         ++set_powered_call_count_;
+        last_set_powered_value_ = powered.GetSetValueForTesting();
         base::Optional<bool> response;
         response.swap(next_set_powered_response_);
         callback.Run(response.value());
@@ -81,6 +87,7 @@ class DEVICE_BLUETOOTH_EXPORT TestBluetoothAdapterClient
 
     size_t set_powered_call_count_ = 0;
     base::Optional<bool> next_set_powered_response_;
+    base::Optional<bool> last_set_powered_value_;
   };
 
   TestBluetoothAdapterClient() = default;
@@ -151,11 +158,9 @@ class DEVICE_BLUETOOTH_EXPORT TestBluetoothAdapterClient
     return it->second.set_powered_call_count_;
   }
 
-  // Simulates the adapter changing states in response to a previous call to
-  // change the "powered" property.
-  void SimulateSetPoweredCompleted(const std::string& object_path_str) {
-    GetProperties(dbus::ObjectPath(object_path_str))
-        ->powered.ReplaceValueWithSetValue();
+  bool GetLastSetPoweredValue(const std::string& object_path_str) {
+    return GetProperties(dbus::ObjectPath(object_path_str))
+        ->GetLastSetPoweredValue();
   }
 
   // Simulates adapter at |object_path_str| changing its discovering state to
@@ -629,20 +634,24 @@ TEST_F(BluetoothSystemTest, SetPoweredOff_SucceedsAdapterInitiallyOn) {
   auto system = CreateBluetoothSystem();
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOn, GetStateAndWait(system));
 
+  // Try to power off the adapter.
   test_bluetooth_adapter_client_->SetNextSetPoweredResponse(kFooObjectPathStr,
                                                             true);
   EXPECT_EQ(mojom::BluetoothSystem::SetPoweredResult::kSuccess,
             SetPoweredAndWait(system, false));
   EXPECT_EQ(1u, test_bluetooth_adapter_client_->GetSetPoweredCallCount(
                     kFooObjectPathStr));
+  EXPECT_FALSE(test_bluetooth_adapter_client_->GetLastSetPoweredValue(
+      kFooObjectPathStr));
   EXPECT_EQ(mojom::BluetoothSystem::State::kTransitioning,
             GetStateAndWait(system));
   EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning}),
             on_state_changed_states_);
   ResetResults();
 
-  test_bluetooth_adapter_client_->SimulateSetPoweredCompleted(
-      kFooObjectPathStr);
+  // Simulate the adapter actually powering off.
+  test_bluetooth_adapter_client_->SimulateAdapterPowerStateChanged(
+      kFooObjectPathStr, false);
 
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOff,
             GetStateAndWait(system));
@@ -657,20 +666,24 @@ TEST_F(BluetoothSystemTest, SetPoweredOn_SucceedsAdapterInitiallyOff) {
 
   auto system = CreateBluetoothSystem();
 
+  // Try to power on the adapter.
   test_bluetooth_adapter_client_->SetNextSetPoweredResponse(kFooObjectPathStr,
                                                             true);
   EXPECT_EQ(mojom::BluetoothSystem::SetPoweredResult::kSuccess,
             SetPoweredAndWait(system, true));
   EXPECT_EQ(1u, test_bluetooth_adapter_client_->GetSetPoweredCallCount(
                     kFooObjectPathStr));
+  EXPECT_TRUE(test_bluetooth_adapter_client_->GetLastSetPoweredValue(
+      kFooObjectPathStr));
   EXPECT_EQ(mojom::BluetoothSystem::State::kTransitioning,
             GetStateAndWait(system));
   EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning}),
             on_state_changed_states_);
   ResetResults();
 
-  test_bluetooth_adapter_client_->SimulateSetPoweredCompleted(
-      kFooObjectPathStr);
+  // Simulate the adapter actually powering on.
+  test_bluetooth_adapter_client_->SimulateAdapterPowerStateChanged(
+      kFooObjectPathStr, true);
 
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOn, GetStateAndWait(system));
   EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kPoweredOn}),
@@ -690,6 +703,8 @@ TEST_F(BluetoothSystemTest, SetPoweredOff_FailsAdapterInitiallyOn) {
             SetPoweredAndWait(system, false));
   EXPECT_EQ(1u, test_bluetooth_adapter_client_->GetSetPoweredCallCount(
                     kFooObjectPathStr));
+  EXPECT_FALSE(test_bluetooth_adapter_client_->GetLastSetPoweredValue(
+      kFooObjectPathStr));
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOn, GetStateAndWait(system));
   EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning,
                          mojom::BluetoothSystem::State::kPoweredOn}),
@@ -709,6 +724,8 @@ TEST_F(BluetoothSystemTest, SetPoweredOn_FailsAdapterInitiallyOff) {
             SetPoweredAndWait(system, true));
   EXPECT_EQ(1u, test_bluetooth_adapter_client_->GetSetPoweredCallCount(
                     kFooObjectPathStr));
+  EXPECT_TRUE(test_bluetooth_adapter_client_->GetLastSetPoweredValue(
+      kFooObjectPathStr));
   EXPECT_EQ(mojom::BluetoothSystem::State::kPoweredOff,
             GetStateAndWait(system));
   EXPECT_EQ(StateVector({mojom::BluetoothSystem::State::kTransitioning,