Add Origin::CreateFromNormalizedTuple and call from WebSecurityOrigin

Add Origin::CreateFromNormalizedTuple and call from WebSecurityOrigin

BUG=

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

[Charlie Harrison, 2016-10-06 13:42:21]

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

[commit-bot: I haz the power, 2016-10-06 13:42:35]

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, 2016-10-06 15:32:39]

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

[commit-bot: I haz the power, 2016-10-06 15:32:40]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2016-10-06 21:03:48]

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

[commit-bot: I haz the power, 2016-10-06 21:04:33]

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, 2016-10-06 21:17:05]

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

[commit-bot: I haz the power, 2016-10-06 21:17:06]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Charlie Harrison, 2016-10-07 00:48:57]

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

[commit-bot: I haz the power, 2016-10-07 00:49:17]

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, 2016-10-07 01:11:18]

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

[commit-bot: I haz the power, 2016-10-07 01:11:19]

Dry run: Exceeded global retry quota

[Charlie Harrison, 2016-10-07 12:57:56]

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

[commit-bot: I haz the power, 2016-10-07 12:58:06]

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, 2016-10-07 13:32:35]

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

[commit-bot: I haz the power, 2016-10-07 13:32:36]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Charlie Harrison, 2016-10-07 14:12:24]

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

[Charlie Harrison, 2016-10-07 14:12:24]

Hey Mike, here is a first pass at the fast-path WebSecurityOrigin->Origin
conversion.

Unfortunately to keep logic simple, we still need to perform a few checks in
IsValidInput because:
 - GURLs can have schemes that "require" ports with 0 port (ip addresses, see
previous test failures in PS 4)
 - GURLs can have invalid schemes (e.g. blob)

So, I kept the scheme check and port checks, which I don't think is too big a
deal.

[Charlie Harrison, 2016-10-07 18:50:03]

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

[commit-bot: I haz the power, 2016-10-07 18:50:39]

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, 2016-10-07 21:01:00]

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

[commit-bot: I haz the power, 2016-10-07 21:01:01]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-10-11 08:38:33]

LGTM! Modulo the gopher bit, which I don't think we need?

https://codereview.chromium.org/2391383003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/weborigin/KnownPorts.cpp (right):

https://codereview.chromium.org/2391383003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/weborigin/KnownPorts.cpp:51: return protocol
== "gopher";
This seems unrelated? Moreover, we don't actually support gopher anywhere, so...

https://codereview.chromium.org/2391383003/diff/80001/url/origin.cc
File url/origin.cc (right):

https://codereview.chromium.org/2391383003/diff/80001/url/origin.cc#newcode64
url/origin.cc:64: return Origin(scheme, host, port, true /* is_canonicalized
*/);
Nit: Did you consider making this an enum rather than adding a comment next to
an opaque bool? I think that would end up being more readable.

[Charlie Harrison, 2016-10-11 11:58:10]

Thanks!

https://codereview.chromium.org/2391383003/diff/80001/content/child/blink_pla...
File content/child/blink_platform_impl_unittest.cc (right):

https://codereview.chromium.org/2391383003/diff/80001/content/child/blink_pla...
content/child/blink_platform_impl_unittest.cc:67: {"gopher://example.com/",
"gopher", "example.com", 70},
FYI: The gopher changes are needed for this test to pass. I included this test
because it's in origin_unittest. I'm happy to remove this though.

[Charlie Harrison, 2016-10-11 16:27:59]

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

[commit-bot: I haz the power, 2016-10-11 16:28:13]

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

[Charlie Harrison, 2016-10-11 16:29:54]

csharrison@chromium.org changed reviewers:
+ nick@chromium.org

[Charlie Harrison, 2016-10-11 16:29:55]

+nick for //content OWNERS.

https://codereview.chromium.org/2391383003/diff/80001/url/origin.cc
File url/origin.cc (right):

https://codereview.chromium.org/2391383003/diff/80001/url/origin.cc#newcode64
url/origin.cc:64: return Origin(scheme, host, port, true /* is_canonicalized
*/);
On 2016/10/11 08:38:33, Mike West wrote:
> Nit: Did you consider making this an enum rather than adding a comment next to
> an opaque bool? I think that would end up being more readable.

Done.

[commit-bot: I haz the power, 2016-10-11 18:09:37]

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

[commit-bot: I haz the power, 2016-10-11 18:09:39]

Dry run: This issue passed the CQ dry run.

[ncarter (slow), 2016-10-11 18:21:52]

https://codereview.chromium.org/2391383003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/weborigin/KnownPorts.cpp (right):

