diff options
author | reillyg <reillyg@chromium.org> | 2016-03-23 14:02:55 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-23 21:03:54 +0000 |
commit | fa968566aec6a7e007792c112b16060748b63fa9 (patch) | |
tree | c5ed07d70dde0ff97f5d7de6b555114400054014 /device/usb/mojo | |
parent | b70595110a1fd24f7225e1d82fd7b92eb5cb2b6c (diff) | |
download | chromium_src-fa968566aec6a7e007792c112b16060748b63fa9.zip chromium_src-fa968566aec6a7e007792c112b16060748b63fa9.tar.gz chromium_src-fa968566aec6a7e007792c112b16060748b63fa9.tar.bz2 |
Fix IPC loop in DeviceManager::GetDeviceChanges.
DeviceManagerImpl::MaybeRunDeviceChangesCallback is supposed to wait
until there are changes to the set of connected devices before calling
the stored callback. A bug introduced in r379393 removed this check
which meant that calls to GetDeviceChanges would return immediately. As
most callers issue another call to this method when it returns this
would lead to an infinite loop.
This change adds a test case exercising this case.
BUG=None
Review URL: https://codereview.chromium.org/1826883002
Cr-Commit-Position: refs/heads/master@{#382924}
Diffstat (limited to 'device/usb/mojo')
-rw-r--r-- | device/usb/mojo/device_manager_impl.cc | 9 | ||||
-rw-r--r-- | device/usb/mojo/device_manager_impl_unittest.cc | 15 |
2 files changed, 19 insertions, 5 deletions
diff --git a/device/usb/mojo/device_manager_impl.cc b/device/usb/mojo/device_manager_impl.cc index 69e238a..d9b8ac3 100644 --- a/device/usb/mojo/device_manager_impl.cc +++ b/device/usb/mojo/device_manager_impl.cc @@ -130,18 +130,17 @@ void DeviceManagerImpl::WillDestroyUsbService() { } void DeviceManagerImpl::MaybeRunDeviceChangesCallback() { - if (!device_change_callbacks_.empty()) { + if (!device_change_callbacks_.empty() && + !(devices_added_.empty() && devices_removed_.empty())) { DeviceChangeNotificationPtr notification = DeviceChangeNotification::New(); notification->devices_added.SetToEmpty(); notification->devices_removed.SetToEmpty(); - for (auto& map_entry : devices_added_) { + for (auto& map_entry : devices_added_) notification->devices_added.push_back(std::move(map_entry.second)); - } devices_added_.clear(); notification->devices_removed.Swap(&devices_removed_); - const GetDeviceChangesCallback& callback = device_change_callbacks_.front(); - callback.Run(std::move(notification)); + device_change_callbacks_.front().Run(std::move(notification)); device_change_callbacks_.pop(); } } diff --git a/device/usb/mojo/device_manager_impl_unittest.cc b/device/usb/mojo/device_manager_impl_unittest.cc index 9fc01df..3a0d50c 100644 --- a/device/usb/mojo/device_manager_impl_unittest.cc +++ b/device/usb/mojo/device_manager_impl_unittest.cc @@ -210,6 +210,21 @@ TEST_F(USBDeviceManagerImplTest, GetDeviceChanges) { loop.QuitClosure())); loop.Run(); } + + { + std::set<std::string> added_guids; + std::set<std::string> removed_guids; + added_guids.insert(device0->guid()); + base::RunLoop loop; + device_manager->GetDeviceChanges(base::Bind(&ExpectDeviceChangesAndThen, + added_guids, removed_guids, + loop.QuitClosure())); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, + base::Bind(&MockUsbService::AddDevice, + base::Unretained(device_client_.usb_service()), device0)); + loop.Run(); + } } } // namespace usb |