Add an Observer interface to UsbService for device add/remove.

Add an Observer interface to UsbService for device add/remove.

On platforms where libusb supports hotplug events this observer interface
allows clients of UsbService to be notified when devices are added and
removed from the system. When hotplug events are not available these
events are only generated in response to an explicit enumeration request.

The UsbDevice::Observer interface has been removed so that UsbDevice can
be a truly thread-safe object.

BUG=411715
Committed: https://crrev.com/4ebb03e08ae49fb5be8306f34bb18c9da905a4aa
Cr-Commit-Position: refs/heads/master@{#310140}

[Reilly Grant (use Gerrit), 2015-01-05 19:19:52]

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

[Reilly Grant (use Gerrit), 2015-01-05 19:21:27]

Antony, please take a look at this patch.

[asargent_no_longer_on_chrome, 2015-01-05 22:01:58]

lgtm w/ a question and a suggestion

https://codereview.chromium.org/809743006/diff/1/device/usb/usb_service_impl.cc
File device/usb/usb_service_impl.cc (right):

https://codereview.chromium.org/809743006/diff/1/device/usb/usb_service_impl....
device/usb/usb_service_impl.cc:183: device->OnDisconnect();
I assume you've already thought about the ramifications of notifying observers
of the UsbDevice removal before calling OnDisconnect on the device itself. Would
there ever be a case where an implementor of the Observer interface might need
to know or care about this ordering? If so it might be worth a mention in a
comment above the Observer interface.

https://codereview.chromium.org/809743006/diff/1/device/usb/usb_service_impl....
device/usb/usb_service_impl.cc:319: UsbService::~UsbService() {
Consider adding a DCHECK in the destructor here that observer_list_ is empty, to
catch cases where someone is accidentally holding on to a pointer to this object
for later calling RemoveObserver (eg via a ScopedObserver).

[Reilly Grant (use Gerrit), 2015-01-05 22:10:34]

https://codereview.chromium.org/809743006/diff/1/device/usb/usb_service_impl.cc
File device/usb/usb_service_impl.cc (right):

https://codereview.chromium.org/809743006/diff/1/device/usb/usb_service_impl....
device/usb/usb_service_impl.cc:183: device->OnDisconnect();
On 2015/01/05 22:01:58, Antony Sargent wrote:
> I assume you've already thought about the ramifications of notifying observers
> of the UsbDevice removal before calling OnDisconnect on the device itself.
Would
> there ever be a case where an implementor of the Observer interface might need
> to know or care about this ordering? If so it might be worth a mention in a
> comment above the Observer interface. 

I'm actually about to remove the observer list from UsbDevice for other reasons
so I'll be removing this call. I'll update the patch with that change.

https://codereview.chromium.org/809743006/diff/1/device/usb/usb_service_impl....
device/usb/usb_service_impl.cc:319: UsbService::~UsbService() {
On 2015/01/05 22:01:58, Antony Sargent wrote:
> Consider adding a DCHECK in the destructor here that observer_list_ is empty,
to
> catch cases where someone is accidentally holding on to a pointer to this
object
> for later calling RemoveObserver (eg via a ScopedObserver). 

There's a template flag I can pass to ObserverList that adds this check.

[Reilly Grant (use Gerrit), 2015-01-06 03:25:52]

Antony, please take another looks as I've updated this patch to remove the old
observer interface.

[asargent_no_longer_on_chrome, 2015-01-06 17:59:03]

new version of code lgtm, just one point of confusion in a comment

https://codereview.chromium.org/809743006/diff/20001/extensions/browser/api/d...
File extensions/browser/api/device_permissions_manager.cc (right):

https://codereview.chromium.org/809743006/diff/20001/extensions/browser/api/d...
extensions/browser/api/device_permissions_manager.cc:445: // observed so that it
is not initialized on profile creation.
I'm confused about what you mean when you say "creating a dependency on the USB
service" here. 

It seems like coming through this branch, you always access the UsbService
(presumably causing it to be created in device::DeviceClient::Get() if it wasn't
already created) and always ensure that if our observer wasn't already
registered as an observer with it, then it will be. How is that delaying
anything?

Perhaps you can reword?

[Reilly Grant (use Gerrit), 2015-01-06 19:06:16]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2015-01-06 19:07:07]

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

[commit-bot: I haz the power, 2015-01-06 20:58:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-06 20:59:45]

Patchset 3 (id:??) landed as
https://crrev.com/4ebb03e08ae49fb5be8306f34bb18c9da905a4aa
Cr-Commit-Position: refs/heads/master@{#310140}