Skip to content

Commit

Permalink
Remove unnecessary optional flag from arrays in USB results
Browse files Browse the repository at this point in the history 
In practice there was never a case where we had a non-fatal USB error
which didn't return a buffer so this simplifies the generate code by
making the data field non-optional. Creating empty vectors is now
trivial so there is little effect on the case where no buffer is
available.

Change-Id: Iddf143039c8de2631d0912f1665a1074676c5fd6
Reviewed-on: https://chromium-review.googlesource.com/674386
Reviewed-by: Chris Palmer <palmer@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503307}
  • Loading branch information
@reillyeon
reillyeon authored and Commit Bot committed Sep 21, 2017
1 parent 5abf6bd commit 24e50c3
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 41 deletions.
14 changes: 5 additions & 9 deletions device/usb/mojo/device_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ void DeviceImpl::ControlTransferIn(UsbControlTransferParamsPtr params,
uint32_t timeout,
ControlTransferInCallback callback) {
if (!device_handle_) {
std::move(callback).Run(mojom::UsbTransferStatus::TRANSFER_ERROR,
base::nullopt);
std::move(callback).Run(mojom::UsbTransferStatus::TRANSFER_ERROR, {});
return;
}

Expand All @@ -312,8 +311,7 @@ void DeviceImpl::ControlTransferIn(UsbControlTransferParamsPtr params,
params->request, params->value, params->index, buffer, length, timeout,
base::BindOnce(&OnTransferIn, std::move(callback)));
} else {
std::move(callback).Run(mojom::UsbTransferStatus::PERMISSION_DENIED,
base::nullopt);
std::move(callback).Run(mojom::UsbTransferStatus::PERMISSION_DENIED, {});
}
}

