Prevent timed-out USB transfers from being cancelled twice.

device/usb/usb_device_handle_usbfs.cc

Index: device/usb/usb_device_handle_usbfs.cc
diff --git a/device/usb/usb_device_handle_usbfs.cc b/device/usb/usb_device_handle_usbfs.cc
index cddb2f69678982c517c90a3ac1d927da9f2ad5f2..de4d92f5a0c8ac9040042e04bf95e39dd421c2a7 100644
--- a/device/usb/usb_device_handle_usbfs.cc
+++ b/device/usb/usb_device_handle_usbfs.cc
@@ -236,9 +236,6 @@ struct UsbDeviceHandleUsbfs::Transfer {
 
   scoped_refptr<net::IOBuffer> control_transfer_buffer;
   scoped_refptr<net::IOBuffer> buffer;
-  TransferCallback callback;
-  IsochronousTransferCallback isoc_callback;
-  scoped_refptr<base::SingleThreadTaskRunner> callback_runner;
   base::CancelableClosure timeout_closure;
   bool cancelled = false;
 
@@ -254,6 +251,10 @@ struct UsbDeviceHandleUsbfs::Transfer {
   usbdevfs_urb urb;
 
  private:
+  TransferCallback callback;

[juncai, 2016/12/20 18:18:54]

The comment for the above |urb| variable says: "The |urb| field must be the last
in the struct ...", so by moving these three variables after |urb|, it may
affect the overridden new function?

[Reilly Grant (use Gerrit), 2016/12/20 20:28:59]

Done, good catch.

+  IsochronousTransferCallback isoc_callback;
+  scoped_refptr<base::SingleThreadTaskRunner> callback_runner;
+
   DISALLOW_COPY_AND_ASSIGN(Transfer);
 };
 
@@ -292,20 +293,22 @@ void* UsbDeviceHandleUsbfs::Transfer::operator new(
 void UsbDeviceHandleUsbfs::Transfer::RunCallback(UsbTransferStatus status,
                                                  size_t bytes_transferred) {
   DCHECK_NE(urb.type, USBDEVFS_URB_TYPE_ISO);
-  DCHECK(!callback.is_null());
+  DCHECK(callback);
   if (!callback_runner || callback_runner->BelongsToCurrentThread()) {
     callback.Run(status, buffer, bytes_transferred);
   } else {
     callback_runner->PostTask(
         FROM_HERE, base::Bind(callback, status, buffer, bytes_transferred));
   }
+  callback.Reset();
 }
 
 void UsbDeviceHandleUsbfs::Transfer::RunIsochronousCallback(
     const std::vector<IsochronousPacket>& packets) {
   DCHECK_EQ(urb.type, USBDEVFS_URB_TYPE_ISO);
-  DCHECK(!isoc_callback.is_null());
+  DCHECK(isoc_callback);
   isoc_callback.Run(buffer, packets);
+  isoc_callback.Reset();
 }
 
 UsbDeviceHandleUsbfs::UsbDeviceHandleUsbfs(
@@ -339,10 +342,12 @@ void UsbDeviceHandleUsbfs::Close() {
   // On the |task_runner_| thread check |device_| to see if the handle is
   // closed. On the |blocking_task_runner_| thread check |fd_.is_valid()| to
   // see if the handle is closed.
-  for (const auto& transfer : transfers_)
-    CancelTransfer(transfer.get(), USB_TRANSFER_CANCELLED);
   device_->HandleClosed(this);
   device_ = nullptr;
+
+  for (const auto& transfer : transfers_)
+    CancelTransfer(transfer.get(), USB_TRANSFER_CANCELLED);
+
   blocking_task_runner_->PostTask(
       FROM_HERE, base::Bind(&UsbDeviceHandleUsbfs::CloseBlocking, this));
 }
@@ -848,6 +853,9 @@ UsbDeviceHandleUsbfs::RemoveFromTransferList(Transfer* transfer_ptr) {
 
 void UsbDeviceHandleUsbfs::CancelTransfer(Transfer* transfer,
                                           UsbTransferStatus status) {
+  if (transfer->cancelled)
+    return;
+
   // |transfer| must stay in |transfers_| as it is still being processed by the
   // kernel and will be reaped later.
   transfer->cancelled = true;