Fixes leaking transfer objects due to improper USB handle closure.

Fixes leaking transfer objects due to improper USB handle closure. 

We were simply close the device handle and left all the flying transfers hanging. libusb guarantees that the transfer callbacks will be called even if canceled. But it was not freed because during the libusb_close the user_data of the transfer is reset, so our handler will not be relaid to. 

This change is to wait for all the transfers to be canceled before we really close the platform handle.  

BUG=223817
BUG=261695
BUG=269048
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216793

[Bei Zhang, 2013-08-08 07:15:45]

Hi Pavel and Zelidrag,

Please review.

Thanks,
Bei

[pfeldman, 2013-08-09 08:58:59]

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
File chrome/browser/usb/usb_device_handle.cc (right):

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.cc:239:
transfer.callback.Run(ConvertTransferStatus(handle->status), buffer,
You should respond on the thread transfer was requested, not on FILE thread.

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.cc:253: const int rv =
libusb_claim_interface(handle_, interface_number);
I think libusb_claim_interface and free_interface should be in the same class
(InterfaceClaimer). You could do

claimer = new InterfaceClaimer();
if (claimer->Claim())
   ...

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.cc:512: AddRef();
You should instead add scoped_refptr<UsbDeviceHandle> on Transfer.

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
File chrome/browser/usb/usb_device_handle.h (right):

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.h:127: friend void
HandleTransferCompletion(PlatformUsbTransferHandle handle);
This can be defined in .cc

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.h:129: class InterfaceClaimer : public
base::RefCountedThreadSafe<InterfaceClaimer> {
No need for it for be ref-counted. You can store it in scoped_ptr.

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.h:175: // callback mechanism in such a way
that it cannot be made private. Invokes
It can now be private (and is such), fix the comment!

[Bei Zhang, 2013-08-09 16:43:55]

Thanks for reviewing!

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
File chrome/browser/usb/usb_device_handle.cc (right):

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.cc:239:
transfer.callback.Run(ConvertTransferStatus(handle->status), buffer,
On 2013/08/09 08:58:59, pfeldman wrote:
> You should respond on the thread transfer was requested, not on FILE thread.

Done.

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.cc:253: const int rv =
libusb_claim_interface(handle_, interface_number);
On 2013/08/09 08:58:59, pfeldman wrote:
> I think libusb_claim_interface and free_interface should be in the same class
> (InterfaceClaimer). You could do
> 
> claimer = new InterfaceClaimer();
> if (claimer->Claim())
>    ...

Done.

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.cc:512: AddRef();
On 2013/08/09 08:58:59, pfeldman wrote:
> You should instead add scoped_refptr<UsbDeviceHandle> on Transfer.

Done.

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
File chrome/browser/usb/usb_device_handle.h (right):

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.h:127: friend void
HandleTransferCompletion(PlatformUsbTransferHandle handle);
On 2013/08/09 08:58:59, pfeldman wrote:
> This can be defined in .cc

How would you do that?

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.h:129: class InterfaceClaimer : public
base::RefCountedThreadSafe<InterfaceClaimer> {
On 2013/08/09 08:58:59, pfeldman wrote:
> No need for it for be ref-counted. You can store it in scoped_ptr.

It will be shared, by both UsbDeviceHandle and Transfer.

https://codereview.chromium.org/22492003/diff/31001/chrome/browser/usb/usb_de...
chrome/browser/usb/usb_device_handle.h:175: // callback mechanism in such a way
that it cannot be made private. Invokes
On 2013/08/09 08:58:59, pfeldman wrote:
> It can now be private (and is such), fix the comment!

Done.

[pfeldman, 2013-08-09 17:13:18]

lgtm

[pfeldman, 2013-08-09 17:13:20]

lgtm

[commit-bot: I haz the power, 2013-08-09 18:49:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ikarienator@chromium.org/22492003/70001

[commit-bot: I haz the power, 2013-08-09 22:35:41]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-08-09 22:47:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ikarienator@chromium.org/22492003/70001

[commit-bot: I haz the power, 2013-08-10 01:05:02]

Change committed as 216793