Add a UsbService::Observer function for cleanup actions.

Add a UsbService::Observer function for cleanup actions.

This patch adds an additional function to the UsbService::Observer
interface that should be used to perform cleanup operations when a USB
device is disconnected. This is separate from the OnDeviceRemoved
function so that implementations of that function, which may depend on
other subsystems' knowledge of a connected device, can run before any
cleanup actions have taken place.

A browsertest for the chrome.usb.getUserSelectedDevices is included
which demonstrates the need for this 2-phase event handling.

BUG=452298
Committed: https://crrev.com/dd6141829c1ad86297b0a2e7faea037ff110956b
Cr-Commit-Position: refs/heads/master@{#314973}

[Reilly Grant (use Gerrit), 2015-01-30 21:57:55]

reillyg@chromium.org changed reviewers:
+ rockot@chromium.org

[Reilly Grant (use Gerrit), 2015-01-30 22:28:57]

Ken, please take a look a this patch we've been bikeshedding.

[Ken Rockot(use gerrit already), 2015-02-02 19:41:50]

I think we could make this work, but I am really not fond of the two-phase
approach. "Cleanup" is really just "Part2" and I worry that introducing a Part2
increases the risk of eventually having Part3 and Part4.

It's especially odd that the only reason to introduce a Part2 is that exactly
one of the observers wants to run after all the others have run. To me this
implies that said observer is not an equal to the other observers and its
special relationship to the UsbService should probably not be shoehorned into
the observer interface.

I'd like to think more about this. Maybe the bikeshed could be striped.

[Reilly Grant (use Gerrit), 2015-02-03 02:28:44]

On 2015/02/02 19:41:50, Ken Rockot wrote:
> I think we could make this work, but I am really not fond of the two-phase
> approach. "Cleanup" is really just "Part2" and I worry that introducing a
Part2
> increases the risk of eventually having Part3 and Part4.
>
> It's especially odd that the only reason to introduce a Part2 is that exactly
> one of the observers wants to run after all the others have run. To me this
> implies that said observer is not an equal to the other observers and its
> special relationship to the UsbService should probably not be shoehorned into
> the observer interface.

I could imagine there being multiple observers that want to subscribe to the
second signal. The goal of the comment is to emphasize that this is not simply
about ordering. It is about expectations about other parts of the system as the
device removal is processed.

> I'd like to think more about this. Maybe the bikeshed could be striped.

This implementation is a simpler form of my original idea, which was to add an
observer interface to UsbDevice itself which notifies observers from within the
UsbDevice destructor. There are a number of complications in that approach
because UsbDevice is RefCountedThreadSafe. Adding the second observer method
results in identical behavior while having a much more straightforward
implementation.

[Ken Rockot(use gerrit already), 2015-02-05 23:30:55]

On 2015/02/03 02:28:44, reillyg wrote:
> On 2015/02/02 19:41:50, Ken Rockot wrote:
> > I think we could make this work, but I am really not fond of the two-phase
> > approach. "Cleanup" is really just "Part2" and I worry that introducing a
> Part2
> > increases the risk of eventually having Part3 and Part4.
> >
> > It's especially odd that the only reason to introduce a Part2 is that
exactly
> > one of the observers wants to run after all the others have run. To me this
> > implies that said observer is not an equal to the other observers and its
> > special relationship to the UsbService should probably not be shoehorned
into
> > the observer interface.
> 
> I could imagine there being multiple observers that want to subscribe to the
> second signal. The goal of the comment is to emphasize that this is not simply
> about ordering. It is about expectations about other parts of the system as
the
> device removal is processed.
> 
> > I'd like to think more about this. Maybe the bikeshed could be striped.
> 
> This implementation is a simpler form of my original idea, which was to add an
> observer interface to UsbDevice itself which notifies observers from within
the
> UsbDevice destructor. There are a number of complications in that approach
> because UsbDevice is RefCountedThreadSafe. Adding the second observer method
> results in identical behavior while having a much more straightforward
> implementation.

OK - sorry for delaying this so long. I haven't been able to come up with any
alternative that I feel good about suggesting. I would still like to bikeshed
this some more in the near future, but LGTM because race conditions are lame!

[Reilly Grant (use Gerrit), 2015-02-06 02:14:13]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2015-02-06 02:14:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/891853002/1

[commit-bot: I haz the power, 2015-02-06 04:15:18]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-02-06 04:16:08]

Patchset 1 (id:??) landed as
https://crrev.com/dd6141829c1ad86297b0a2e7faea037ff110956b
Cr-Commit-Position: refs/heads/master@{#314973}