Fix crash when closing a device from a USB transfer timeout

device/usb/usb_device_handle_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "device/usb/usb_device_handle.h"
 
 #include <stddef.h>
 
 #include <memory>
 
@@ skipping 98 matching lines @@
     transferred_ = transferred;
     run_loop_.Quit();
   }
 
   const UsbDeviceHandle::TransferCallback callback_;
   base::RunLoop run_loop_;
   UsbTransferStatus status_;
   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;
   }
 
   std::unique_ptr<UsbTestGadget> gadget =
       UsbTestGadget::Claim(io_thread_->task_runner());
   ASSERT_TRUE(gadget.get());
   ASSERT_TRUE(gadget->SetType(UsbTestGadget::ECHO));
 
@@ skipping 274 matching lines @@
                           buffer->size(),
                           10,  // 10 millisecond timeout
                           completion.callback());
 
   completion.WaitForResult();
   ASSERT_EQ(UsbTransferStatus::TIMEOUT, completion.status());
 
   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