Skip to content

Commit

Permalink
[CrOS Bluetooth] Close all mojo connections when pairing dialog closes.
Browse files Browse the repository at this point in the history
Update <bluetooth-pairing-ui> to close all Mojo connections when it
is removed from the DOM. Previously, it would not do so, so if it is
removed from the DOM and Bluetooth is disabled, then re-enabled, it will
automatically start discovery because it is still observing
CrosBluetoothConfig properties. In some cases, this behavior can cause
Bluetooth to automatically re-enable after being disabled.

Recording: https://drive.google.com/file/d/1sIuCT-NLAurxRd22asYq_qlobOCO_kOy/view?usp=sharing

(cherry picked from commit cbf51ee)

Fixed: b/231738454
Change-Id: I51183eacdb9b0294d3de79d92075fa5f23345835
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3669998
Reviewed-by: Chad Duffin <chadduffin@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Chad Duffin <chadduffin@chromium.org>
Auto-Submit: Gordon Seto <gordonseto@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1008266}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3689704
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#531}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Gordon Seto authored and Chromium LUCI CQ committed Jun 3, 2022
1 parent 21a99b4 commit a60d4be
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1055,4 +1055,25 @@ suite('CrComponentsBluetoothPairingUiTest', function() {
// Error text should no longer be showing.
assertEquals(getDeviceSelectionPage().failedPairingDeviceId, '');
});

// Regression test for b/231738454.
test(
'Mojo connections are closed after dialog is removed from DOM',
async function() {
await init();
assertEquals(1, bluetoothConfig.getNumStartDiscoveryCalls());

// Remove the pairing dialog from the DOM.
bluetoothPairingUi.remove();

// Disable Bluetooth.
bluetoothConfig.setSystemState(BluetoothSystemState.kDisabled);
await flushTasks();

// Re-enable Bluetooth. If the Mojo connections are still alive, this
// will trigger discovery to start again.
bluetoothConfig.setSystemState(BluetoothSystemState.kEnabled);
await flushTasks();
assertEquals(1, bluetoothConfig.getNumStartDiscoveryCalls());
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ export class FakeBluetoothConfig {
* @private {?FakeDevicePairingHandler}
*/
this.lastPairingHandler_ = null;

/** @private {number} */
this.numStartDiscoveryCalls_ = 0;
}

/**
Expand Down Expand Up @@ -144,6 +147,7 @@ export class FakeBluetoothConfig {
this.lastDiscoveryDelegate_ = delegate;
this.notifyDiscoveryStarted_();
this.notifyDelegatesPropertiesUpdated_();
this.numStartDiscoveryCalls_++;
}

/**
Expand Down Expand Up @@ -497,4 +501,11 @@ export class FakeBluetoothConfig {
d => d.deviceProperties.id === deviceId);
return device ? device : null;
}

/**
* @return {number}
*/
getNumStartDiscoveryCalls() {
return this.numStartDiscoveryCalls_;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,6 @@ export class SettingsBluetoothPairingUiElement extends PolymerElement {

ready() {
super.ready();
getBluetoothConfig().observeSystemProperties(
this.systemPropertiesObserverReceiver_.$.bindNewPipeAndPassRemote());

// If there's a specific device to pair with, immediately go to the spinner
// page.
Expand All @@ -252,12 +250,28 @@ export class SettingsBluetoothPairingUiElement extends PolymerElement {
}
}

connectedCallback() {
super.connectedCallback();

getBluetoothConfig().observeSystemProperties(
this.systemPropertiesObserverReceiver_.$.bindNewPipeAndPassRemote());
}

disconnectedCallback() {
super.disconnectedCallback();

if (this.systemPropertiesObserverReceiver_) {
this.systemPropertiesObserverReceiver_.$.close();
}
if (this.bluetoothDiscoveryDelegateReceiver_) {
this.bluetoothDiscoveryDelegateReceiver_.$.close();
}
if (this.pairingDelegateReceiver_) {
this.pairingDelegateReceiver_.$.close();
}
if (this.keyEnteredReceiver_) {
this.keyEnteredReceiver_.close();
}
}

/** @override */
Expand Down

0 comments on commit a60d4be

Please sign in to comment.