Added validation of the policy specified in the 'csp' attribute

Added validation of the policy specified in the 'csp' attribute

Spec: https://w3c.github.io/webappsec-csp/embedded/#csp-attribute

The 'csp' attribute is supposed to be validated as a serialized-policy
and set to null if not a valid one

Re-used the existing wpt test that documents this functionality

Fixed the expected and actual order in said test as it was reversed

BUG=647588
Review-Url: https://codereview.chromium.org/2896833002
Cr-Commit-Position: refs/heads/master@{#475487}
Committed: https://chromium.googlesource.com/chromium/src/+/4124bfba87179d73b33c54d6add5b2ac82a090b4

[andypaicu, 2017-05-22 14:08:25]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 14:08:40]

Dry run: CQ is trying da patch.

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

[andypaicu, 2017-05-22 14:08:46]

andypaicu@chromium.org changed reviewers:
+ mkwst@chromium.org

[andypaicu, 2017-05-22 14:09:17]

Description was changed from

==========
Added validation of the policy specified in the 'csp' attribute

Spec: https://w3c.github.io/webappsec-csp/embedded/#csp-attribute

The 'csp' attribute is supposed to be validated as a serialized-policy
and set to null if not a valid one

Re-used the existing wpt test that document this functionality

BUG=647588
==========

to

==========
Added validation of the policy specified in the 'csp' attribute

Spec: https://w3c.github.io/webappsec-csp/embedded/#csp-attribute

The 'csp' attribute is supposed to be validated as a serialized-policy
and set to null if not a valid one

Re-used the existing wpt test that documents this functionality

BUG=647588
==========

[andypaicu, 2017-05-22 14:10:12]

Description was changed from

==========
Added validation of the policy specified in the 'csp' attribute

Spec: https://w3c.github.io/webappsec-csp/embedded/#csp-attribute

The 'csp' attribute is supposed to be validated as a serialized-policy
and set to null if not a valid one

Re-used the existing wpt test that documents this functionality

BUG=647588
==========

to

==========
Added validation of the policy specified in the 'csp' attribute

Spec: https://w3c.github.io/webappsec-csp/embedded/#csp-attribute

The 'csp' attribute is supposed to be validated as a serialized-policy
and set to null if not a valid one

Re-used the existing wpt test that documents this functionality

Fixed the expected and actual order in said test as it was reversed

BUG=647588
==========

[andypaicu, 2017-05-22 14:10:36]

The CQ bit was unchecked by andypaicu@chromium.org

[andypaicu, 2017-05-22 14:29:53]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 14:30:10]

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-05-22 16:18:01]

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

[commit-bot: I haz the power, 2017-05-22 16:18:03]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[andypaicu, 2017-05-23 10:49:46]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 10:50:00]

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-05-23 12:32:52]

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

[commit-bot: I haz the power, 2017-05-23 12:32:54]

Dry run: This issue passed the CQ dry run.

[Mike West, 2017-05-23 19:21:37]

Thanks! The implementation looks good, sorry it took me so long to get to this
review. I've left a few comments inline, mostly about tests I'd like to see. :)

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:991: name = value
= "";
I don't think you actually need two strings. Something like

```
String unused;
...
if (!ParseDirective(directive_begin, position, unused, unused, nullptr))
```

should work.

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:992: if
(!ParseDirective(directive_begin, position, name, value, nullptr))
This is doing to dump console messages that probably don't make sense. I'd
suggest adding something to ... oh. That's why you added the `nullptr`. I see.
Hrm. I think I'd prefer that to be an enum. You can reuse the existing
`SecurityViolationReportingPolicy` enum if you like, but that seems better to me
than passing in a policy at the other callsites.

Oh. Hrm. Nevermind. You made it static. So that doesn't work. Hrm.

Ok. I guess it's not _too_ magical. :/ Can you add a comment to the .h file so
that potential callers understand when reports are generated?

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:996:
ContentSecurityPolicy::DirectiveType::kUndefined)
Style nit: {} after multi-line conditionals.

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right):

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:1159: false);
It would be nice to get a little more coverage here. Can you copy/paste some
items out of
//LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-*.html
to flesh this out with some edge cases?

Ditto for the test we're upstreaming to WPT.

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp (left):

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp:149: if
(!value.GetString().ContainsOnlyASCII()) {
We should probably add some tests for non-ASCII characters.

[andypaicu, 2017-05-26 14:34:17]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-26 14:34:33]

Dry run: CQ is trying da patch.

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

[andypaicu, 2017-05-26 14:41:09]

Modified the way validation is done to simulate creating a CSPDirectiveList and
catching whatever errors might occur.

The previous implementation would not catch directive-specific errors (e.g.
incorrect host in script-src).

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:991: name = value
= "";
On 2017/05/23 at 19:21:36, Mike West wrote:
> I don't think you actually need two strings. Something like
> 
> ```
> String unused;
> ...
> if (!ParseDirective(directive_begin, position, unused, unused, nullptr))
> ```
> 
> should work.

