bluetooth: show better error messages for services, characteristics and descriptors

bluetooth: show better error messages for services, characteristics and descriptors.

The error message now contains the UUID of the respective service, characteristic or descriptor.

BUG=619932
Review-Url: https://codereview.chromium.org/2680783002
Cr-Commit-Position: refs/heads/master@{#453590}
Committed: https://chromium.googlesource.com/chromium/src/+/b120052928a9adbea3433c206ad659a6197c8600

[perja, 2017-02-07 13:02:53]

The CQ bit was checked by perja@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 13:03:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-07 14:48:32]

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

[commit-bot: I haz the power, 2017-02-07 14:48:33]

Dry run: This issue passed the CQ dry run.

[perja, 2017-02-07 15:01:59]

perja@opera.com changed reviewers:
+ scheib@chromium.org

[perja, 2017-02-07 15:01:59]

@scheib: PTAL.

[scheib, 2017-02-07 18:36:28]

Thanks, please expand this to descriptors as well, now that they've landed.

[ortuno, 2017-02-07 21:54:45]

ortuno@chromium.org changed reviewers:
+ ortuno@chromium.org

[ortuno, 2017-02-07 21:54:46]

https://codereview.chromium.org/2680783002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right):

https://codereview.chromium.org/2680783002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:27: static
DOMException* take(
I don't think it makes sense to keep using take. We needed it because of
CallbackPromiseAdapter which we no longer use. I think it would make more sense
to move CreateDOMException from BluetoothRemoteGATTUtils here and add enums for
the new types of errors.

https://codereview.chromium.org/2680783002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:30: String
detailedMessage /* Detailed error message - can be empty.*/);
Also could this be an optional argument so we can use the same method?

[perja, 2017-02-08 15:01:47]

On 2017/02/07 18:36:28, scheib wrote:
> Thanks, please expand this to descriptors as well, now that they've landed.

Good point. Will create a patch for that.

[perja, 2017-02-08 15:05:47]

https://codereview.chromium.org/2680783002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right):

https://codereview.chromium.org/2680783002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:27: static
DOMException* take(
On 2017/02/07 21:54:46, ortuno wrote:
> I don't think it makes sense to keep using take. We needed it because of
> CallbackPromiseAdapter which we no longer use. I think it would make more
sense
> to move CreateDOMException from BluetoothRemoteGATTUtils here and add enums
for
> the new types of errors.

Done.

https://codereview.chromium.org/2680783002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:30: String
detailedMessage /* Detailed error message - can be empty.*/);
On 2017/02/07 21:54:46, ortuno wrote:
> Also could this be an optional argument so we can use the same method?

Done.

[perja, 2017-02-08 15:05:58]

The CQ bit was checked by perja@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 15:06:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 16:36:46]

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

[commit-bot: I haz the power, 2017-02-08 16:36:47]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[scheib, 2017-02-08 18:58:04]

On 2017/02/08 15:01:47, perja wrote:
> On 2017/02/07 18:36:28, scheib wrote:
> > Thanks, please expand this to descriptors as well, now that they've landed.
> 
> Good point. Will create a patch for that.

LGTM -- though I see that descriptors are not handled in this change. Unsure if
"Will create a patch for that" means you intended to do it in this updated
patch, or in a new change. Either way sounds good.

[dougt, 2017-02-09 04:00:50]

On 2017/02/08 18:58:04, scheib wrote:
> On 2017/02/08 15:01:47, perja wrote:
> > On 2017/02/07 18:36:28, scheib wrote:
> > > Thanks, please expand this to descriptors as well, now that they've
landed.
> > 
> > Good point. Will create a patch for that.
> 
> LGTM -- though I see that descriptors are not handled in this change. Unsure
if
> "Will create a patch for that" means you intended to do it in this updated
> patch, or in a new change. Either way sounds good.

The issue for descriptors is 619932, fwiw.

[perja, 2017-02-09 10:01:37]

The CQ bit was checked by perja@opera.com

[commit-bot: I haz the power, 2017-02-09 10:02:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 10:09:02]

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

[commit-bot: I haz the power, 2017-02-09 10:09:03]

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

[perja, 2017-02-09 10:25:59]

perja@opera.com changed reviewers:
+ dcheng@chromium.org

[perja, 2017-02-09 10:26:00]

dcheng: SECURITY_OWNERS review for the changes in web_bluetooth.mojom.

[dcheng, 2017-02-09 19:16:42]

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp (right):

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp:25: return
DOMException::create(name, detailedMessage); \
FWIW, I'm not an owner of this code, but I'd prefer that we just have a separate
exception helper for exceptions with a detailed exception.

