Add a service to track devices selected by the user.

Add a service to track devices selected by the user.

apps::SavedDevicesService tracks USB devices that have been selected by
the user in the context of a given extension. Devices that can be
identified accurately after they have been reconnected because they have
a serial number are written out to ExtensionPrefs. All others are only
remembered until they are disconnected.

A new OnDisconnect observer function has been added to UsbDevice to
enable this monitoring.

BUG=346953
Committed: https://crrev.com/89b276fbcd87b3ce88be7b0a00df6d9266d90ad6
Cr-Commit-Position: refs/heads/master@{#295708}

[Reilly Grant (use Gerrit), 2014-09-17 21:43:53]

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

[Reilly Grant (use Gerrit), 2014-09-17 22:58:31]

Patchset #2 (id:20001) has been deleted

[Reilly Grant (use Gerrit), 2014-09-17 22:58:37]

Patchset #1 (id:1) has been deleted

[Reilly Grant (use Gerrit), 2014-09-17 23:36:27]

Patchset #1 (id:40001) has been deleted

[Reilly Grant (use Gerrit), 2014-09-17 23:36:34]

Patchset #1 (id:60001) has been deleted

[Reilly Grant (use Gerrit), 2014-09-18 00:03:52]

Patchset #1 (id:80001) has been deleted

[Reilly Grant (use Gerrit), 2014-09-18 19:00:58]

Patchset #2 (id:120001) has been deleted

[scheib, 2014-09-18 20:31:49]

https://codereview.chromium.org/580963002/diff/100001/apps/DEPS
File apps/DEPS (right):

https://codereview.chromium.org/580963002/diff/100001/apps/DEPS#newcode12
apps/DEPS:12: "+device/usb",
Update apps/BUILD.gn also

https://codereview.chromium.org/580963002/diff/100001/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/580963002/diff/100001/apps/apps.gypi#newcode48
apps/apps.gypi:48: 'saved_devices_service.cc',
Update apps/BUILD.gn also

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
File apps/saved_devices_service.cc (right):

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:169: Clear(extension->id());
Maybe just Clear(host->extension_id())

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:183: SavedDevicesService::SavedDevices*
SavedDevicesService::Get(
Please keep the definitions in the same order as the declarations.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Declaration_O...

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:233: ephemeral_devices_.begin();
Add comments here explaining why only ephemeral devices are observing. OK to
just cite a common explanation at class level.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:268: device->AddObserver(this);
Probably comment here too why only ephemeral devices observe.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:310: ephemeral_devices_.erase(device);
... explaining comment

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
File apps/saved_devices_service.h (right):

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.h:74: explicit SavedDevices(Profile* profile, const
std::string& extension_id);
explicit is unnecessary for multiple argument constructors.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
File apps/saved_devices_service_unittest.cc (right):

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service_unittest.cc:188: }
Consider testing the restoration of saved devices after an extension is deleted
and recreated?

[Reilly Grant (use Gerrit), 2014-09-18 21:28:00]

https://codereview.chromium.org/580963002/diff/100001/apps/DEPS
File apps/DEPS (right):

https://codereview.chromium.org/580963002/diff/100001/apps/DEPS#newcode12
apps/DEPS:12: "+device/usb",
On 2014/09/18 20:31:48, scheib wrote:
> Update apps/BUILD.gn also

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/580963002/diff/100001/apps/apps.gypi#newcode48
apps/apps.gypi:48: 'saved_devices_service.cc',
On 2014/09/18 20:31:48, scheib wrote:
> Update apps/BUILD.gn also

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
File apps/saved_devices_service.cc (right):

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:169: Clear(extension->id());
On 2014/09/18 20:31:48, scheib wrote:
> Maybe just Clear(host->extension_id())

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:183: SavedDevicesService::SavedDevices*
SavedDevicesService::Get(
On 2014/09/18 20:31:48, scheib wrote:
> Please keep the definitions in the same order as the declarations.
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Declaration_O...

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:233: ephemeral_devices_.begin();
On 2014/09/18 20:31:48, scheib wrote:
> Add comments here explaining why only ephemeral devices are observing. OK to
> just cite a common explanation at class level.

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:268: device->AddObserver(this);
On 2014/09/18 20:31:48, scheib wrote:
> Probably comment here too why only ephemeral devices observe.

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.cc:310: ephemeral_devices_.erase(device);
On 2014/09/18 20:31:48, scheib wrote:
> ... explaining comment

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
File apps/saved_devices_service.h (right):

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service.h:74: explicit SavedDevices(Profile* profile, const
std::string& extension_id);
On 2014/09/18 20:31:49, scheib wrote:
> explicit is unnecessary for multiple argument constructors.

Done.

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
File apps/saved_devices_service_unittest.cc (right):

https://codereview.chromium.org/580963002/diff/100001/apps/saved_devices_serv...
apps/saved_devices_service_unittest.cc:188: }
On 2014/09/18 20:31:49, scheib wrote:
> Consider testing the restoration of saved devices after an extension is
deleted
> and recreated?

I'm not sure how to do that with TestExtensionEnvironment. I'll figure it out.

[scheib, 2014-09-18 22:37:51]

lgtm

[Reilly Grant (use Gerrit), 2014-09-18 22:46:30]

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

[Reilly Grant (use Gerrit), 2014-09-18 22:47:54]

Adding pfeldman@ to review the android_usb_browsertests.cc fixes that are
required due to a Visual Studio bug that affects gMock when pass-by-value
parameters are marked as const.

[pfeldman, 2014-09-19 11:05:23]

lgtm.

[Reilly Grant (use Gerrit), 2014-09-19 14:31:49]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2014-09-19 14:32:26]

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

[commit-bot: I haz the power, 2014-09-19 15:00:11]

Committed patchset #3 (id:160001) as 967f02b4072272c305a0083e36748a865a0e737f

[commit-bot: I haz the power, 2014-09-19 15:00:56]

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