Commit ad8a1d2d authored by Friedrich [CET]'s avatar Friedrich [CET] Committed by Commit Bot

Revert "Track WinRT device connection status"

This reverts commit 2f4e14b2.

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/+/1947648Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720898}
parent e2327b85
......@@ -2233,20 +2233,4 @@ TEST_F(BluetoothTest, DISABLED_GattConnectedNameChange) {
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
......@@ -245,7 +245,15 @@ bool BluetoothDeviceWinrt::IsGattConnected() const {
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 @@ void BluetoothDeviceWinrt::OnFromBluetoothAddress(
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::OnFromBluetoothAddress(
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();
......
......@@ -127,8 +127,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDeviceWinrt : public BluetoothDevice {
void ClearGattServices();
void ClearEventRegistrations();
ABI::Windows::Devices::Bluetooth::BluetoothConnectionStatus
connection_status_;
uint64_t raw_address_;
std::string address_;
base::Optional<std::string> local_name_;
......
......@@ -332,9 +332,6 @@ class BluetoothTestBase : public testing::Test {
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.
......
......@@ -841,16 +841,6 @@ void BluetoothTestWinrt::SimulateGattNameChange(BluetoothDevice* device,
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) {
......
......@@ -144,7 +144,6 @@ class BluetoothTestWinrt : public BluetoothTestWin,
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;
......
......@@ -219,11 +219,6 @@ void FakeBluetoothLEDeviceWinrt::SimulateGattConnection() {
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 @@ void FakeBluetoothLEDeviceWinrt::SimulateGattDisconnection() {
// 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 @@ void FakeBluetoothLEDeviceWinrt::SimulateDeviceBreaksConnection() {
}
// Simulate a Gatt Disconnecion regardless of the reference count.
SimulateStatusChangeToDisconnect();
status_ = BluetoothConnectionStatus_Disconnected;
connection_status_changed_handler_->Invoke(this, nullptr);
}
void FakeBluetoothLEDeviceWinrt::SimulateGattNameChange(
......
......@@ -130,7 +130,6 @@ class FakeBluetoothLEDeviceWinrt
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,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment