Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(234)

Issue 1618393004: Update device/usb and its Mojo interface for variable size ISO packets. (Closed)

Created:
4 years, 11 months ago by Reilly Grant (use Gerrit)
Modified:
4 years, 10 months ago
Reviewers:
scheib, dgozman
CC:
chromium-reviews, extensions-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, devtools-reviews_chromium.org, chromium-apps-reviews_chromium.org, darin (slow to review), ben+mojo_chromium.org, pfeldman
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

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}

Patch Set 1 : #

Total comments: 9

Patch Set 2 : Add test for isochronous transfer failures. #

Patch Set 3 : Document use of ErrorWithArguments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+625 lines, -280 lines) Patch
M chrome/browser/devtools/device/usb/android_usb_browsertest.cc View 1 1 chunk +12 lines, -8 lines 0 comments Download
M device/devices_app/usb/device_impl.h View 3 chunks +6 lines, -6 lines 0 comments Download
M device/devices_app/usb/device_impl.cc View 3 chunks +51 lines, -45 lines 0 comments Download
M device/devices_app/usb/device_impl_unittest.cc View 1 8 chunks +135 lines, -60 lines 0 comments Download
M device/devices_app/usb/public/interfaces/device.mojom View 3 chunks +24 lines, -13 lines 0 comments Download
M device/devices_app/usb/type_converters.h View 1 2 chunks +10 lines, -2 lines 0 comments Download
M device/devices_app/usb/type_converters.cc View 1 1 chunk +13 lines, -0 lines 0 comments Download
M device/usb/mock_usb_device_handle.h View 1 3 chunks +13 lines, -10 lines 0 comments Download
M device/usb/usb_device_handle.h View 1 3 chunks +23 lines, -9 lines 0 comments Download
M device/usb/usb_device_handle_impl.h View 2 chunks +22 lines, -13 lines 0 comments Download
M device/usb/usb_device_handle_impl.cc View 1 14 chunks +158 lines, -82 lines 0 comments Download
M extensions/browser/api/usb/usb_api.h View 3 chunks +6 lines, -2 lines 0 comments Download
M extensions/browser/api/usb/usb_api.cc View 1 2 4 chunks +70 lines, -25 lines 0 comments Download
M extensions/browser/api/usb/usb_apitest.cc View 1 4 chunks +46 lines, -5 lines 0 comments Download
M extensions/test/data/api_test/usb/transfer_event/test.js View 1 4 chunks +4 lines, -0 lines 0 comments Download
M extensions/test/data/api_test/usb/transfer_failure/test.js View 1 1 chunk +32 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (9 generated)
Reilly Grant (use Gerrit)
Please take a look.
4 years, 10 months ago (2016-01-26 20:10:44 UTC) #6
scheib
https://codereview.chromium.org/1618393004/diff/80001/extensions/browser/api/usb/usb_api.cc File extensions/browser/api/usb/usb_api.cc (right): https://codereview.chromium.org/1618393004/diff/80001/extensions/browser/api/usb/usb_api.cc#newcode1193 extensions/browser/api/usb/usb_api.cc:1193: for (const auto& packet : packets) { test for ...
4 years, 10 months ago (2016-01-27 01:42:13 UTC) #7
scheib
https://codereview.chromium.org/1618393004/diff/80001/device/devices_app/usb/type_converters.h File device/devices_app/usb/type_converters.h (right): https://codereview.chromium.org/1618393004/diff/80001/device/devices_app/usb/type_converters.h#newcode19 device/devices_app/usb/type_converters.h:19: // Type converters to convert objects between internal device/usb ...
4 years, 10 months ago (2016-01-28 03:55:54 UTC) #8
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1618393004/diff/80001/device/devices_app/usb/type_converters.h File device/devices_app/usb/type_converters.h (right): https://codereview.chromium.org/1618393004/diff/80001/device/devices_app/usb/type_converters.h#newcode19 device/devices_app/usb/type_converters.h:19: // Type converters to convert objects between internal device/usb ...
4 years, 10 months ago (2016-01-29 23:25:36 UTC) #9
scheib
LGTM, with comment: https://codereview.chromium.org/1618393004/diff/80001/extensions/browser/api/usb/usb_api.cc File extensions/browser/api/usb/usb_api.cc (right): https://codereview.chromium.org/1618393004/diff/80001/extensions/browser/api/usb/usb_api.cc#newcode1215 extensions/browser/api/usb/usb_api.cc:1215: // Returning arguments with an error ...
4 years, 10 months ago (2016-01-30 22:01:06 UTC) #10
Reilly Grant (use Gerrit)
dgozman@chromium.org: Please review changes in android_usb_browsertest.cc.
4 years, 10 months ago (2016-02-01 21:56:36 UTC) #12
dgozman
android_usb_browsertest.cc lgtm
4 years, 10 months ago (2016-02-01 22:58:32 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1618393004/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1618393004/120001
4 years, 10 months ago (2016-02-01 23:08:20 UTC) #16
commit-bot: I haz the power
Committed patchset #3 (id:120001)
4 years, 10 months ago (2016-02-02 00:56:29 UTC) #17
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/051c98e9d3e843295d659b5676fcfa9dc1be5da6 Cr-Commit-Position: refs/heads/master@{#372844}
4 years, 10 months ago (2016-02-02 00:58:18 UTC) #19
zhaoqin1
4 years, 10 months ago (2016-02-02 17:22:31 UTC) #20
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:120001) has been created in
https://codereview.chromium.org/1658953003/ by zhaoqin@chromium.org.

The reason for reverting is: Uninit error from UsbApiTest.TransferFailure

BUG=583343.

Powered by Google App Engine
This is Rietveld 408576698