With this, I really can't tell when I'm supposed to provide a detailed message
or not, since it's completely runtime controlled. It's also a bunch of code for
code paths that will literally never be taken.

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right):

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:22: static
DOMException* CreateDOMException(
Nit: Blink naming convention is createDOMException. It's better to be
consistent, even though this is changing soon.

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:24: String
detailedMessage = String());
const String&

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTUtils.h
(left):

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTUtils.h:22:
kInvalidDescriptor
Are these still needed?

[perja, 2017-02-10 13:28:16]

dcheng: thanks, PTAL again.

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp (right):

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp:25: return
DOMException::create(name, detailedMessage); \
On 2017/02/09 19:16:41, dcheng wrote:
> FWIW, I'm not an owner of this code, but I'd prefer that we just have a
separate
> exception helper for exceptions with a detailed exception.
> 
> With this, I really can't tell when I'm supposed to provide a detailed message
> or not, since it's completely runtime controlled. It's also a bunch of code
for
> code paths that will literally never be taken.

Done.

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right):

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:22: static
DOMException* CreateDOMException(
On 2017/02/09 19:16:41, dcheng wrote:
> Nit: Blink naming convention is createDOMException. It's better to be
> consistent, even though this is changing soon.

Done.

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:24: String
detailedMessage = String());
On 2017/02/09 19:16:41, dcheng wrote:
> const String&

Done.

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTUtils.h
(left):

https://codereview.chromium.org/2680783002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTUtils.h:22:
kInvalidDescriptor
On 2017/02/09 19:16:42, dcheng wrote:
> Are these still needed?

Done.

[perja, 2017-02-10 15:17:55]

The CQ bit was checked by perja@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-10 15:18:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[perja, 2017-02-10 15:20:34]

Description was changed from

==========
bluetooth: show better error messages for missing services and characteristics.

BUG=619932
==========

to

==========
bluetooth: show better error messages for services, characteristics and
descriptors.

The error message now contains the UUID of the respective service,
characteristic or descriptor.

BUG=619932
==========

[commit-bot: I haz the power, 2017-02-10 16:50:22]

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

[commit-bot: I haz the power, 2017-02-10 16:50:23]

Dry run: This issue passed the CQ dry run.

[scheib, 2017-02-10 18:25:34]

https://codereview.chromium.org/2680783002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp (right):

