Track WinRT device connection status

Windows sometimes sends OnConnectionStatusChange events when the status
has not changed. Previously we had assumed the status changed and acted
accordingly.  This change keeps track of the connection status and if
OnConnctionStatusChange fires with the same status it is now ignored.

Bug: 127810
Change-Id: I64c5b3d99e0ac827f05ca1618d570ff83296a053
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1939827
Reviewed-by: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Commit-Queue: James Hollyer <jameshollyer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720876}
diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc
index 1a98b75..7a2eaca 100644
--- a/device/bluetooth/bluetooth_device_unittest.cc
+++ b/device/bluetooth/bluetooth_device_unittest.cc
@@ -2233,4 +2233,20 @@
   EXPECT_EQ(base::UTF8ToUTF16(kTestDeviceName), device->GetNameForDisplay());
 }
 
+#if defined(OS_WIN)
+// WinRT sometimes calls OnConnectionStatusChanged when the status is
+// initialized and not when changed.
+TEST_P(BluetoothTestWinrtOnly, FalseStatusChangedTest) {
+  InitWithFakeAdapter();
+  StartLowEnergyDiscoverySession();
+  BluetoothDevice* device = SimulateLowEnergyDevice(3);
+  EXPECT_FALSE(device->IsConnected());
+  device->CreateGattConnection(GetGattConnectionCallback(Call::NOT_EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+  SimulateStatusChangeToDisconnect(device);
+
+  base::RunLoop().RunUntilIdle();
+}
+#endif
+
 }  // namespace device
diff --git a/device/bluetooth/bluetooth_device_winrt.cc b/device/bluetooth/bluetooth_device_winrt.cc
index f480e6c..f7d1b77 100644
--- a/device/bluetooth/bluetooth_device_winrt.cc
+++ b/device/bluetooth/bluetooth_device_winrt.cc
@@ -245,15 +245,7 @@
   if (!ble_device_)
     return false;
 
-  BluetoothConnectionStatus status;
-  HRESULT hr = ble_device_->get_ConnectionStatus(&status);
-  if (FAILED(hr)) {
-    BLUETOOTH_LOG(DEBUG) << "Getting ConnectionStatus failed: "
-                         << logging::SystemErrorCodeToString(hr);
-    return false;
-  }
-
-  return status == BluetoothConnectionStatus_Connected;
+  return connection_status_;
 }
 
 bool BluetoothDeviceWinrt::IsConnectable() const {
@@ -508,7 +500,7 @@
   ClearEventRegistrations();
 
   ble_device_ = std::move(ble_device);
-
+  ble_device_->get_ConnectionStatus(&connection_status_);
   connection_changed_token_ = AddTypedEventHandler(
       ble_device_.Get(), &IBluetoothLEDevice::add_ConnectionStatusChanged,
       base::BindRepeating(&BluetoothDeviceWinrt::OnConnectionStatusChanged,
@@ -543,6 +535,14 @@
 void BluetoothDeviceWinrt::OnConnectionStatusChanged(
     IBluetoothLEDevice* ble_device,
     IInspectable* object) {
+  BluetoothConnectionStatus new_status;
+  ble_device->get_ConnectionStatus(&new_status);
+  // Windows sometimes returns a status changed event with a status that has
+  // not changed.
+  if (new_status == connection_status_)
+    return;
+
+  connection_status_ = new_status;
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (IsGattConnected()) {
     DidConnectGatt();
diff --git a/device/bluetooth/bluetooth_device_winrt.h b/device/bluetooth/bluetooth_device_winrt.h
index 6fba93b6..04d95a9 100644
--- a/device/bluetooth/bluetooth_device_winrt.h
+++ b/device/bluetooth/bluetooth_device_winrt.h
@@ -127,6 +127,8 @@
   void ClearGattServices();
   void ClearEventRegistrations();
 
+  ABI::Windows::Devices::Bluetooth::BluetoothConnectionStatus
+      connection_status_;
   uint64_t raw_address_;
   std::string address_;
   base::Optional<std::string> local_name_;
diff --git a/device/bluetooth/test/bluetooth_test.h b/device/bluetooth/test/bluetooth_test.h
index 006e54b..ebe4f3c4 100644
--- a/device/bluetooth/test/bluetooth_test.h
+++ b/device/bluetooth/test/bluetooth_test.h
@@ -332,6 +332,9 @@
   virtual void SimulateGattNameChange(BluetoothDevice* device,
                                       const std::string& new_name) {}
 
+  // Simulates a connection status change to disconnect.
+  virtual void SimulateStatusChangeToDisconnect(BluetoothDevice* device) {}
+
   // Simulates success of discovering services. |uuids| is used to create a
   // service for each UUID string. Multiple UUIDs with the same value produce
   // multiple service instances.
diff --git a/device/bluetooth/test/bluetooth_test_win.cc b/device/bluetooth/test/bluetooth_test_win.cc
index 7c4c1b6..3d735f9 100644
--- a/device/bluetooth/test/bluetooth_test_win.cc
+++ b/device/bluetooth/test/bluetooth_test_win.cc
@@ -841,6 +841,16 @@
   ble_device->SimulateGattNameChange(new_name);
 }
 
+void BluetoothTestWinrt::SimulateStatusChangeToDisconnect(
+    BluetoothDevice* device) {
+  // Spin the message loop to make sure a device instance was obtained.
+  base::RunLoop().RunUntilIdle();
+  auto* const ble_device =
+      static_cast<TestBluetoothDeviceWinrt*>(device)->ble_device();
+  DCHECK(ble_device);
+  ble_device->SimulateStatusChangeToDisconnect();
+}
+
 void BluetoothTestWinrt::SimulateGattConnectionError(
     BluetoothDevice* device,
     BluetoothDevice::ConnectErrorCode error_code) {
diff --git a/device/bluetooth/test/bluetooth_test_win.h b/device/bluetooth/test/bluetooth_test_win.h
index f9f7c2d..6029617e 100644
--- a/device/bluetooth/test/bluetooth_test_win.h
+++ b/device/bluetooth/test/bluetooth_test_win.h
@@ -144,6 +144,7 @@
   void SimulateDeviceBreaksConnection(BluetoothDevice* device) override;
   void SimulateGattNameChange(BluetoothDevice* device,
                               const std::string& new_name) override;
+  void SimulateStatusChangeToDisconnect(BluetoothDevice* device) override;
   void SimulateGattServicesDiscovered(
       BluetoothDevice* device,
       const std::vector<std::string>& uuids) override;
diff --git a/device/bluetooth/test/fake_bluetooth_le_device_winrt.cc b/device/bluetooth/test/fake_bluetooth_le_device_winrt.cc
index 7d1d3d4..ab8eea3 100644
--- a/device/bluetooth/test/fake_bluetooth_le_device_winrt.cc
+++ b/device/bluetooth/test/fake_bluetooth_le_device_winrt.cc
@@ -219,6 +219,11 @@
   connection_status_changed_handler_->Invoke(this, nullptr);
 }
 
+void FakeBluetoothLEDeviceWinrt::SimulateStatusChangeToDisconnect() {
+  status_ = BluetoothConnectionStatus_Disconnected;
+  connection_status_changed_handler_->Invoke(this, nullptr);
+}
+
 void FakeBluetoothLEDeviceWinrt ::SimulateGattConnectionError(
     BluetoothDevice::ConnectErrorCode error_code) {
   if (!gatt_services_callback_)
@@ -240,10 +245,8 @@
 
   // Simulate production UWP behavior that only really disconnects once all
   // references to a device are dropped.
-  if (reference_count_ == 0u) {
-    status_ = BluetoothConnectionStatus_Disconnected;
-    connection_status_changed_handler_->Invoke(this, nullptr);
-  }
+  if (reference_count_ == 0u)
+    SimulateStatusChangeToDisconnect();
 }
 
 void FakeBluetoothLEDeviceWinrt::SimulateDeviceBreaksConnection() {
@@ -256,8 +259,7 @@
   }
 
   // Simulate a Gatt Disconnecion regardless of the reference count.
-  status_ = BluetoothConnectionStatus_Disconnected;
-  connection_status_changed_handler_->Invoke(this, nullptr);
+  SimulateStatusChangeToDisconnect();
 }
 
 void FakeBluetoothLEDeviceWinrt::SimulateGattNameChange(
diff --git a/device/bluetooth/test/fake_bluetooth_le_device_winrt.h b/device/bluetooth/test/fake_bluetooth_le_device_winrt.h
index 91493ed..888d73f 100644
--- a/device/bluetooth/test/fake_bluetooth_le_device_winrt.h
+++ b/device/bluetooth/test/fake_bluetooth_le_device_winrt.h
@@ -130,6 +130,7 @@
   void SimulateGattNameChange(const std::string& new_name);
   void SimulateGattServicesDiscovered(const std::vector<std::string>& uuids);
   void SimulateGattServicesChanged();
+  void SimulateStatusChangeToDisconnect();
   void SimulateGattServiceRemoved(BluetoothRemoteGattService* service);
   void SimulateGattCharacteristic(BluetoothRemoteGattService* service,
                                   const std::string& uuid,