Merge logic for SameDomainOrHost for GURLs and Origins

Merge logic for SameDomainOrHost for GURLs and Origins

Previously, we used to just call:
SameDomainOrHost(origin1.GetURL(), origin2.GetURL())

This isn't necessary, as we have all the information in the Origin itself.
Additionally, GetURL() isn't free. We do a bunch of string copies,
allocations, and some non-cheap policy decisions.

This patch moves GURL::HostIsIPAddress logic to url_util, and reuses
that code for both GURLs and Origins.

BUG=664174, 348655
Committed: https://crrev.com/475851dad085b226d5459363e4daf152eb4cfc77
Cr-Commit-Position: refs/heads/master@{#439287}

[Charlie Harrison, 2016-12-08 21:11:35]

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

[commit-bot: I haz the power, 2016-12-08 21:12:11]

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-12-08 22:43:19]

csharrison@chromium.org changed reviewers:
+ brettw@chromium.org, pkasting@chromium.org

[Charlie Harrison, 2016-12-08 22:43:19]

Brett, Peter: PTAL at this patch? In my opinion it makes things conceptually
simpler, as it makes total sense for SameDomainOrHost to operate on hosts (also,
Origin->GURL transforms are strange and its nice to remove them).

[commit-bot: I haz the power, 2016-12-08 23:18:27]

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

[commit-bot: I haz the power, 2016-12-08 23:18:27]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2016-12-09 00:17:59]

LGTM

https://codereview.chromium.org/2562813003/diff/1/net/base/registry_controlle...
File net/base/registry_controlled_domains/registry_controlled_domain.cc (right):

https://codereview.chromium.org/2562813003/diff/1/net/base/registry_controlle...
net/base/registry_controlled_domains/registry_controlled_domain.cc:316: if
(!host1.length() || !host2.length())
Nit: Prefer .empty() to !.length()

[Charlie Harrison, 2016-12-09 01:14:01]

Thanks!

https://codereview.chromium.org/2562813003/diff/1/net/base/registry_controlle...
File net/base/registry_controlled_domains/registry_controlled_domain.cc (right):

https://codereview.chromium.org/2562813003/diff/1/net/base/registry_controlle...
net/base/registry_controlled_domains/registry_controlled_domain.cc:316: if
(!host1.length() || !host2.length())
On 2016/12/09 00:17:59, Peter Kasting wrote:
> Nit: Prefer .empty() to !.length()

Done.

[Charlie Harrison, 2016-12-14 23:22:08]

brettw: friendly ping

[brettw, 2016-12-15 18:45:32]

lgtm

https://codereview.chromium.org/2562813003/diff/20001/url/url_util.h
File url/url_util.h (right):

https://codereview.chromium.org/2562813003/diff/20001/url/url_util.h#newcode149
url/url_util.h:149: // Hosts
-----------------------------------------------------------------------
I'm not sure these section headers are helping much if there's only one thing in
each. I think this and "DomainIs" could be classified as "Hosts" so maybe just
rename that header and have both functions udner it?

[Charlie Harrison, 2016-12-16 22:48:32]

Thanks!

https://codereview.chromium.org/2562813003/diff/20001/url/url_util.h
File url/url_util.h (right):

https://codereview.chromium.org/2562813003/diff/20001/url/url_util.h#newcode149
url/url_util.h:149: // Hosts
-----------------------------------------------------------------------
On 2016/12/15 18:45:32, brettw (ping after 24h) wrote:
> I'm not sure these section headers are helping much if there's only one thing
in
> each. I think this and "DomainIs" could be classified as "Hosts" so maybe just
> rename that header and have both functions udner it?

Done.

[Charlie Harrison, 2016-12-16 22:48:41]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-12-16 22:48:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2562813003/#ps40001
(title: "brettw review")

[commit-bot: I haz the power, 2016-12-16 22:49:20]

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-12-17 02:19:46]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481928521780050,
"parent_rev": "29a4dcc0699a051115532a22e2d1ca6dcd6fc03f", "commit_rev":
"9a07bbe466c3cd333f8a23c77fdd149422f241a8"}

[commit-bot: I haz the power, 2016-12-17 02:20:48]

Description was changed from

==========
Merge logic for SameDomainOrHost for GURLs and Origins

Previously, we used to just call:
SameDomainOrHost(origin1.GetURL(), origin2.GetURL())

This isn't necessary, as we have all the information in the Origin itself.
Additionally, GetURL() isn't free. We do a bunch of string copies,
allocations, and some non-cheap policy decisions.

This patch moves GURL::HostIsIPAddress logic to url_util, and reuses
that code for both GURLs and Origins.

BUG=664174,348655
==========

to

==========
Merge logic for SameDomainOrHost for GURLs and Origins

Previously, we used to just call:
SameDomainOrHost(origin1.GetURL(), origin2.GetURL())

This isn't necessary, as we have all the information in the Origin itself.
Additionally, GetURL() isn't free. We do a bunch of string copies,
allocations, and some non-cheap policy decisions.

This patch moves GURL::HostIsIPAddress logic to url_util, and reuses
that code for both GURLs and Origins.

BUG=664174,348655

Review-Url: https://codereview.chromium.org/2562813003
==========

[commit-bot: I haz the power, 2016-12-17 02:20:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-17 02:26:21]

Description was changed from

==========
Merge logic for SameDomainOrHost for GURLs and Origins

Previously, we used to just call:
SameDomainOrHost(origin1.GetURL(), origin2.GetURL())

This isn't necessary, as we have all the information in the Origin itself.
Additionally, GetURL() isn't free. We do a bunch of string copies,
allocations, and some non-cheap policy decisions.

This patch moves GURL::HostIsIPAddress logic to url_util, and reuses
that code for both GURLs and Origins.

BUG=664174,348655

Review-Url: https://codereview.chromium.org/2562813003
==========

to

==========
Merge logic for SameDomainOrHost for GURLs and Origins

Previously, we used to just call:
SameDomainOrHost(origin1.GetURL(), origin2.GetURL())

This isn't necessary, as we have all the information in the Origin itself.
Additionally, GetURL() isn't free. We do a bunch of string copies,
allocations, and some non-cheap policy decisions.

This patch moves GURL::HostIsIPAddress logic to url_util, and reuses
that code for both GURLs and Origins.

BUG=664174,348655

Committed: https://crrev.com/475851dad085b226d5459363e4daf152eb4cfc77
Cr-Commit-Position: refs/heads/master@{#439287}
==========

[commit-bot: I haz the power, 2016-12-17 02:26:22]

Patchset 3 (id:??) landed as
https://crrev.com/475851dad085b226d5459363e4daf152eb4cfc77
Cr-Commit-Position: refs/heads/master@{#439287}