Fix parsing of effective TLD wildcarding across IPC.

To use URLPattern with an effective TLD wildcard, a second parameter must be passed to the constructor specifically allowing this.

The IPC message parsing for URLPattern should always accept an effective TLD wildcard. This is since the check would have been done on the initial parse to populate a URLPattern.

This adds a unit test of the runtime blocked hosts and runtime allowed hosts in the extension messages IPC. This also adds a browser test for an effective TLD wildcard.

BUG=737315
Review-Url: https://codereview.chromium.org/2957233002
Cr-Commit-Position: refs/heads/master@{#483860}
Committed: https://chromium.googlesource.com/chromium/src/+/605f1df1854d4bc8868bfbd3bc4945c8d200ece0

[nrpeter, 2017-06-27 23:38:39]

Description was changed from

==========
Fix parsing of effective TLD wildcarding across IPC.

BUG=737315
==========

to

==========
To use URLPattern with an effective TLD wildcard, a second parameter must be
passed to the constructor specifically allowing this.

The IPC message parsing for URLPattern should always accept an effective TLD
wildcard. This is since the check would have been done on the initial parse to
populate a URLPattern.

This adds a unit test of the runtime blocked hosts and runtime allowed hosts in
the extension messages IPC. This also adds a browser test for an effective TLD
wildcard.

BUG=737315
==========

[nrpeter, 2017-06-27 23:44:43]

nrpeter@google.com changed reviewers:
+ estark@chromium.org

[nrpeter, 2017-06-27 23:44:43]

nrpeter@google.com changed required reviewers:
+ estark@chromium.org

[estark, 2017-06-28 00:40:04]

IPC security lgtm. AFAICT these URLPatterns only get sent from browser ->
renderer so there shouldn't be any security issues.

[nrpeter, 2017-06-30 17:15:07]

nrpeter@google.com changed reviewers:
+ lazyboy@chromium.org

[nrpeter, 2017-06-30 17:15:07]

nrpeter@google.com changed required reviewers:
+ lazyboy@chromium.org

[lazyboy, 2017-06-30 20:43:22]

few nits only.

https://codereview.chromium.org/2957233002/diff/1/chrome/browser/extensions/c...
File chrome/browser/extensions/content_script_apitest.cc (right):

https://codereview.chromium.org/2957233002/diff/1/chrome/browser/extensions/c...
chrome/browser/extensions/content_script_apitest.cc:425: // Verifies wildcard
can be used for effecitve TLD.
Move this line as a test description before line 422.

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
File extensions/common/extension_messages.cc (right):

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
extensions/common/extension_messages.cc:142: // Allow effective TLD wildcarding
since this check is only needed on initial
nit: move this comment to next line as this is talking about Parse method call
below. (And the previous comment is about SetValidSchemes)

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
extensions/common/extension_messages.cc:143: // creation of URLPattern and not
as part of serialization.
nit: this is deserialization, rather than serialization?

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
File extensions/common/extension_messages_unittest.cc (right):

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
extensions/common/extension_messages_unittest.cc:43: static void
AddPattern(URLPatternSet* extent, const std::string& pattern) {
nit: since this is already in anonymous namespace, remove "static"
Also pattern parameter comes before extent as |extent| is output parameter.

[nrpeter, 2017-06-30 21:22:15]

https://codereview.chromium.org/2957233002/diff/1/chrome/browser/extensions/c...
File chrome/browser/extensions/content_script_apitest.cc (right):

https://codereview.chromium.org/2957233002/diff/1/chrome/browser/extensions/c...
chrome/browser/extensions/content_script_apitest.cc:425: // Verifies wildcard
can be used for effecitve TLD.
On 2017/06/30 20:43:22, lazyboy wrote:
> Move this line as a test description before line 422.

Done.

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
File extensions/common/extension_messages.cc (right):

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
extensions/common/extension_messages.cc:142: // Allow effective TLD wildcarding
since this check is only needed on initial
On 2017/06/30 20:43:22, lazyboy wrote:
> nit: move this comment to next line as this is talking about Parse method call
> below. (And the previous comment is about SetValidSchemes)

Done.

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
extensions/common/extension_messages.cc:143: // creation of URLPattern and not
as part of serialization.
On 2017/06/30 20:43:22, lazyboy wrote:
> nit: this is deserialization, rather than serialization?

Done.

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
File extensions/common/extension_messages_unittest.cc (right):

https://codereview.chromium.org/2957233002/diff/1/extensions/common/extension...
extensions/common/extension_messages_unittest.cc:43: static void
AddPattern(URLPatternSet* extent, const std::string& pattern) {
On 2017/06/30 20:43:22, lazyboy wrote:
> nit: since this is already in anonymous namespace, remove "static"
> Also pattern parameter comes before extent as |extent| is output parameter.

Done.

[lazyboy, 2017-06-30 21:24:59]

lgtm

[nrpeter, 2017-06-30 21:27:58]

The CQ bit was checked by nrpeter@google.com

[nrpeter, 2017-06-30 21:27:59]

The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2957233002/#ps20001
(title: "Fix nits")

[commit-bot: I haz the power, 2017-06-30 21:28:13]

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-06-30 23:24:07]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498858078673490,
"parent_rev": "80276e5af7882c5c239c8d0e60053813e7a1f826", "commit_rev":
"605f1df1854d4bc8868bfbd3bc4945c8d200ece0"}

[commit-bot: I haz the power, 2017-06-30 23:24:20]

Description was changed from

==========
To use URLPattern with an effective TLD wildcard, a second parameter must be
passed to the constructor specifically allowing this.

The IPC message parsing for URLPattern should always accept an effective TLD
wildcard. This is since the check would have been done on the initial parse to
populate a URLPattern.

This adds a unit test of the runtime blocked hosts and runtime allowed hosts in
the extension messages IPC. This also adds a browser test for an effective TLD
wildcard.

BUG=737315
==========

to

==========
To use URLPattern with an effective TLD wildcard, a second parameter must be
passed to the constructor specifically allowing this.

The IPC message parsing for URLPattern should always accept an effective TLD
wildcard. This is since the check would have been done on the initial parse to
populate a URLPattern.

This adds a unit test of the runtime blocked hosts and runtime allowed hosts in
the extension messages IPC. This also adds a browser test for an effective TLD
wildcard.

BUG=737315

Review-Url: https://codereview.chromium.org/2957233002
Cr-Commit-Position: refs/heads/master@{#483860}
Committed:
https://chromium.googlesource.com/chromium/src/+/605f1df1854d4bc8868bfbd3bc49...
==========

[commit-bot: I haz the power, 2017-06-30 23:24:22]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/605f1df1854d4bc8868bfbd3bc49...