https://codereview.chromium.org/2680783002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp:66:
MAP_ERROR(INVALID_SERVICE, InvalidStateError,
On 2017/02/10 13:28:16, perja wrote:
> On 2017/02/09 19:16:41, dcheng wrote:
> > FWIW, I'm not an owner of this code, but I'd prefer that we just have a
> separate
> > exception helper for exceptions with a detailed exception.
> > 
> > With this, I really can't tell when I'm supposed to provide a detailed
message
> > or not, since it's completely runtime controlled. It's also a bunch of code
> for
> > code paths that will literally never be taken.
> 
> Done.

This new pattern does not LGTM. (Sorry for noticing this late.) It creates case
handlers that won't be executed, and in this file need to either be
speculatively filled in or runtime checked with NOTREACHED(). We end up
duplicating handlers e.g. for INVALID_SERVICE.

I suggest we split the mojom enum WebBluetoothResult into two:
enum WebBluetoothResult
enum WebBluetoothResultWithMessage
and each handler will know precisely how to handle that enum. I think all enums
will fall into only one or the other.

We should avoid switch statements having a "default:" handler, so that the
compiler can help us verify that all cases are handled.

[perja, 2017-02-20 15:51:38]

> I suggest we split the mojom enum WebBluetoothResult into two:
> enum WebBluetoothResult
> enum WebBluetoothResultWithMessage
> and each handler will know precisely how to handle that enum. I think all
enums
> will fall into only one or the other.
> 
> We should avoid switch statements having a "default:" handler, so that the
> compiler can help us verify that all cases are handled.

scheib: Splitting the enum wasn't so straightforward as I had hoped for. The
main problem is that functions in web_bluetooth.mojom returns exactly one result
code and it is of type WebBluetoothResult. So introducing a new enum
(WebBluetoothResultWithMessage) would mean that the error code would have to be
wrapped in a mojom union or something similar. Is that a way to solve this?

[ortuno, 2017-02-20 21:01:23]

On 2017/02/20 at 15:51:38, perja wrote:
> > I suggest we split the mojom enum WebBluetoothResult into two:
> > enum WebBluetoothResult
> > enum WebBluetoothResultWithMessage
> > and each handler will know precisely how to handle that enum. I think all
enums
> > will fall into only one or the other.
> > 
> > We should avoid switch statements having a "default:" handler, so that the
> > compiler can help us verify that all cases are handled.
> 
> scheib: Splitting the enum wasn't so straightforward as I had hoped for. The
main problem is that functions in web_bluetooth.mojom returns exactly one result
code and it is of type WebBluetoothResult. So introducing a new enum
(WebBluetoothResultWithMessage) would mean that the error code would have to be
wrapped in a mojom union or something similar. Is that a way to solve this?

Is there any reason why we need a mojo enum? We could just have a enum in
BluetoothError similar to what we had in BluetoothRemoteGATTUtils.

[perja, 2017-02-21 15:54:36]

On 2017/02/20 21:01:23, ortuno wrote:
> On 2017/02/20 at 15:51:38, perja wrote:
> > > I suggest we split the mojom enum WebBluetoothResult into two:
> > > enum WebBluetoothResult
> > > enum WebBluetoothResultWithMessage
> > > and each handler will know precisely how to handle that enum. I think all
> enums
> > > will fall into only one or the other.
> > > 
> > > We should avoid switch statements having a "default:" handler, so that the
> > > compiler can help us verify that all cases are handled.
> > 
> > scheib: Splitting the enum wasn't so straightforward as I had hoped for. The
> main problem is that functions in web_bluetooth.mojom returns exactly one
result
> code and it is of type WebBluetoothResult. So introducing a new enum
> (WebBluetoothResultWithMessage) would mean that the error code would have to
be
> wrapped in a mojom union or something similar. Is that a way to solve this?
> 
> Is there any reason why we need a mojo enum? We could just have a enum in
> BluetoothError similar to what we had in BluetoothRemoteGATTUtils.

The mojo enum is part of the interface. So I guess it needs to be there, but
maybe it is possible to move it out of mojo and still use it? I'm not that
familiar with mojo.

Another option is to follow what is done in
chrome/browser/media/router/mojo/media_router.mojom and add a "string?
error_text" parameter. That seems kind of nice as the caller provides both the
error code and the message together. The mapping from result code to
ExceptionCode still needs to happen on the blink side. What do you think?

[ortuno, 2017-02-21 21:33:16]

On 2017/02/21 at 15:54:36, perja wrote:
> On 2017/02/20 21:01:23, ortuno wrote:
> > On 2017/02/20 at 15:51:38, perja wrote:
> > > > I suggest we split the mojom enum WebBluetoothResult into two:
> > > > enum WebBluetoothResult
> > > > enum WebBluetoothResultWithMessage
> > > > and each handler will know precisely how to handle that enum. I think
all
> > enums
> > > > will fall into only one or the other.
> > > > 
> > > > We should avoid switch statements having a "default:" handler, so that
the
> > > > compiler can help us verify that all cases are handled.
> > > 
> > > scheib: Splitting the enum wasn't so straightforward as I had hoped for.
The
> > main problem is that functions in web_bluetooth.mojom returns exactly one
result
> > code and it is of type WebBluetoothResult. So introducing a new enum
> > (WebBluetoothResultWithMessage) would mean that the error code would have to
be
> > wrapped in a mojom union or something similar. Is that a way to solve this?
> > 
> > Is there any reason why we need a mojo enum? We could just have a enum in
> > BluetoothError similar to what we had in BluetoothRemoteGATTUtils.
> 
> The mojo enum is part of the interface. So I guess it needs to be there, but
maybe it is possible to move it out of mojo and still use it? I'm not that
familiar with mojo.
> 

Sorry I should have been clearer. The mojo interface is used to talk between the
browser and renderer; these new values are only going to be used by the renderer
so I don't understand why we are introducing a new enum or adding values to the
WebBluetoothError mojo enum. Since these values are renderer only we should be
able to make a new enum in BluetoothError and add these values there.

> Another option is to follow what is done in
chrome/browser/media/router/mojo/media_router.mojom and add a "string?
error_text" parameter. That seems kind of nice as the caller provides both the
error code and the message together. The mapping from result code to
ExceptionCode still needs to happen on the blink side. What do you think?

[perja, 2017-02-22 15:32:46]

On 2017/02/21 21:33:16, ortuno wrote:
> On 2017/02/21 at 15:54:36, perja wrote:
> > On 2017/02/20 21:01:23, ortuno wrote:
> > > On 2017/02/20 at 15:51:38, perja wrote:
> > > > > I suggest we split the mojom enum WebBluetoothResult into two:
> > > > > enum WebBluetoothResult
> > > > > enum WebBluetoothResultWithMessage
> > > > > and each handler will know precisely how to handle that enum. I think
> all
> > > enums
> > > > > will fall into only one or the other.
> > > > > 
> > > > > We should avoid switch statements having a "default:" handler, so that
> the
> > > > > compiler can help us verify that all cases are handled.
> > > > 
> > > > scheib: Splitting the enum wasn't so straightforward as I had hoped for.
> The
> > > main problem is that functions in web_bluetooth.mojom returns exactly one
> result
> > > code and it is of type WebBluetoothResult. So introducing a new enum
> > > (WebBluetoothResultWithMessage) would mean that the error code would have
to
> be
> > > wrapped in a mojom union or something similar. Is that a way to solve
this?
> > > 
> > > Is there any reason why we need a mojo enum? We could just have a enum in
> > > BluetoothError similar to what we had in BluetoothRemoteGATTUtils.
> > 
> > The mojo enum is part of the interface. So I guess it needs to be there, but
> maybe it is possible to move it out of mojo and still use it? I'm not that
> familiar with mojo.
> > 
> 
> Sorry I should have been clearer. The mojo interface is used to talk between
the
> browser and renderer; these new values are only going to be used by the
renderer
> so I don't understand why we are introducing a new enum or adding values to
the
> WebBluetoothError mojo enum. Since these values are renderer only we should be
> able to make a new enum in BluetoothError and add these values there.
> 
> > Another option is to follow what is done in
> chrome/browser/media/router/mojo/media_router.mojom and add a "string?
> error_text" parameter. That seems kind of nice as the caller provides both the
> error code and the message together. The mapping from result code to
> ExceptionCode still needs to happen on the blink side. What do you think?

Right, yes, that is true for INVALID_[SERVICE|CHARACTERISTIC|DESCRIPTOR]. I can
move those to BluetoothError. The others are returned via callbacks from
web_bluetooth_service_impl.cc.

[perja, 2017-02-22 16:01:57]

The CQ bit was checked by perja@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 16:02:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dcheng, 2017-02-22 18:09:06]