https://codereview.chromium.org/2391383003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/weborigin/KnownPorts.cpp:51: return protocol
== "gopher";
On 2016/10/11 08:38:33, Mike West wrote:
> This seems unrelated? Moreover, we don't actually support gopher anywhere,
so...

Agree, we don't handle gopher, and won't ever. I've added a few tests that
exercise it, because it's a weird case, an unhandled protocol that GURL knows
about, but mostly out of whimsy. If these tests are causing trouble, it's okay
to delete the tests.

[Charlie Harrison, 2016-10-11 18:24:33]

On 2016/10/11 18:21:52, ncarter wrote:
>
https://codereview.chromium.org/2391383003/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/platform/weborigin/KnownPorts.cpp (right):
> 
>
https://codereview.chromium.org/2391383003/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/platform/weborigin/KnownPorts.cpp:51: return
protocol
> == "gopher";
> On 2016/10/11 08:38:33, Mike West wrote:
> > This seems unrelated? Moreover, we don't actually support gopher anywhere,
> so...
> 
> Agree, we don't handle gopher, and won't ever. I've added a few tests that
> exercise it, because it's a weird case, an unhandled protocol that GURL knows
> about, but mostly out of whimsy. If these tests are causing trouble, it's okay
> to delete the tests.

There's just one test in origin_unittest that handles a gopher GURL -> Origin
transformation, but it's not causing any issues. I've removed the gopher stuff
in the renderer.

[ncarter (slow), 2016-10-11 18:26:57]

content lgtm

https://codereview.chromium.org/2391383003/diff/100001/content/child/blink_pl...
File content/child/blink_platform_impl_unittest.cc (right):

https://codereview.chromium.org/2391383003/diff/100001/content/child/blink_pl...
content/child/blink_platform_impl_unittest.cc:85:
{"blob:http://u:p@example.com/guid-goes-here", "http", "example.com", 80},
Should we add a foo.appspot.com example (or co.uk, or k12.va.us, or something
else where it's necessary to preserve more than one dot) to this list?

[Charlie Harrison, 2016-10-11 23:10:22]

Thanks

https://codereview.chromium.org/2391383003/diff/100001/content/child/blink_pl...
File content/child/blink_platform_impl_unittest.cc (right):

https://codereview.chromium.org/2391383003/diff/100001/content/child/blink_pl...
content/child/blink_platform_impl_unittest.cc:85:
{"blob:http://u:p@example.com/guid-goes-here", "http", "example.com", 80},
On 2016/10/11 18:26:57, ncarter wrote:
> Should we add a http://foo.appspot.com example (or co.uk, or k12.va.us, or
something
> else where it's necessary to preserve more than one dot) to this list?

Done.

[Charlie Harrison, 2016-10-11 23:10:31]

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

[commit-bot: I haz the power, 2016-10-11 23:11:05]

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

[Charlie Harrison, 2016-10-11 23:16:57]

The CQ bit was unchecked by csharrison@chromium.org

[Charlie Harrison, 2016-10-11 23:16:58]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-10-11 23:16:59]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org,
nick@chromium.org
Link to the patchset: https://codereview.chromium.org/2391383003/#ps120001
(title: "ncarter review")

[commit-bot: I haz the power, 2016-10-11 23:17:38]

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

[ncarter (slow), 2016-10-11 23:29:46]

https://codereview.chromium.org/2391383003/diff/120001/content/child/blink_pl...
File content/child/blink_platform_impl_unittest.cc (right):

https://codereview.chromium.org/2391383003/diff/120001/content/child/blink_pl...
content/child/blink_platform_impl_unittest.cc:86:
{"blob:https://example.co.uk/guid-goes-here", "https", "example.co.uk", 443},
80 cols.

[Charlie Harrison, 2016-10-11 23:32:02]

The CQ bit was unchecked by csharrison@chromium.org

[Charlie Harrison, 2016-10-11 23:35:09]

Good catch :)

[Charlie Harrison, 2016-10-11 23:35:41]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-10-11 23:35:41]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org,
mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2391383003/#ps140001
(title: "git cl format")

[commit-bot: I haz the power, 2016-10-11 23:36:01]

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, 2016-10-12 01:43:20]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-10-12 01:48:25]

Description was changed from

==========
Add Origin::CreateFromNormalizedTuple and call from WebSecurityOrigin

BUG=
==========

to

==========
Add Origin::CreateFromNormalizedTuple and call from WebSecurityOrigin

BUG=

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

[commit-bot: I haz the power, 2016-10-12 01:48:26]

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