Add chrome.hid.getUserSelectedDevices API.

extensions/browser/api/hid/hid_api.cc

Index: extensions/browser/api/hid/hid_api.cc
diff --git a/extensions/browser/api/hid/hid_api.cc b/extensions/browser/api/hid/hid_api.cc
index d67f56499d88346f99b184179e09d220e31e4776..209601e92e29ee5042319a11b83d8c7e1744c519 100644
--- a/extensions/browser/api/hid/hid_api.cc
+++ b/extensions/browser/api/hid/hid_api.cc
@@ -14,6 +14,8 @@
 #include "device/hid/hid_device_info.h"
 #include "device/hid/hid_service.h"
 #include "extensions/browser/api/api_resource_manager.h"
+#include "extensions/browser/api/device_permissions_prompt.h"
+#include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/common/api/hid.h"
 #include "net/base/io_buffer.h"
 
@@ -101,6 +103,54 @@ void HidGetDevicesFunction::OnEnumerationComplete(
   Respond(OneArgument(devices.release()));
 }
 
+HidGetUserSelectedDevicesFunction::HidGetUserSelectedDevicesFunction() {
+}
+
+HidGetUserSelectedDevicesFunction::~HidGetUserSelectedDevicesFunction() {
+}
+
+ExtensionFunction::ResponseAction HidGetUserSelectedDevicesFunction::Run() {

[Devlin, 2015/05/05 22:19:50]

I thought I saw something in the IDL about this needing user gesture, but I
don't see that implemented anywhere?

[Reilly Grant (use Gerrit), 2015/05/06 00:52:52]

Good catch.

+  scoped_ptr<core_api::hid::GetUserSelectedDevices::Params> parameters =
+      hid::GetUserSelectedDevices::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(parameters.get());

[Devlin, 2015/05/05 22:19:50]

nit: scoped_ptrs are Testable, so you don't need the .get().  I don't really
care between using it and not, but you should probably be consistent :)  (It
looks like you don't use it on, e.g., line 123)

[Reilly Grant (use Gerrit), 2015/05/06 00:52:52]

Fixed.

+
+  bool multiple = false;
+  if (parameters->options.multiple) {

[Devlin, 2015/05/05 22:19:50]

inline all this?
bool multiple = parameters->options.multiple && *parameters->options.multiple;

[Reilly Grant (use Gerrit), 2015/05/06 00:52:52]

Done.

+    multiple = *parameters->options.multiple;
+  }
+
+  std::vector<HidDeviceFilter> filters;
+  if (parameters->options.filters) {
+    filters.resize(parameters->options.filters->size());
+    for (size_t i = 0; i < parameters->options.filters->size(); ++i) {
+      ConvertHidDeviceFilter(parameters->options.filters->at(i), &filters[i]);
+    }
+  }
+
+  prompt_ = ExtensionsAPIClient::Get()->CreateDevicePermissionsPrompt(
+      GetAssociatedWebContents());

[Devlin, 2015/05/05 22:19:50]

Prefer GetSenderWebContents() in most cases.

[Reilly Grant (use Gerrit), 2015/05/06 00:52:53]

Done.

+  if (!prompt_) {
+    return RespondNow(Error(kErrorServiceUnavailable));
+  }
+
+  prompt_->AskForHidDevices(
+      extension(), browser_context(), multiple, filters,
+      base::Bind(&HidGetUserSelectedDevicesFunction::OnDevicesChosen, this));
+  return RespondLater();
+}
+
+void HidGetUserSelectedDevicesFunction::OnDevicesChosen(
+    const std::vector<scoped_refptr<HidDeviceInfo>>& devices) {
+  HidDeviceManager* device_manager = HidDeviceManager::Get(browser_context());
+  if (!device_manager) {

[Devlin, 2015/05/05 22:19:50]

I'm seeing a lot of these null checks in this patch.  Under what circumstances
can device_manager be unavailable?  Should we gate the availability on the API
on the availability of the manager?  Should we at least make this check before
we try and show a picker to the user?

[Reilly Grant (use Gerrit), 2015/05/06 00:52:53]

I've moved the check into Run. HidDeviceManager should always be available but
I've been bitten by incognito bugs where something goes wrong and it isn't and
I'd rather a clean failure than a crash.

[Devlin, 2015/05/06 23:04:28]

I can sympathize, but if something should always be available, we should treat
it as though it is (feel free to put in a DCHECK/CHECK so we can get really nice
traces), for the same reasons that we discourage behavior like
DCHECK(!bad)
if (bad)
  return;

So if the manager is meant to be null, e.g., in incognito, then the check should
be
if (!manager)
  return Error()  // Manager can be null while incognito

But otherwise, we should assume it exists, so we can flush out any times it
doesn't erroneously.

[Reilly Grant (use Gerrit), 2015/05/06 23:48:52]

I've removed the kErrorServiceUnavailable message and turned all instances into
CHECKs.

+    Respond(Error(kErrorServiceUnavailable));
+    return;
+  }
+
+  Respond(
+      OneArgument(device_manager->GetApiDevicesFromList(devices).release()));

[Devlin, 2015/05/05 22:19:50]

There's a OneArgument variation that takes a scoped_ptr - use that instead of
release()ing here.

[Reilly Grant (use Gerrit), 2015/05/06 00:52:53]

Done.

+}
+
 HidConnectFunction::HidConnectFunction() : connection_manager_(nullptr) {
 }
 
@@ -128,7 +178,7 @@ ExtensionFunction::ResponseAction HidConnectFunction::Run() {
     return RespondNow(Error(kErrorInvalidDeviceId));
   }
 
-  if (!HidDeviceManager::HasPermission(extension(), device_info)) {
+  if (!device_manager->HasPermission(extension(), device_info, true)) {
     return RespondNow(Error(kErrorPermissionDenied));
   }