0

[CodeHealth] Fix leaked, dangling pointer in device::FakeUsbDeviceInfo

FakeUsbDeviceInfo contains a leaked, dangling pointer to
MockUsbMojoDevice. Fix this by:

1. Calling RemoveAllDevices() in FakeUsbDeviceManager's destructor. This
   will clear the scoped_refptrs that FakeUsbDeviceInfo's observers
   have, allowing FakeUsbDeviceInfo to be destroyed.
2. Fixing the member declaration order in tests.

Fixed: 1462085
Change-Id: Iab0ad729b7f8d7fd9179f3c11dcb980acf446712
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4828847
Commit-Queue: Andy Phan <andyphan@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1193834}
This commit is contained in:
Andy Phan
2023-09-07 23:17:48 +00:00
committed by Chromium LUCI CQ
parent 0a8be10402
commit 7ca1ebf697
4 changed files with 12 additions and 6 deletions
chrome/browser/extensions/api/usb
extensions/browser/api/usb
services/device/public/cpp/test

@ -83,9 +83,11 @@ class ChromeUsbApiTest : public ExtensionBrowserTest {
kPolicySetting, extension->url().spec().c_str())));
}
device::FakeUsbDeviceManager fake_usb_manager_;
scoped_refptr<device::FakeUsbDeviceInfo> fake_device_;
// `mock_device_`, `fake_device_`, and `fake_usb_manager_` must be declared in
// this order to avoid dangling pointers.
device::MockUsbMojoDevice mock_device_;
scoped_refptr<device::FakeUsbDeviceInfo> fake_device_;
device::FakeUsbDeviceManager fake_usb_manager_;
};
IN_PROC_BROWSER_TEST_F(ChromeUsbApiTest, GetDevicesByPolicy) {

@ -149,9 +149,11 @@ class UsbApiTest : public ShellApiTest {
->UpdateActiveConfig(fake_device_->guid(), config_value);
}
device::FakeUsbDeviceManager fake_usb_manager_;
scoped_refptr<device::FakeUsbDeviceInfo> fake_device_;
// `mock_device_`, `fake_device_`, and `fake_usb_manager_` must be declared in
// this order to avoid dangling pointers.
device::MockUsbMojoDevice mock_device_;
scoped_refptr<device::FakeUsbDeviceInfo> fake_device_;
device::FakeUsbDeviceManager fake_usb_manager_;
};
} // namespace

@ -96,7 +96,7 @@ class FakeUsbDeviceInfo : public base::RefCounted<FakeUsbDeviceInfo> {
void SetDefault();
mojom::UsbDeviceInfo device_info_;
base::ObserverList<Observer> observer_list_;
raw_ptr<MockUsbMojoDevice, LeakedDanglingUntriaged> mock_device_ = nullptr;
raw_ptr<MockUsbMojoDevice> mock_device_ = nullptr;
};
} // namespace device

@ -21,7 +21,9 @@ namespace device {
FakeUsbDeviceManager::FakeUsbDeviceManager() {}
FakeUsbDeviceManager::~FakeUsbDeviceManager() {}
FakeUsbDeviceManager::~FakeUsbDeviceManager() {
RemoveAllDevices();
}
void FakeUsbDeviceManager::EnumerateDevicesAndSetClient(
mojo::PendingAssociatedRemote<mojom::UsbDeviceManagerClient> client,