Expand Down Expand Up @@ -343,8 +341,7 @@ void DeviceImpl::GenericTransferIn(uint8_t endpoint_number,
uint32_t timeout,
GenericTransferInCallback callback) {
if (!device_handle_) {
std::move(callback).Run(mojom::UsbTransferStatus::TRANSFER_ERROR,
base::nullopt);
std::move(callback).Run(mojom::UsbTransferStatus::TRANSFER_ERROR, {});
return;
}

Expand Down Expand Up @@ -379,9 +376,8 @@ void DeviceImpl::IsochronousTransferIn(
IsochronousTransferInCallback callback) {
if (!device_handle_) {
std::move(callback).Run(
base::nullopt,
BuildIsochronousPacketArray(packet_lengths,
mojom::UsbTransferStatus::TRANSFER_ERROR));
{}, BuildIsochronousPacketArray(
packet_lengths, mojom::UsbTransferStatus::TRANSFER_ERROR));
return;
}

Expand Down
27 changes: 12 additions & 15 deletions device/usb/mojo/device_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,15 @@ void ExpectResultAndThen(bool expected_result,
continuation.Run();
}

void ExpectTransferInAndThen(
mojom::UsbTransferStatus expected_status,
const std::vector<uint8_t>& expected_bytes,
const base::Closure& continuation,
mojom::UsbTransferStatus actual_status,
const base::Optional<std::vector<uint8_t>>& actual_bytes) {
void ExpectTransferInAndThen(mojom::UsbTransferStatus expected_status,
const std::vector<uint8_t>& expected_bytes,
const base::Closure& continuation,
mojom::UsbTransferStatus actual_status,
const std::vector<uint8_t>& actual_bytes) {
EXPECT_EQ(expected_status, actual_status);
ASSERT_TRUE(actual_bytes);
ASSERT_EQ(expected_bytes.size(), actual_bytes->size());
for (size_t i = 0; i < actual_bytes->size(); ++i) {
EXPECT_EQ(expected_bytes[i], (*actual_bytes)[i])
ASSERT_EQ(expected_bytes.size(), actual_bytes.size());
for (size_t i = 0; i < actual_bytes.size(); ++i) {
EXPECT_EQ(expected_bytes[i], actual_bytes[i])
<< "Contents differ at index: " << i;
}
continuation.Run();
Expand All @@ -112,7 +110,7 @@ void ExpectPacketsInAndThen(
const std::vector<uint8_t>& expected_bytes,
const std::vector<uint32_t>& expected_packets,
const base::Closure& continuation,
const base::Optional<std::vector<uint8_t>>& actual_bytes,
const std::vector<uint8_t>& actual_bytes,
std::vector<UsbIsochronousPacketPtr> actual_packets) {
ASSERT_EQ(expected_packets.size(), actual_packets.size());
for (size_t i = 0; i < expected_packets.size(); ++i) {
Expand All @@ -121,10 +119,9 @@ void ExpectPacketsInAndThen(
EXPECT_EQ(mojom::UsbTransferStatus::COMPLETED, actual_packets[i]->status)
<< "Packet at index " << i << " not completed.";
}
ASSERT_TRUE(actual_bytes);
ASSERT_EQ(expected_bytes.size(), actual_bytes->size());
for (size_t i = 0; i < actual_bytes->size(); ++i) {
EXPECT_EQ(expected_bytes[i], (*actual_bytes)[i])
ASSERT_EQ(expected_bytes.size(), actual_bytes.size());
for (size_t i = 0; i < actual_bytes.size(); ++i) {
EXPECT_EQ(expected_bytes[i], actual_bytes[i])
<< "Contents differ at index: " << i;
}
continuation.Run();
Expand Down
6 changes: 3 additions & 3 deletions device/usb/public/interfaces/device.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ interface UsbDevice {
ControlTransferIn(UsbControlTransferParams params,
uint32 length,
uint32 timeout)
=> (UsbTransferStatus status, array<uint8>? data);
=> (UsbTransferStatus status, array<uint8> data);

// Initiates an inbound control transfer request. |params| determine the
// details of the request. Transfers to recipients other than DEVICE require a
Expand Down Expand Up @@ -210,7 +210,7 @@ interface UsbDevice {
// indicates no timeout: the request will remain pending indefinitely until
// completed or otherwise terminated.
GenericTransferIn(uint8 endpoint_number, uint32 length, uint32 timeout)
=> (UsbTransferStatus status, array<uint8>? data);
=> (UsbTransferStatus status, array<uint8> data);

// Initiates an outbound generic transfer request on a specific endpoint. The
// interface to which |endpoint_number| belongs must be claimed, and the
Expand Down Expand Up @@ -247,7 +247,7 @@ interface UsbDevice {
IsochronousTransferIn(uint8 endpoint_number,
array<uint32> packet_lengths,
uint32 timeout)
=> (array<uint8>? data, array<UsbIsochronousPacket> packets);
=> (array<uint8> data, array<UsbIsochronousPacket> packets);

// Initiates an outbound isochronous transfer request on a specific endpoint.
// The interface to which |endpoint_number| belongs must be claimed, and the
Expand Down
9 changes: 4 additions & 5 deletions third_party/WebKit/Source/modules/webusb/USBDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ void USBDevice::AsyncSelectAlternateInterface(size_t interface_index,

void USBDevice::AsyncControlTransferIn(ScriptPromiseResolver* resolver,
UsbTransferStatus status,
const Optional<Vector<uint8_t>>& data) {
const Vector<uint8_t>& data) {
if (!MarkRequestComplete(resolver))
return;

Expand Down Expand Up @@ -826,7 +826,7 @@ void USBDevice::AsyncClearHalt(ScriptPromiseResolver* resolver, bool success) {

void USBDevice::AsyncTransferIn(ScriptPromiseResolver* resolver,
UsbTransferStatus status,
const Optional<Vector<uint8_t>>& data) {
const Vector<uint8_t>& data) {
if (!MarkRequestComplete(resolver))
return;

Expand Down Expand Up @@ -856,13 +856,12 @@ void USBDevice::AsyncTransferOut(unsigned transfer_length,

void USBDevice::AsyncIsochronousTransferIn(
ScriptPromiseResolver* resolver,
const Optional<Vector<uint8_t>>& data,
const Vector<uint8_t>& data,
Vector<UsbIsochronousPacketPtr> mojo_packets) {
if (!MarkRequestComplete(resolver))
return;

DOMArrayBuffer* buffer =
data ? DOMArrayBuffer::Create(data->data(), data->size()) : nullptr;
DOMArrayBuffer* buffer = DOMArrayBuffer::Create(data.data(), data.size());
HeapVector<Member<USBIsochronousInTransferPacket>> packets;
packets.ReserveCapacity(mojo_packets.size());
size_t byte_offset = 0;
Expand Down
6 changes: 3 additions & 3 deletions third_party/WebKit/Source/modules/webusb/USBDevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,20 +145,20 @@ class USBDevice : public GarbageCollectedFinalized<USBDevice>,
bool success);
void AsyncControlTransferIn(ScriptPromiseResolver*,
device::mojom::blink::UsbTransferStatus,
const Optional<Vector<uint8_t>>&);
const Vector<uint8_t>&);
void AsyncControlTransferOut(unsigned,
ScriptPromiseResolver*,
device::mojom::blink::UsbTransferStatus);
void AsyncClearHalt(ScriptPromiseResolver*, bool success);
void AsyncTransferIn(ScriptPromiseResolver*,
device::mojom::blink::UsbTransferStatus,
const Optional<Vector<uint8_t>>&);
const Vector<uint8_t>&);
void AsyncTransferOut(unsigned,
ScriptPromiseResolver*,
device::mojom::blink::UsbTransferStatus);
void AsyncIsochronousTransferIn(
ScriptPromiseResolver*,
const Optional<Vector<uint8_t>>&,
const Vector<uint8_t>&,
Vector<device::mojom::blink::UsbIsochronousPacketPtr>);
void AsyncIsochronousTransferOut(
ScriptPromiseResolver*,
Expand Down
9 changes: 3 additions & 6 deletions third_party/WebKit/Source/modules/webusb/USBInTransferResult.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,9 @@ class USBInTransferResult final

public:
static USBInTransferResult* Create(const String& status,
const Optional<Vector<uint8_t>>& data) {
DOMDataView* data_view = nullptr;
if (data) {
data_view = DOMDataView::Create(
DOMArrayBuffer::Create(data->data(), data->size()), 0, data->size());
}
const Vector<uint8_t>& data) {
DOMDataView* data_view = DOMDataView::Create(
DOMArrayBuffer::Create(data.data(), data.size()), 0, data.size());
return new USBInTransferResult(status, data_view);
}

Expand Down

0 comments on commit 24e50c3

Please sign in to comment.