(Is this ready for re-review?)

[commit-bot: I haz the power, 2017-02-22 18:12:11]

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

[commit-bot: I haz the power, 2017-02-22 18:12:13]

Dry run: This issue passed the CQ dry run.

[perja, 2017-02-22 21:10:23]

On 2017/02/22 18:09:06, dcheng wrote:
> (Is this ready for re-review?)

Yes, PTAK again.

[perja, 2017-02-24 09:53:37]

@scheib: PTAL again.

I couldn't find a way to easily split the enum in web_bluetooth.mojo so ended up
with a separate enum in BluetoothError for the error codes that requires a
separate error message.

[scheib, 2017-02-27 17:57:14]

Sorry for the difficulty in trying to split the mojom enum, thanks for seeing
through to this solution. LGTM, but please:

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp (right):

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp:38: NOTREACHED();
Please add a comment here explaining why these three are not expected in this
switch, because they have detailed messages and are expected to be redirected to
the switch above.

[dcheng, 2017-02-28 05:09:51]

mojo lgtm

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right):

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:15: enum
BluetoothErrorCode {
enum class or nest it inside the BluetoothError class. These names are generic
enough that we probably shouldn't have them in the global namespace.

(I'd suggest enum class since it seems like a lot of places already qualify it
with BluetoothErrorCode)

[perja, 2017-02-28 11:45:19]

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp (right):

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp:38: NOTREACHED();
On 2017/02/27 17:57:14, scheib wrote:
> Please add a comment here explaining why these three are not expected in this
> switch, because they have detailed messages and are expected to be redirected
to
> the switch above.

Done.

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right):

https://codereview.chromium.org/2680783002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:15: enum
BluetoothErrorCode {
On 2017/02/28 05:09:51, dcheng wrote:
> enum class or nest it inside the BluetoothError class. These names are generic
> enough that we probably shouldn't have them in the global namespace.
> 
> (I'd suggest enum class since it seems like a lot of places already qualify it
> with BluetoothErrorCode)

Done.

[perja, 2017-02-28 11:45:28]

The CQ bit was checked by perja@opera.com

[perja, 2017-02-28 11:45:28]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2680783002/#ps180001
(title: "Added comment and changed to enum class.")

[commit-bot: I haz the power, 2017-02-28 11:45:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 13:49:35]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1488282328422560,
"parent_rev": "7fd6ca24a1fa92bcaa0484e6792aacb2b87f34ef", "commit_rev":
"b120052928a9adbea3433c206ad659a6197c8600"}

[commit-bot: I haz the power, 2017-02-28 13:50:13]

Description was changed from

==========
bluetooth: show better error messages for services, characteristics and
descriptors.

The error message now contains the UUID of the respective service,
characteristic or descriptor.

BUG=619932
==========

to

==========
bluetooth: show better error messages for services, characteristics and
descriptors.

The error message now contains the UUID of the respective service,
characteristic or descriptor.

BUG=619932

Review-Url: https://codereview.chromium.org/2680783002
Cr-Commit-Position: refs/heads/master@{#453590}
Committed:
https://chromium.googlesource.com/chromium/src/+/b120052928a9adbea3433c206ad6...
==========

[commit-bot: I haz the power, 2017-02-28 13:50:14]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/b120052928a9adbea3433c206ad6...