Fix crash when closing a device from a USB transfer timeout

device/usb/usb_device_handle_unittest.cc

Index: device/usb/usb_device_handle_unittest.cc
diff --git a/device/usb/usb_device_handle_unittest.cc b/device/usb/usb_device_handle_unittest.cc
index 20a76e4f24443ae7a729919c26acb081881c4e3b..2c925df95340338338234b14f0cc446235c33e72 100644
--- a/device/usb/usb_device_handle_unittest.cc
+++ b/device/usb/usb_device_handle_unittest.cc
@@ -116,6 +116,16 @@ class TestCompletionCallback {
   size_t transferred_;
 };
 
+void ExpectTimeoutAndClose(scoped_refptr<UsbDeviceHandle> handle,
+                           const base::Closure& quit_closure,
+                           UsbTransferStatus status,
+                           scoped_refptr<net::IOBuffer> buffer,
+                           size_t transferred) {
+  EXPECT_EQ(status, UsbTransferStatus::TIMEOUT);

[mcasas, 2017/04/24 21:19:23]

nit: EXPECT_EQ(expected, actual) [1]  so reverse them.

(confusingly enough, gtest is opposite from the js test harness)

[1]
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/gtest.h?type...

[Reilly Grant (use Gerrit), 2017/04/24 22:14:00]

Done.

+  handle->Close();
+  quit_closure.Run();
+}
+
 TEST_F(UsbDeviceHandleTest, InterruptTransfer) {
   if (!UsbTestGadget::IsTestEnabled()) {
     return;
@@ -410,6 +420,35 @@ TEST_F(UsbDeviceHandleTest, Timeout) {
   handle->Close();
 }
 
+TEST_F(UsbDeviceHandleTest, CloseReentrancy) {
+  if (!UsbTestGadget::IsTestEnabled())
+    return;
+
+  std::unique_ptr<UsbTestGadget> gadget =
+      UsbTestGadget::Claim(io_thread_->task_runner());
+  ASSERT_TRUE(gadget.get());
+  ASSERT_TRUE(gadget->SetType(UsbTestGadget::ECHO));
+
+  TestOpenCallback open_device;
+  gadget->GetDevice()->Open(open_device.callback());
+  scoped_refptr<UsbDeviceHandle> handle = open_device.WaitForResult();
+  ASSERT_TRUE(handle.get());
+
+  TestResultCallback claim_interface;
+  handle->ClaimInterface(1, claim_interface.callback());
+  ASSERT_TRUE(claim_interface.WaitForResult());

[mcasas, 2017/04/24 21:19:22]

nit: This preamble l.424-439 looks very similar to e.g.
l. 392-408 and l.359-375. Consider refactoring them out.

[Reilly Grant (use Gerrit), 2017/04/24 22:14:00]

The preamble varies subtly depending on what kind of transfer we're testing.

+
+  base::RunLoop run_loop;
+  scoped_refptr<net::IOBufferWithSize> buffer(new net::IOBufferWithSize(512));
+  handle->GenericTransfer(
+      UsbTransferDirection::INBOUND, 0x82, buffer.get(), buffer->size(),
+      10,  // 10 millisecond timeout
+      base::Bind(&ExpectTimeoutAndClose, handle, run_loop.QuitClosure()));
+  // Drop handle so that the completion callback holds the last reference.
+  handle = nullptr;
+  run_loop.Run();
+}
+
 }  // namespace
 
 }  // namespace device