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}

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