I've modified this whole bit.

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:992: if
(!ParseDirective(directive_begin, position, name, value, nullptr))
On 2017/05/23 at 19:21:36, Mike West wrote:
> This is doing to dump console messages that probably don't make sense. I'd
suggest adding something to ... oh. That's why you added the `nullptr`. I see.
Hrm. I think I'd prefer that to be an enum. You can reuse the existing
`SecurityViolationReportingPolicy` enum if you like, but that seems better to me
than passing in a policy at the other callsites.
> 
> Oh. Hrm. Nevermind. You made it static. So that doesn't work. Hrm.
> 
> Ok. I guess it's not _too_ magical. :/ Can you add a comment to the .h file so
that potential callers understand when reports are generated?

I've modified this whole bit.

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:996:
ContentSecurityPolicy::DirectiveType::kUndefined)
On 2017/05/23 at 19:21:36, Mike West wrote:
> Style nit: {} after multi-line conditionals.

I've modified this whole bit.

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp (right):

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveListTest.cpp:1159: false);
On 2017/05/23 at 19:21:36, Mike West wrote:
> It would be nice to get a little more coverage here. Can you copy/paste some
items out of
//LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-*.html
to flesh this out with some edge cases?
> 
> Ditto for the test we're upstreaming to WPT.

Done. I'll do the WPT tests separately for now. Raised crbug.com/726724

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp (left):

