Get all the UsbConfigDescriptor for the device configuration

Add function to get all the UsbConfigDescriptor for the device
configuration

This patch added a function to get all the UsbConfigDescriptor for the
device configuration. And changed GetConfiguration function
name to GetActiveConfiguration.

Committed: https://crrev.com/e4d532aa1982271f5b8a65345a92aa6434758826
Cr-Commit-Position: refs/heads/master@{#341925}

[juncai, 2015-07-31 23:07:39]

juncai@chromium.org changed reviewers:
+ achuith@chromium.org, dgozman@chromium.org, reillyg@chromium.org

[juncai, 2015-07-31 23:07:39]

achuith@chromium.org: Please review changes in 
chrome/browser/chromeos/printer_detector/printer_detector_unittest.cc

dgozman@chromium.org: Please review changes in 
chrome/browser/devtools/device/usb/android_usb_browsertest.cc
chrome/browser/devtools/device/usb/android_usb_device.cc

reillyg@chromium.org: Please review changes in 
the remaining files

[Reilly Grant (use Gerrit), 2015-07-31 23:36:13]

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device.h
File device/usb/usb_device.h (right):

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device.h...
device/usb/usb_device.h:57: virtual const UsbConfigDescriptor*
GetCurrentConfiguration() = 0;
GetCurrentConfiguration -> GetActiveConfiguration

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device.h...
device/usb/usb_device.h:60: virtual const std::vector<UsbConfigDescriptor>&
configurations() const = 0;
I'd rather this was not a virtual function. You can declare a protected
set_configurations() method that UsbDeviceImpl uses to set the configurations
after the device has been constructed.

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device_i...
File device/usb/usb_device_impl.cc (right):

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device_i...
device/usb/usb_device_impl.cc:251: SetConfigDescriptor(platform_config,
configuration_.get());
Instead of reparsing platform_config just find the member of configurations_
that has the right bConfigurationValue and return a const pointer to it.

[dgozman, 2015-08-03 07:55:55]

chrome/browser/devtools rubberstamp lgtm

Note that you may just TBR trivial API renames as long as the owner of said API
has reviewed the patch.

[juncai, 2015-08-03 17:46:01]

Please review the change.

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device.h
File device/usb/usb_device.h (right):

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device.h...
device/usb/usb_device.h:57: virtual const UsbConfigDescriptor*
GetCurrentConfiguration() = 0;
On 2015/07/31 23:36:12, reillyg wrote:
> GetCurrentConfiguration -> GetActiveConfiguration

Done.

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device.h...
device/usb/usb_device.h:60: virtual const std::vector<UsbConfigDescriptor>&
configurations() const = 0;
On 2015/07/31 23:36:12, reillyg wrote:
> I'd rather this was not a virtual function. You can declare a protected
> set_configurations() method that UsbDeviceImpl uses to set the configurations
> after the device has been constructed.

Done.

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device_i...
File device/usb/usb_device_impl.cc (right):

https://codereview.chromium.org/1265833005/diff/60001/device/usb/usb_device_i...
device/usb/usb_device_impl.cc:251: SetConfigDescriptor(platform_config,
configuration_.get());
On 2015/07/31 23:36:12, reillyg wrote:
> Instead of reparsing platform_config just find the member of configurations_
> that has the right bConfigurationValue and return a const pointer to it.

Done.

[Reilly Grant (use Gerrit), 2015-08-03 18:04:04]

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device.h
File device/usb/usb_device.h (right):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device....
device/usb/usb_device.h:59: // Gets all the UsbConfigDescriptor for the device
configuration.
"Gets all of the device's UsbConfigDescriptors."

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device....
device/usb/usb_device.h:79: // The device's all configuration descriptors.
"All of the device's configuration descriptors."

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
File device/usb/usb_device_impl.cc (right):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:91: void SetConfigDescriptor(const
libusb_config_descriptor* platform_config,
I suggest renaming this to "ConvertConfigDescriptor()".

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:164: set_configurations();
This function is non-trivial and so cannot use this style of function name. I
suggest "ReadAllConfigurations()".

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:165: RefreshConfiguration();
I suggest renaming this to "RefreshActiveConfiguration()".

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
File device/usb/usb_device_impl.h (left):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.h:8: #include <vector>
Don't remove these includes just because usb_device.h includes vector and
usb_descriptors.h.

See:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Ord...

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
File device/usb/usb_device_impl.h (right):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.h:103: const UsbConfigDescriptor* configuration_;
Maybe call this "active_configuration_" and document that it is a pointer into
configurations_.

[juncai, 2015-08-03 21:56:25]

Please review the change.

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device.h
File device/usb/usb_device.h (right):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device....
device/usb/usb_device.h:59: // Gets all the UsbConfigDescriptor for the device
configuration.
On 2015/08/03 18:04:04, reillyg wrote:
> "Gets all of the device's UsbConfigDescriptors."

Done.

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device....
device/usb/usb_device.h:79: // The device's all configuration descriptors.
On 2015/08/03 18:04:04, reillyg wrote:
> "All of the device's configuration descriptors."

Done.

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
File device/usb/usb_device_impl.cc (right):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:91: void SetConfigDescriptor(const
libusb_config_descriptor* platform_config,
On 2015/08/03 18:04:04, reillyg wrote:
> I suggest renaming this to "ConvertConfigDescriptor()".

Done.

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:164: set_configurations();
On 2015/08/03 18:04:04, reillyg wrote:
> This function is non-trivial and so cannot use this style of function name. I
> suggest "ReadAllConfigurations()".

Done.

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:165: RefreshConfiguration();
On 2015/08/03 18:04:04, reillyg wrote:
> I suggest renaming this to "RefreshActiveConfiguration()".

Done.

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
File device/usb/usb_device_impl.h (left):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.h:8: #include <vector>
On 2015/08/03 18:04:04, reillyg wrote:
> Don't remove these includes just because usb_device.h includes vector and
> usb_descriptors.h.
> 
> See:
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Ord...

Done.

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
File device/usb/usb_device_impl.h (right):

https://codereview.chromium.org/1265833005/diff/100001/device/usb/usb_device_...
device/usb/usb_device_impl.h:103: const UsbConfigDescriptor* configuration_;
On 2015/08/03 18:04:04, reillyg wrote:
> Maybe call this "active_configuration_" and document that it is a pointer into
> configurations_.

Done.

[Reilly Grant (use Gerrit), 2015-08-03 22:06:55]

lgtm

https://codereview.chromium.org/1265833005/diff/120001/device/usb/usb_device_...
File device/usb/usb_device_impl.cc (right):

https://codereview.chromium.org/1265833005/diff/120001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:272: size_t num_configurations =
configurations_.size();
There's no reason not to just use configurations_.size() in the loop.

[juncai, 2015-08-04 04:19:00]

https://codereview.chromium.org/1265833005/diff/120001/device/usb/usb_device_...
File device/usb/usb_device_impl.cc (right):

https://codereview.chromium.org/1265833005/diff/120001/device/usb/usb_device_...
device/usb/usb_device_impl.cc:272: size_t num_configurations =
configurations_.size();
On 2015/08/03 22:06:55, reillyg wrote:
> There's no reason not to just use configurations_.size() in the loop.

Done.

[juncai, 2015-08-04 04:19:12]

The CQ bit was checked by juncai@chromium.org

[juncai, 2015-08-04 04:19:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, reillyg@chromium.org
Link to the patchset: https://codereview.chromium.org/1265833005/#ps180001
(title: " ")

[commit-bot: I haz the power, 2015-08-04 04:19:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1265833005/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1265833005/180001

[commit-bot: I haz the power, 2015-08-04 04:27:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-04 04:27:37]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[juncai, 2015-08-04 04:43:01]

achuithb@, ping?

[Reilly Grant (use Gerrit), 2015-08-04 23:47:08]

https://codereview.chromium.org/1265833005/diff/180001/device/devices_app/usb...
File device/devices_app/usb/device_impl.cc (left):

https://codereview.chromium.org/1265833005/diff/180001/device/devices_app/usb...
device/devices_app/usb/device_impl.cc:127: info->configurations =
mojo::Array<ConfigurationInfoPtr>::New(0);
Please update this method to fill info->configurations with the full set of
configurations instead of just the active configuration as it does now. You can
do this as a separate review if you'd like.

[juncai, 2015-08-05 04:23:15]

juncai@chromium.org changed reviewers:
+ stevenjb@chromium.org
- achuith@chromium.org

[juncai, 2015-08-05 04:23:16]

stevenjb@chromium.org: Please review changes in 
chrome/browser/chromeos/printer_detector/printer_detector_unittest.cc

[juncai, 2015-08-05 04:24:44]

juncai@chromium.org changed reviewers:
+ achuith@google.com

[juncai, 2015-08-05 04:25:43]

juncai@chromium.org changed reviewers:
+ achuith@chromium.org
- achuith@google.com

[stevenjb, 2015-08-05 15:11:48]

c/b/chromeos lgtm

[juncai, 2015-08-05 15:36:23]

On 2015/08/04 23:47:08, reillyg wrote:
>
https://codereview.chromium.org/1265833005/diff/180001/device/devices_app/usb...
> File device/devices_app/usb/device_impl.cc (left):
> 
>
https://codereview.chromium.org/1265833005/diff/180001/device/devices_app/usb...
> device/devices_app/usb/device_impl.cc:127: info->configurations =
> mojo::Array<ConfigurationInfoPtr>::New(0);
> Please update this method to fill info->configurations with the full set of
> configurations instead of just the active configuration as it does now. You
can
> do this as a separate review if you'd like.

I will do this in another patch.

[juncai, 2015-08-05 15:37:23]

The CQ bit was checked by juncai@chromium.org

[commit-bot: I haz the power, 2015-08-05 15:38:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1265833005/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1265833005/180001

[juncai, 2015-08-05 15:42:17]

https://codereview.chromium.org/1265833005/diff/180001/device/devices_app/usb...
File device/devices_app/usb/device_impl.cc (left):

https://codereview.chromium.org/1265833005/diff/180001/device/devices_app/usb...
device/devices_app/usb/device_impl.cc:127: info->configurations =
mojo::Array<ConfigurationInfoPtr>::New(0);
On 2015/08/04 23:47:08, reillyg wrote:
> Please update this method to fill info->configurations with the full set of
> configurations instead of just the active configuration as it does now. You
can
> do this as a separate review if you'd like.

I will do this in another patch.

[commit-bot: I haz the power, 2015-08-05 18:05:44]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-08-05 18:06:33]

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