Track USB device configuration state in Blink.

Track USB device configuration state in Blink.

Blink now tracks the currently selected device configuration and can
thus provide it as an IDL attribute without a getConfiguration() method
which has been removed.

This continues the trend of being able to handle errors in Blink before
calling methods on the Mojo interface.

BUG=593164
R=juncai@chromium.org
TBR=dgozman@chromium.org

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

[Reilly Grant (use Gerrit), 2016-03-10 01:17:16]

Please take a look.

[juncai, 2016-03-10 22:14:04]

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webusb/USBDevice.cpp (right):

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:128:
m_device->onConfigurationSelected(true, m_configurationValue);
nit: add comment for the true parameter.

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:137:
m_device->onConfigurationSelected(false, m_configurationValue);
nit: add comment for the false parameter.

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:324:
resolver->reject(DOMException::create(InvalidStateError,
kDeviceStateChangeInProgress));
The above two if conditions don't need the foundConfiguration, so by moving the
for loop inside an else can save the for loop computation for these two if
conditions:

} else {
     bool foundConfiguration = false;
     for (size_t i = 0; i < info().configurations.size(); ++i) {
         if (info().configurations[i].configurationValue == configurationValue)
{
             foundConfiguration = true;
             break;
         }
    }

    if (!foundConfiguration) {
        resolver->reject(DOMException::create(NotFoundError, "The configuration
value provided is not supported by the device."));
    } else if (m_activeConfiguration == configurationValue) {
        resolver->resolve();
    } else {
        m_deviceStateChangeInProgress = true;
        m_device->setConfiguration(configurationValue, new
SelectConfigurationPromiseAdapter(this, resolver, configurationValue));
    }
}

[Reilly Grant (use Gerrit), 2016-03-11 00:08:36]

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webusb/USBDevice.cpp (right):

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:128:
m_device->onConfigurationSelected(true, m_configurationValue);
On 2016/03/10 at 22:14:04, juncai wrote:
> nit: add comment for the true parameter.

Done.

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:137:
m_device->onConfigurationSelected(false, m_configurationValue);
On 2016/03/10 at 22:14:04, juncai wrote:
> nit: add comment for the false parameter.

Done.

https://codereview.chromium.org/1784733002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:324:
resolver->reject(DOMException::create(InvalidStateError,
kDeviceStateChangeInProgress));
On 2016/03/10 at 22:14:04, juncai wrote:
> The above two if conditions don't need the foundConfiguration, so by moving
the for loop inside an else can save the for loop computation for these two if
conditions:
> 
> } else {
>      bool foundConfiguration = false;
>      for (size_t i = 0; i < info().configurations.size(); ++i) {
>          if (info().configurations[i].configurationValue ==
configurationValue) {
>              foundConfiguration = true;
>              break;
>          }
>     }
> 
>     if (!foundConfiguration) {
>         resolver->reject(DOMException::create(NotFoundError, "The
configuration value provided is not supported by the device."));
>     } else if (m_activeConfiguration == configurationValue) {
>         resolver->resolve();
>     } else {
>         m_deviceStateChangeInProgress = true;
>         m_device->setConfiguration(configurationValue, new
SelectConfigurationPromiseAdapter(this, resolver, configurationValue));
>     }
> }

Done.

[juncai, 2016-03-11 02:17:22]

https://codereview.chromium.org/1784733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webusb/USBDevice.cpp (right):

https://codereview.chromium.org/1784733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:255: for (size_t i = 0; i
< info().configurations.size(); ++i) {
This info() is a function call, maybe can store it into a variable:
const WebUSBDeviceInfo& info = info(); to avoid multiple function calls in a for
loop.

https://codereview.chromium.org/1784733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:286: for (size_t i = 0; i
< info().configurations.size(); ++i)
ditto.

https://codereview.chromium.org/1784733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:331: for (size_t i = 0; i
< info().configurations.size(); ++i) {
ditto.

[Reilly Grant (use Gerrit), 2016-03-12 02:43:03]

https://codereview.chromium.org/1784733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webusb/USBDevice.cpp (right):

https://codereview.chromium.org/1784733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:255: for (size_t i = 0; i
< info().configurations.size(); ++i) {
On 2016/03/11 at 02:17:21, juncai wrote:
> This info() is a function call, maybe can store it into a variable:
> const WebUSBDeviceInfo& info = info(); to avoid multiple function calls in a
for loop.

Done.

https://codereview.chromium.org/1784733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:331: for (size_t i = 0; i
< info().configurations.size(); ++i) {
On 2016/03/11 at 02:17:21, juncai wrote:
> ditto.

Done.

[juncai, 2016-03-14 16:47:45]

LGTM with nits.

https://codereview.chromium.org/1784733002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webusb/USBDevice.cpp (right):

https://codereview.chromium.org/1784733002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:501:
resolver->reject(DOMException::create(InvalidStateError, "The device must have a
configuration selected."));
nit: there was a const char kNotConfigured[] for this string before but was
removed, why?

[Reilly Grant (use Gerrit), 2016-03-14 17:20:36]

https://codereview.chromium.org/1784733002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webusb/USBDevice.cpp (right):

https://codereview.chromium.org/1784733002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:501:
resolver->reject(DOMException::create(InvalidStateError, "The device must have a
configuration selected."));
On 2016/03/14 at 16:47:45, juncai wrote:
> nit: there was a const char kNotConfigured[] for this string before but was
removed, why?

In this most recent patchset the string is now only used in this one place.

[juncai, 2016-03-14 17:22:45]

https://codereview.chromium.org/1784733002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webusb/USBDevice.cpp (right):

https://codereview.chromium.org/1784733002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webusb/USBDevice.cpp:501:
resolver->reject(DOMException::create(InvalidStateError, "The device must have a
configuration selected."));
On 2016/03/14 17:20:36, Reilly Grant wrote:
> On 2016/03/14 at 16:47:45, juncai wrote:
> > nit: there was a const char kNotConfigured[] for this string before but was
> removed, why?
> 
> In this most recent patchset the string is now only used in this one place.

I see. Thanks!

[Reilly Grant (use Gerrit), 2016-03-15 18:25:34]

The CQ bit was checked by reillyg@chromium.org

[Reilly Grant (use Gerrit), 2016-03-15 18:25:34]

The patchset sent to the CQ was uploaded after l-g-t-m from juncai@chromium.org
Link to the patchset: https://codereview.chromium.org/1784733002/#ps80001
(title: "Rebased.")

[commit-bot: I haz the power, 2016-03-15 18:25:47]

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

[commit-bot: I haz the power, 2016-03-15 19:47:18]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-15 19:48:22]

Description was changed from

==========
Track USB device configuration state in Blink.

Blink now tracks the currently selected device configuration and can
thus provide it as an IDL attribute without a getConfiguration() method
which has been removed.

This continues the trend of being able to handle errors in Blink before
calling methods on the Mojo interface.

BUG=593164
R=juncai@chromium.org
TBR=dgozman@chromium.org
==========

to

==========
Track USB device configuration state in Blink.

Blink now tracks the currently selected device configuration and can
thus provide it as an IDL attribute without a getConfiguration() method
which has been removed.

This continues the trend of being able to handle errors in Blink before
calling methods on the Mojo interface.

BUG=593164
R=juncai@chromium.org
TBR=dgozman@chromium.org

Committed: https://crrev.com/cc9b0c5fa36368bb012b4b4d572ada52ceb1146b
Cr-Commit-Position: refs/heads/master@{#381288}
==========

[commit-bot: I haz the power, 2016-03-15 19:48:23]

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