https://codereview.chromium.org/2896833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp:149: if
(!value.GetString().ContainsOnlyASCII()) {
On 2017/05/23 at 19:21:36, Mike West wrote:
> We should probably add some tests for non-ASCII characters.

Done

[commit-bot: I haz the power, 2017-05-26 16:18:10]

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

[commit-bot: I haz the power, 2017-05-26 16:18:11]

Dry run: This issue passed the CQ dry run.

[Mike West, 2017-05-29 07:46:21]

https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:995:
CSPDirectiveList::Create(policy, begin, end, header_type, header_source);
I think this will accept something like `script-src 'none', object-src 'none'`
(which is multiple policies due to the `,`). Can you add a check somewhere for
this case, which shouldn't be accepted as a `csp` attribute?

https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.h (right):

https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.h:132: return new
ContentSecurityPolicy(reporting_policy);
I don't think you need this, because we only collect parse errors in
`console_messages_` during parsing. We send them to the console during
`ApplyPolicySideEffectsToExecutionContext()`, which you'll never call on a CSP
object you only use for validity checking.

How does this sound: add a static `IsValidRequiredCSP(const String& attr)`
method to `ContentSecurityPolicy`, something along the lines of:

```
ContentSecurityPolicy* policy = ContentSecurityPolicy::Create();
policy->AddPolicyFromHeaderValue(attr, kContentSecurityPolicyHeaderTypeEnforce,
kContentSecurityPolicyHeaderSourceHTTP);
if (!policy->console_messages_.IsEmpty()) {
  // Do some extra validity checks here, like checking for
`report-uri`/`report-to`?
  return true;
}
return false;
```

https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp (right):

https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp:171: if
(!CSPDirectiveList::IsValid(
If you have a method on `ContentSecurityPolicy` itself, why do you need to
expose/use the version on CSPDirectiveList?

[andypaicu, 2017-05-29 13:35:13]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-29 13:35:23]

Dry run: CQ is trying da patch.

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

[andypaicu, 2017-05-29 13:36:39]

The CQ bit was unchecked by andypaicu@chromium.org

[andypaicu, 2017-05-29 13:43:44]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-29 13:43:56]

Dry run: CQ is trying da patch.

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

[andypaicu, 2017-05-29 13:45:24]

On 2017/05/29 at 07:46:21, mkwst wrote:
>
https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):
> 
>
https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:995:
CSPDirectiveList::Create(policy, begin, end, header_type, header_source);
> I think this will accept something like `script-src 'none', object-src 'none'`
(which is multiple policies due to the `,`). Can you add a check somewhere for
this case, which shouldn't be accepted as a `csp` attribute?
> 
>
https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.h (right):
> 
>
https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.h:132: return
new ContentSecurityPolicy(reporting_policy);
> I don't think you need this, because we only collect parse errors in
`console_messages_` during parsing. We send them to the console during
`ApplyPolicySideEffectsToExecutionContext()`, which you'll never call on a CSP
object you only use for validity checking.
> 
> How does this sound: add a static `IsValidRequiredCSP(const String& attr)`
method to `ContentSecurityPolicy`, something along the lines of:
> 
> ```
> ContentSecurityPolicy* policy = ContentSecurityPolicy::Create();
> policy->AddPolicyFromHeaderValue(attr,
kContentSecurityPolicyHeaderTypeEnforce,
kContentSecurityPolicyHeaderSourceHTTP);
> if (!policy->console_messages_.IsEmpty()) {
>   // Do some extra validity checks here, like checking for
`report-uri`/`report-to`?
>   return true;
> }
> return false;
> ```
> 
>
https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp (right):
> 
>
https://codereview.chromium.org/2896833002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp:171: if
(!CSPDirectiveList::IsValid(
> If you have a method on `ContentSecurityPolicy` itself, why do you need to
expose/use the version on CSPDirectiveList?

Good idea, I've done pretty much exactly that.

[Mike West, 2017-05-29 13:49:31]

LGTM % nits. Thanks for working on this!

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html
(right):

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html:23:
"expected": null },
Can you copy/paste some of the unittest values in here so that other vendors can
benefit?

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp (right):

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp:1715: return
false;
Hrm. I think I'd prefer that you do both of these checks by verifying that
`policy->policies_.size() == 1` after adding the header to the policy.

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp
(right):

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1180:
EXPECT_EQ(
Nit: EXPECT_TRUE

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1183:
EXPECT_EQ(ContentSecurityPolicy::IsValidCSPAttr(
Nit: EXPECT_FALSE

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1188:
EXPECT_EQ(
Ditto, all the way down.

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1235:
"script-src 127.0.0.1:8000/thisisa;pathwithasemicolon"),
Do we have layout tests covering URL encoded policies? That is, do we do the
right thing with HTML entities (`<` in the attribute), percent-encoding, etc?

[commit-bot: I haz the power, 2017-05-29 15:18:43]

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

[commit-bot: I haz the power, 2017-05-29 15:18:44]

Dry run: This issue passed the CQ dry run.

[andypaicu, 2017-05-30 08:47:07]

The CQ bit was checked by andypaicu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-30 08:47:24]

Dry run: CQ is trying da patch.

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

[andypaicu, 2017-05-30 08:51:22]

Thank you very much Mike for bearing with me through this.

I've raised crbug.com/726724 for the WPT test side of things.

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html
(right):

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html:23:
"expected": null },
On 2017/05/29 at 13:49:31, Mike West wrote:
> Can you copy/paste some of the unittest values in here so that other vendors
can benefit?

I've deferred the WPT tests to a later CL. I've raised crbug.com/726724 to keep
track.

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp (right):

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp:1715: return
false;
On 2017/05/29 at 13:49:31, Mike West wrote:
> Hrm. I think I'd prefer that you do both of these checks by verifying that
`policy->policies_.size() == 1` after adding the header to the policy.

Done

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp
(right):

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1180:
EXPECT_EQ(
On 2017/05/29 at 13:49:31, Mike West wrote:
> Nit: EXPECT_TRUE

Done

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1183:
EXPECT_EQ(ContentSecurityPolicy::IsValidCSPAttr(
On 2017/05/29 at 13:49:31, Mike West wrote:
> Nit: EXPECT_FALSE

Done

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1188:
EXPECT_EQ(
On 2017/05/29 at 13:49:31, Mike West wrote:
> Ditto, all the way down.

Done

https://codereview.chromium.org/2896833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp:1235:
"script-src 127.0.0.1:8000/thisisa;pathwithasemicolon"),
On 2017/05/29 at 13:49:31, Mike West wrote:
> Do we have layout tests covering URL encoded policies? That is, do we do the
right thing with HTML entities (`<` in the attribute), percent-encoding, etc?

No we don't, I'll add some to the WPT tests as part of crbug.com/726724.

[commit-bot: I haz the power, 2017-05-30 10:30:34]

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

[commit-bot: I haz the power, 2017-05-30 10:30:35]

Dry run: This issue passed the CQ dry run.

[andypaicu, 2017-05-30 10:37:35]

The CQ bit was checked by andypaicu@chromium.org

[andypaicu, 2017-05-30 10:37:35]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2896833002/#ps120001
(title: "Code Review suggestions")

[commit-bot: I haz the power, 2017-05-30 10:37:43]

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-05-30 10:42:27]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1496140655119470,
"parent_rev": "2f78b5b66d3637fa3d2b2b13ff2d39d98985d27f", "commit_rev":
"4124bfba87179d73b33c54d6add5b2ac82a090b4"}

[commit-bot: I haz the power, 2017-05-30 10:42:34]

Description was changed from

==========
Added validation of the policy specified in the 'csp' attribute

Spec: https://w3c.github.io/webappsec-csp/embedded/#csp-attribute

The 'csp' attribute is supposed to be validated as a serialized-policy
and set to null if not a valid one

Re-used the existing wpt test that documents this functionality

Fixed the expected and actual order in said test as it was reversed

BUG=647588
==========

to

==========
Added validation of the policy specified in the 'csp' attribute

Spec: https://w3c.github.io/webappsec-csp/embedded/#csp-attribute

The 'csp' attribute is supposed to be validated as a serialized-policy
and set to null if not a valid one

Re-used the existing wpt test that documents this functionality

Fixed the expected and actual order in said test as it was reversed

BUG=647588

Review-Url: https://codereview.chromium.org/2896833002
Cr-Commit-Position: refs/heads/master@{#475487}
Committed:
https://chromium.googlesource.com/chromium/src/+/4124bfba87179d73b33c54d6add5...
==========

[commit-bot: I haz the power, 2017-05-30 10:42:35]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/4124bfba87179d73b33c54d6add5...