Reland of Update device/usb and its Mojo interface for variable size ISO packets. (patchset #1 id:1 of https://codereview.chromium.org/1658953003/ )

Reason for revert:
Fixed the uninitialized test buffer.

Original issue's description:
> Revert of Update device/usb and its Mojo interface for variable size ISO packets. (patchset #3 id:120001 of https://codereview.chromium.org/1618393004/ )
>
> Reason for revert:
> Uninit error from UsbApiTest.TransferFailure
>
> BUG=583343
>
> Original issue's description:
> > Update device/usb and its Mojo interface for variable size ISO packets.
> >
> > To support the WebUSB API our underlying USB library needs to support
> > isochronous transfers with full control over the packet size. We also
> > need to know the completion status of each packet which was previously
> > not available.
> >
> > This patch updates the interface to match that provided by libusb and
> > the underlying platform specific APIs.
> >
> > BUG=492204
> >
> > Committed: https://crrev.com/051c98e9d3e843295d659b5676fcfa9dc1be5da6
> > Cr-Commit-Position: refs/heads/master@{#372844}
>
> TBR=scheib@chromium.org,dgozman@chromium.org,reillyg@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=492204
>
> Committed: https://crrev.com/c40c36ef3d3a722e0df920a4bbae25695f0b18e8
> Cr-Commit-Position: refs/heads/master@{#372971}

TBR=scheib@chromium.org,dgozman@chromium.org,zhaoqin@chromium.org
BUG=583343

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

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

3 files changed, 123 insertions, 32 deletions

diff --git a/extensions/browser/api/usb/usb_api.cc b/extensions/browser/api/usb/usb_api.cc
index 16710ddab..3035239 100644
--- a/extensions/browser/api/usb/usb_api.cc
+++ b/extensions/browser/api/usb/usb_api.cc
@@ -4,6 +4,8 @@
 
 #include "extensions/browser/api/usb/usb_api.h"
 
+#include <algorithm>
+#include <numeric>
 #include <string>
 #include <utility>
 #include <vector>
@@ -468,7 +470,8 @@ void UsbTransferFunction::OnCompleted(UsbTransferStatus status,
   } else {
     scoped_ptr<base::ListValue> error_args(new base::ListValue());
     error_args->Append(std::move(transfer_info));
-    // Returning arguments with an error is wrong but we're stuck with it.
+    // Using ErrorWithArguments is discouraged but required to provide the
+    // detailed transfer info as the transfer may have partially succeeded.
     Respond(ErrorWithArguments(std::move(error_args),
                                ConvertTransferStatusToApi(status)));
   }
@@ -1134,48 +1137,89 @@ ExtensionFunction::ResponseAction UsbIsochronousTransferFunction::Run() {
   size_t size = 0;
   UsbEndpointDirection direction = device::USB_DIRECTION_INBOUND;
 
-  if (!ConvertDirectionFromApi(generic_transfer.direction, &direction)) {
+  if (!ConvertDirectionFromApi(generic_transfer.direction, &direction))
     return RespondNow(Error(kErrorConvertDirection));
-  }
 
-  if (!GetTransferSize(generic_transfer, &size)) {
+  if (!GetTransferSize(generic_transfer, &size))
     return RespondNow(Error(kErrorInvalidTransferLength));
-  }
 
-  if (transfer.packets < 0 || transfer.packets >= kMaxPackets) {
+  if (transfer.packets < 0 || transfer.packets >= kMaxPackets)
     return RespondNow(Error(kErrorInvalidNumberOfPackets));
-  }
+  size_t packets = transfer.packets;
 
-  unsigned int packets = transfer.packets;
   if (transfer.packet_length < 0 ||
       transfer.packet_length >= kMaxPacketLength) {
     return RespondNow(Error(kErrorInvalidPacketLength));
   }
 
-  unsigned int packet_length = transfer.packet_length;
-  const uint64_t total_length = packets * packet_length;
-  if (packets > size || total_length > size) {
+  size_t total_length = packets * transfer.packet_length;
+  if (packets > size || total_length > size)
     return RespondNow(Error(kErrorTransferLength));
-  }
-
-  scoped_refptr<net::IOBuffer> buffer =
-      CreateBufferForTransfer(generic_transfer, direction, size);
-  if (!buffer.get()) {
-    return RespondNow(Error(kErrorMalformedParameters));
-  }
+  std::vector<uint32_t> packet_lengths(packets, transfer.packet_length);
 
   int timeout = generic_transfer.timeout ? *generic_transfer.timeout : 0;
-  if (timeout < 0) {
+  if (timeout < 0)
     return RespondNow(Error(kErrorInvalidTimeout));
-  }
 
-  device_handle->IsochronousTransfer(
-      direction, generic_transfer.endpoint, buffer.get(), size, packets,
-      packet_length, timeout,
-      base::Bind(&UsbIsochronousTransferFunction::OnCompleted, this));
+  if (direction == device::USB_DIRECTION_INBOUND) {
+    device_handle->IsochronousTransferIn(
+        generic_transfer.endpoint, packet_lengths, timeout,
+        base::Bind(&UsbIsochronousTransferFunction::OnCompleted, this));
+  } else {
+    scoped_refptr<net::IOBuffer> buffer = CreateBufferForTransfer(
+        generic_transfer, direction, transfer.packets * transfer.packet_length);
+    if (!buffer.get())
+      return RespondNow(Error(kErrorMalformedParameters));
+
+    device_handle->IsochronousTransferOut(
+        generic_transfer.endpoint, buffer.get(), packet_lengths, timeout,
+        base::Bind(&UsbIsochronousTransferFunction::OnCompleted, this));
+  }
   return RespondLater();
 }
 
+void UsbIsochronousTransferFunction::OnCompleted(
+    scoped_refptr<net::IOBuffer> data,
+    const std::vector<UsbDeviceHandle::IsochronousPacket>& packets) {
+  size_t length = std::accumulate(
+      packets.begin(), packets.end(), 0,
+      [](const size_t& a, const UsbDeviceHandle::IsochronousPacket& packet) {
+        return a + packet.transferred_length;
+      });
+  scoped_ptr<char[]> buffer(new char[length]);
+
+  UsbTransferStatus status = device::USB_TRANSFER_COMPLETED;
+  size_t buffer_offset = 0;
+  size_t data_offset = 0;
+  for (const auto& packet : packets) {
+    // Capture the error status of the first unsuccessful packet.
+    if (status == device::USB_TRANSFER_COMPLETED &&
+        packet.status != device::USB_TRANSFER_COMPLETED) {
+      status = packet.status;
+    }
+
+    memcpy(&buffer[buffer_offset], data->data() + data_offset,
+           packet.transferred_length);
+    buffer_offset += packet.transferred_length;
+    data_offset += packet.length;
+  }
+
+  scoped_ptr<base::DictionaryValue> transfer_info(new base::DictionaryValue());
+  transfer_info->SetInteger(kResultCodeKey, status);
+  transfer_info->Set(kDataKey,
+                     new base::BinaryValue(std::move(buffer), length));
+  if (status == device::USB_TRANSFER_COMPLETED) {
+    Respond(OneArgument(std::move(transfer_info)));
+  } else {
+    scoped_ptr<base::ListValue> error_args(new base::ListValue());
+    error_args->Append(std::move(transfer_info));
+    // Using ErrorWithArguments is discouraged but required to provide the
+    // detailed transfer info as the transfer may have partially succeeded.
+    Respond(ErrorWithArguments(std::move(error_args),
+                               ConvertTransferStatusToApi(status)));
+  }
+}
+
 UsbResetDeviceFunction::UsbResetDeviceFunction() {
 }
 
@@ -1210,7 +1254,8 @@ void UsbResetDeviceFunction::OnComplete(bool success) {
 
     scoped_ptr<base::ListValue> error_args(new base::ListValue());
     error_args->AppendBoolean(false);
-    // Returning arguments with an error is wrong but we're stuck with it.
+    // Using ErrorWithArguments is discouraged but required to maintain
+    // compatibility with existing applications.
     Respond(ErrorWithArguments(std::move(error_args), kErrorResetDevice));
   }
 }
diff --git a/extensions/browser/api/usb/usb_api.h b/extensions/browser/api/usb/usb_api.h
index 5bea383..3d1dec1 100644
--- a/extensions/browser/api/usb/usb_api.h
+++ b/extensions/browser/api/usb/usb_api.h
@@ -245,7 +245,7 @@ class UsbClaimInterfaceFunction : public UsbConnectionFunction {
 
   UsbClaimInterfaceFunction();
 
- protected:
+ private:
   ~UsbClaimInterfaceFunction() override;
 
   // ExtensionFunction:
@@ -334,7 +334,7 @@ class UsbInterruptTransferFunction : public UsbTransferFunction {
   DISALLOW_COPY_AND_ASSIGN(UsbInterruptTransferFunction);
 };
 
-class UsbIsochronousTransferFunction : public UsbTransferFunction {
+class UsbIsochronousTransferFunction : public UsbConnectionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("usb.isochronousTransfer", USB_ISOCHRONOUSTRANSFER)
 
@@ -346,6 +346,10 @@ class UsbIsochronousTransferFunction : public UsbTransferFunction {
   // ExtensionFunction:
   ResponseAction Run() override;
 
+  void OnCompleted(
+      scoped_refptr<net::IOBuffer> data,
+      const std::vector<device::UsbDeviceHandle::IsochronousPacket>& packets);
+
   DISALLOW_COPY_AND_ASSIGN(UsbIsochronousTransferFunction);
 };
 
diff --git a/extensions/browser/api/usb/usb_apitest.cc b/extensions/browser/api/usb/usb_apitest.cc
index cc6b47b..30601a9 100644
--- a/extensions/browser/api/usb/usb_apitest.cc
+++ b/extensions/browser/api/usb/usb_apitest.cc
@@ -4,6 +4,8 @@
 
 #include <stddef.h>
 
+#include <numeric>
+
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_utils.h"
@@ -48,6 +50,44 @@ ACTION_TEMPLATE(InvokeUsbTransferCallback,
   ::std::tr1::get<k>(args).Run(p1, io_buffer, 1);
 }
 
+ACTION_P2(InvokeUsbIsochronousTransferOutCallback,
+          transferred_length,
+          success_packets) {
+  std::vector<UsbDeviceHandle::IsochronousPacket> packets(arg2.size());
+  for (size_t i = 0; i < packets.size(); ++i) {
+    packets[i].length = arg2[i];
+    if (i < success_packets) {
+      packets[i].transferred_length = transferred_length;
+      packets[i].status = device::USB_TRANSFER_COMPLETED;
+    } else {
+      packets[i].transferred_length = 0;
+      packets[i].status = device::USB_TRANSFER_ERROR;
+    }
+  }
+  arg4.Run(arg1, packets);
+}
+
+ACTION_P2(InvokeUsbIsochronousTransferInCallback,
+          transferred_length,
+          success_packets) {
+  size_t total_length = std::accumulate(arg1.begin(), arg1.end(), 0u);
+  net::IOBuffer* io_buffer = new net::IOBuffer(total_length);
+  memset(io_buffer->data(), 0, total_length);  // Avoid uninitialized reads.
+  std::vector<UsbDeviceHandle::IsochronousPacket> packets(arg1.size());
+  for (size_t i = 0; i < packets.size(); ++i) {
+    packets[i].length = arg1[i];
+    packets[i].transferred_length = transferred_length;
+    if (i < success_packets) {
+      packets[i].transferred_length = transferred_length;
+      packets[i].status = device::USB_TRANSFER_COMPLETED;
+    } else {
+      packets[i].transferred_length = 0;
+      packets[i].status = device::USB_TRANSFER_ERROR;
+    }
+  }
+  arg3.Run(io_buffer, packets);
+}
+
 class TestDevicePermissionsPrompt
     : public DevicePermissionsPrompt,
       public DevicePermissionsPrompt::Prompt::Observer {
@@ -165,10 +205,8 @@ IN_PROC_BROWSER_TEST_F(UsbApiTest, TransferEvent) {
   EXPECT_CALL(*mock_device_handle_.get(),
               GenericTransfer(device::USB_DIRECTION_OUTBOUND, 2, _, 1, _, _))
       .WillOnce(InvokeUsbTransferCallback<5>(device::USB_TRANSFER_COMPLETED));
-  EXPECT_CALL(
-      *mock_device_handle_.get(),
-      IsochronousTransfer(device::USB_DIRECTION_OUTBOUND, 3, _, 1, 1, 1, _, _))
-      .WillOnce(InvokeUsbTransferCallback<7>(device::USB_TRANSFER_COMPLETED));
+  EXPECT_CALL(*mock_device_handle_.get(), IsochronousTransferOut(3, _, _, _, _))
+      .WillOnce(InvokeUsbIsochronousTransferOutCallback(1, 1u));
   EXPECT_CALL(*mock_device_handle_.get(), Close()).Times(AnyNumber());
   ASSERT_TRUE(RunAppTest("api_test/usb/transfer_event"));
 }
@@ -181,10 +219,14 @@ IN_PROC_BROWSER_TEST_F(UsbApiTest, ZeroLengthTransfer) {
 }
 
 IN_PROC_BROWSER_TEST_F(UsbApiTest, TransferFailure) {
-  EXPECT_CALL(*mock_device_handle_.get(), GenericTransfer(_, _, _, _, _, _))
+  EXPECT_CALL(*mock_device_handle_.get(),
+              GenericTransfer(device::USB_DIRECTION_OUTBOUND, 1, _, _, _, _))
       .WillOnce(InvokeUsbTransferCallback<5>(device::USB_TRANSFER_COMPLETED))
       .WillOnce(InvokeUsbTransferCallback<5>(device::USB_TRANSFER_ERROR))
       .WillOnce(InvokeUsbTransferCallback<5>(device::USB_TRANSFER_TIMEOUT));
+  EXPECT_CALL(*mock_device_handle_.get(), IsochronousTransferIn(2, _, _, _))
+      .WillOnce(InvokeUsbIsochronousTransferInCallback(8, 10u))
+      .WillOnce(InvokeUsbIsochronousTransferInCallback(8, 5u));
   EXPECT_CALL(*mock_device_handle_.get(), Close()).Times(AnyNumber());
   ASSERT_TRUE(RunAppTest("api_test/usb/transfer_failure"));
 }