Revert "Track WinRT device connection status"
This reverts commit 2f4e14b295bb503f51758a69a650dc1dda31b09a.
Reason for revert: Most likely culprit for the consistently failing Windows devices tests (also bluetooth-related).
Original change's description:
> 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}
TBR=odejesush@chromium.org,jameshollyer@chromium.org
Change-Id: I8add21a3922e2a2c56ad72cd8e986e4a7b67a228
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 127810
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1947648
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720898}
diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc
index 7a2eaca..1a98b75 100644
--- a/device/bluetooth/bluetooth_device_unittest.cc
+++ b/device/bluetooth/bluetooth_device_unittest.cc
@@ -2233,20 +2233,4 @@
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 f7d1b77a..f480e6c 100644
--- a/device/bluetooth/bluetooth_device_winrt.cc
+++ b/device/bluetooth/bluetooth_device_winrt.cc
@@ -245,7 +245,15 @@
if (!ble_device_)
return false;
- return connection_status_;
+ 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;
}
bool BluetoothDeviceWinrt::IsConnectable() const {
@@ -500,7 +508,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,
@@ -535,14 +543,6 @@
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 04d95a9..6fba93b6 100644
--- a/device/bluetooth/bluetooth_device_winrt.h
+++ b/device/bluetooth/bluetooth_device_winrt.h
@@ -127,8 +127,6 @@
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 ebe4f3c4..006e54b 100644
--- a/device/bluetooth/test/bluetooth_test.h
+++ b/device/bluetooth/test/bluetooth_test.h
@@ -332,9 +332,6 @@
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 3d735f9..7c4c1b6 100644
--- a/device/bluetooth/test/bluetooth_test_win.cc
+++ b/device/bluetooth/test/bluetooth_test_win.cc
@@ -841,16 +841,6 @@
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 6029617e..f9f7c2d 100644
--- a/device/bluetooth/test/bluetooth_test_win.h
+++ b/device/bluetooth/test/bluetooth_test_win.h
@@ -144,7 +144,6 @@
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 ab8eea3..7d1d3d4d 100644
--- a/device/bluetooth/test/fake_bluetooth_le_device_winrt.cc
+++ b/device/bluetooth/test/fake_bluetooth_le_device_winrt.cc
@@ -219,11 +219,6 @@
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_)
@@ -245,8 +240,10 @@
// Simulate production UWP behavior that only really disconnects once all
// references to a device are dropped.
- if (reference_count_ == 0u)
- SimulateStatusChangeToDisconnect();
+ if (reference_count_ == 0u) {
+ status_ = BluetoothConnectionStatus_Disconnected;
+ connection_status_changed_handler_->Invoke(this, nullptr);
+ }
}
void FakeBluetoothLEDeviceWinrt::SimulateDeviceBreaksConnection() {
@@ -259,7 +256,8 @@
}
// Simulate a Gatt Disconnecion regardless of the reference count.
- SimulateStatusChangeToDisconnect();
+ status_ = BluetoothConnectionStatus_Disconnected;
+ connection_status_changed_handler_->Invoke(this, nullptr);
}
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 888d73fa..91493edd 100644
--- a/device/bluetooth/test/fake_bluetooth_le_device_winrt.h
+++ b/device/bluetooth/test/fake_bluetooth_le_device_winrt.h
@@ -130,7 +130,6 @@
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,
