hid: Don't interpret the report ID from incoming feature reports.

The USB HID GET_REPORT(Feature) request includes the report ID that the
host is requesting in the SETUP packet. When the report ID is not zero
the device will send this back to the host as the first byte of the
report. Unlike input and output reports we want to preserve this part of
the report because the device may in fact by sending a value other than
the report ID as the first byte of the response.

To achieve this, on Windows this patch stops stripping the report ID
from the buffer received from the OS. On OS X this was already the
behavior. On Linux this was the behavior if the report ID was non-zero,
otherwise it would artificially prepend a zero to the buffer which must
be removed.

BUG=420112

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

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

4 files changed, 22 insertions, 8 deletions

diff --git a/device/hid/hid_connection.h b/device/hid/hid_connection.h
index 9d2503c2..0a3e599 100644
--- a/device/hid/hid_connection.h
+++ b/device/hid/hid_connection.h
@@ -41,7 +41,9 @@ class HidConnection : public base::RefCountedThreadSafe<HidConnection> {
              size_t size,
              const WriteCallback& callback);
 
-  // The report ID is not returned in the buffer.
+  // The buffer will contain whatever report data was received from the device.
+  // This may include the report ID. The report ID is not stripped because a
+  // device may respond with other data in place of the report ID.
   void GetFeatureReport(uint8_t report_id, const ReadCallback& callback);
 
   // The report ID (or 0 if report IDs are not supported by the device) is
diff --git a/device/hid/hid_connection_linux.cc b/device/hid/hid_connection_linux.cc
index ac2e554..936f016 100644
--- a/device/hid/hid_connection_linux.cc
+++ b/device/hid/hid_connection_linux.cc
@@ -110,7 +110,7 @@ void HidConnectionLinux::PlatformGetFeatureReport(
   // and is overwritten by the feature report.
   DCHECK_GT(device_info().max_feature_report_size, 0);
   scoped_refptr<net::IOBufferWithSize> buffer(
-      new net::IOBufferWithSize(device_info().max_feature_report_size));
+      new net::IOBufferWithSize(device_info().max_feature_report_size + 1));
   buffer->data()[0] = report_id;
 
   int result = ioctl(device_file_.GetPlatformFile(),
@@ -119,6 +119,14 @@ void HidConnectionLinux::PlatformGetFeatureReport(
   if (result < 0) {
     VPLOG(1) << "Failed to get feature report";
     callback.Run(false, NULL, 0);
+  } else if (result == 0) {
+    VLOG(1) << "Get feature result too short.";
+    callback.Run(false, NULL, 0);
+  } else if (report_id == 0) {
+    // Linux adds a 0 to the beginning of the data received from the device.
+    scoped_refptr<net::IOBuffer> copied_buffer(new net::IOBuffer(result - 1));
+    memcpy(copied_buffer->data(), buffer->data() + 1, result - 1);
+    callback.Run(true, copied_buffer, result - 1);
   } else {
     callback.Run(true, buffer, result);
   }
diff --git a/device/hid/hid_connection_win.cc b/device/hid/hid_connection_win.cc
index a0bad16..5182d9c 100644
--- a/device/hid/hid_connection_win.cc
+++ b/device/hid/hid_connection_win.cc
@@ -235,10 +235,7 @@ void HidConnectionWin::OnReadFeatureComplete(
   DWORD bytes_transferred;
   if (GetOverlappedResult(
           file_.Get(), transfer->GetOverlapped(), &bytes_transferred, FALSE)) {
-    scoped_refptr<net::IOBuffer> new_buffer(
-        new net::IOBuffer(bytes_transferred - 1));
-    memcpy(new_buffer->data(), buffer->data() + 1, bytes_transferred - 1);
-    CompleteRead(new_buffer, bytes_transferred, callback);
+    callback.Run(true, buffer, bytes_transferred);
   } else {
     VPLOG(1) << "HID read failed";
     callback.Run(false, NULL, 0);
diff --git a/extensions/common/api/hid.idl b/extensions/common/api/hid.idl
index 36f2a60..3b81b2b 100644
--- a/extensions/common/api/hid.idl
+++ b/extensions/common/api/hid.idl
@@ -67,10 +67,11 @@ namespace hid {
   callback DisconnectCallback = void ();
 
   // |reportId|: The report ID or <code>0</code> if none.
-  // |data|: The content of the report.
+  // |data|: The report data, the report ID prefix (if present) is removed.
   callback ReceiveCallback = void (long reportId, ArrayBuffer data);
 
-  // |data|: The content of the report.
+  // |data|: The report data, including a report ID prefix if one is sent by the
+  // device.
   callback ReceiveFeatureReportCallback = void (ArrayBuffer data);
 
   callback SendCallback = void();
@@ -98,6 +99,9 @@ namespace hid {
                         ReceiveCallback callback);
 
     // Send an output report to the device.
+    //
+    // <em>Note:</em> Do not include a report ID prefix in <code>data</code>.
+    // It will be added if necessary.
     // |connectionId|: The <code>connectionId</code> returned by $(ref:connect).
     // |reportId|: The report ID to use, or <code>0</code> if none.
     // |data|: The report data.
@@ -114,6 +118,9 @@ namespace hid {
                                      ReceiveFeatureReportCallback callback);
 
     // Send a feature report to the device.
+    //
+    // <em>Note:</em> Do not include a report ID prefix in <code>data</code>.
+    // It will be added if necessary.
     // |connectionId|: The <code>connectionId</code> returned by $(ref:connect).
     // |reportId|: The report ID to use, or <code>0</code> if none.
     // |data|: The report data.