Add a UsbService::Observer function for cleanup actions.

This patch adds an additional function to the UsbService::Observer
interface that should be used to perform cleanup operations when a USB
device is disconnected. This is separate from the OnDeviceRemoved
function so that implementations of that function, which may depend on
other subsystems' knowledge of a connected device, can run before any
cleanup actions have taken place.

A browsertest for the chrome.usb.getUserSelectedDevices is included
which demonstrates the need for this 2-phase event handling.

BUG=452298

Review URL: https://codereview.chromium.org/891853002

Cr-Commit-Position: refs/heads/master@{#314973}

device/usb/usb_service.cc

@@ -44,6 +44,10 @@ void UsbService::Observer::OnDeviceAdded(scoped_refptr<UsbDevice> device) {
 void UsbService::Observer::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {
 }
 
+void UsbService::Observer::OnDeviceRemovedCleanup(
+    scoped_refptr<UsbDevice> device) {
+}
+
 // static
 UsbService* UsbService::GetInstance(
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
@@ -85,6 +89,7 @@ void UsbService::NotifyDeviceAdded(scoped_refptr<UsbDevice> device) {
 void UsbService::NotifyDeviceRemoved(scoped_refptr<UsbDevice> device) {
   DCHECK(CalledOnValidThread());
   FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(device));
+  FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemovedCleanup(device));
 }
 
 }  // namespace device

device/usb/usb_service.h

@@ -35,6 +35,9 @@ class UsbService : public base::NonThreadSafe {
     // was created.
     virtual void OnDeviceAdded(scoped_refptr<UsbDevice> device);
     virtual void OnDeviceRemoved(scoped_refptr<UsbDevice> device);
+    // For observers that need to process device removal after others have run.
+    // Should not depend on any other service's knowledge of connected devices.
+    virtual void OnDeviceRemovedCleanup(scoped_refptr<UsbDevice> device);
   };
 
   // The UI task runner reference is used to talk to the PermissionBrokerClient

extensions/browser/api/device_permissions_manager.cc

@@ -398,7 +398,7 @@ class DevicePermissionsManager::FileThreadHelper : public UsbService::Observer {
   }
 
  private:
-  void OnDeviceRemoved(scoped_refptr<UsbDevice> device) override {
+  void OnDeviceRemovedCleanup(scoped_refptr<UsbDevice> device) override {
     DCHECK_CURRENTLY_ON(BrowserThread::FILE);
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,

extensions/browser/api/usb/usb_apitest.cc

@@ -6,7 +6,9 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_utils.h"
 #include "device/usb/usb_service.h"
+#include "extensions/browser/api/device_permissions_prompt.h"
 #include "extensions/browser/api/usb/usb_api.h"
+#include "extensions/shell/browser/shell_extensions_api_client.h"
 #include "extensions/shell/test/shell_apitest.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "net/base/io_buffer.h"
@@ -42,6 +44,38 @@ void RequestUsbAccess(int interface_id,
   base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, true));
 }
 
+class TestDevicePermissionsPrompt
+    : public DevicePermissionsPrompt,
+      public DevicePermissionsPrompt::Prompt::Observer {
+ public:
+  TestDevicePermissionsPrompt(content::WebContents* web_contents)
+      : DevicePermissionsPrompt(web_contents) {}
+
+  void ShowDialog() override { prompt()->SetObserver(this); }
+
+  void OnDevicesChanged() override {
+    std::vector<scoped_refptr<UsbDevice>> devices;
+    for (size_t i = 0; i < prompt()->GetDeviceCount(); ++i) {
+      prompt()->GrantDevicePermission(i);
+      devices.push_back(prompt()->GetDevice(i));
+      if (!prompt()->multiple()) {
+        break;
+      }
+    }
+    delegate()->OnUsbDevicesChosen(devices);
+  }
+};
+
+class TestExtensionsAPIClient : public ShellExtensionsAPIClient {
+ public:
+  TestExtensionsAPIClient() : ShellExtensionsAPIClient() {}
+
+  scoped_ptr<DevicePermissionsPrompt> CreateDevicePermissionsPrompt(
+      content::WebContents* web_contents) const override {
+    return make_scoped_ptr(new TestDevicePermissionsPrompt(web_contents));
+  }
+};
+
 class MockUsbDeviceHandle : public UsbDeviceHandle {
  public:
   MockUsbDeviceHandle() : UsbDeviceHandle() {}
@@ -157,6 +191,10 @@ class UsbApiTest : public ShellApiTest {
     ShellApiTest::SetUpOnMainThread();
 
     mock_device_ = new MockUsbDevice(0, 0, 0);
+    EXPECT_CALL(*mock_device_.get(), GetManufacturer(_))
+        .WillRepeatedly(Return(false));
+    EXPECT_CALL(*mock_device_.get(), GetProduct(_))
+        .WillRepeatedly(Return(false));
     EXPECT_CALL(*mock_device_.get(), GetSerialNumber(_))
         .WillRepeatedly(Return(false));
 
@@ -298,7 +336,6 @@ IN_PROC_BROWSER_TEST_F(UsbApiTest, OnDeviceAdded) {
   run_loop.Run();
 
   ASSERT_TRUE(result_listener.WaitUntilSatisfied());
-  ASSERT_EQ("success", result_listener.message());
 }
 
 IN_PROC_BROWSER_TEST_F(UsbApiTest, OnDeviceRemoved) {
@@ -318,7 +355,28 @@ IN_PROC_BROWSER_TEST_F(UsbApiTest, OnDeviceRemoved) {
   run_loop.Run();
 
   ASSERT_TRUE(result_listener.WaitUntilSatisfied());
-  ASSERT_EQ("success", result_listener.message());
+}
+
+IN_PROC_BROWSER_TEST_F(UsbApiTest, GetUserSelectedDevices) {
+  ExtensionTestMessageListener ready_listener("opened_device", false);
+  ExtensionTestMessageListener result_listener("success", false);
+  result_listener.set_failure_message("failure");
+
+  EXPECT_CALL(*mock_device_handle_.get(), Close()).Times(1);
+
+  TestExtensionsAPIClient test_api_client;
+  ASSERT_TRUE(LoadApp("api_test/usb/get_user_selected_devices"));
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+
+  base::RunLoop run_loop;
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&MockUsbService::NotifyDeviceRemoved,
+                 base::Unretained(mock_service_), mock_device_),
+      run_loop.QuitClosure());
+  run_loop.Run();
+
+  ASSERT_TRUE(result_listener.WaitUntilSatisfied());
 }
 
 }  // namespace extensions

extensions/test/data/api_test/usb/get_user_selected_devices/background.js

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var device_from_user = undefined;
+
+chrome.test.runWithUserGesture(function() {
+  chrome.usb.getDevices({}, function(devices) {
+    chrome.test.assertEq(0, devices.length);
+    chrome.usb.getUserSelectedDevices({ multiple: false }, function(devices) {
+      chrome.test.assertEq(1, devices.length);
+      device_from_user = devices[0];
+      chrome.usb.openDevice(device_from_user, function(connection) {
+        chrome.usb.closeDevice(connection);
+        chrome.test.sendMessage("opened_device");
+      });
+    });
+  });
+});
+
+chrome.usb.onDeviceRemoved.addListener(function(device) {
+  if (device.device == device_from_user.device) {
+    chrome.test.sendMessage("success");
+  } else {
+    chrome.test.sendMessage("failure");
+  }
+});

extensions/test/data/api_test/usb/get_user_selected_devices/manifest.json

+{
+  "name": "chrome.usb.getUserSelectedDevices",
+  "version": "0.1",
+  "description": "browser test for chrome.usb.getUserSelectedDevices",
+  "app": {
+    "background": {
+      "scripts": ["background.js"]
+    }
+  },
+  "permissions": [
+    "usb"
+  ]
+}