[2 of 4] Add some test strings to registry_controlled_domain_unittest.cc

Add some test strings to registry_controlled_domain_unittest.cc

These strings will be interesting values if we reverse the lookup order,
which occurs in the next patch set of this series.

BUG=678334
Review-Url: https://codereview.chromium.org/2648213003
Cr-Commit-Position: refs/heads/master@{#451165}
Committed: https://chromium.googlesource.com/chromium/src/+/6b6710cc340138ce79eba7a6e8d371eeacf58f56

[ncarter (slow), 2017-01-24 15:28:30]

Description was changed from

==========
Add some test strings to registry_controlled_domain_unittest.cc

These strings will be interesting values if we reverse the lookup order,
which I intend to do in a follow-on CL.

BUG=678334
==========

to

==========
Add some test strings to registry_controlled_domain_unittest.cc

These strings will be interesting values if we reverse the lookup order,
occurs in the next patch set of this series.

BUG=678334
==========

[Ryan Sleevi, 2017-01-25 22:05:23]

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

[Ryan Sleevi, 2017-01-25 22:05:24]

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
File net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc
(right):

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:186:
EXCLUDE_UNKNOWN_REGISTRIES));  // 1
What are you denoting with the trailing comments? That's not as clear - and
digging through the blame doesn't really shed light on it.

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:189:
EXPECT_EQ(0U, GetRegistryLengthFromURL("http://.baz.bar.jp",
This is an invalid URL - so what's actually being tested here?

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:191:
EXPECT_EQ(0U, GetRegistryLengthFromURL("http://..baz.bar.jp",
Ditto

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:193:
EXPECT_EQ(11U, GetRegistryLengthFromURL("http://foo..baz.bar.jp",
Ditto

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:211:
EXPECT_EQ(6U, GetRegistryLengthFromURL("http://baz..pref.bar.jp",
Also invalid

[ncarter (slow), 2017-02-16 00:23:22]

Description was changed from

==========
Add some test strings to registry_controlled_domain_unittest.cc

These strings will be interesting values if we reverse the lookup order,
occurs in the next patch set of this series.

BUG=678334
==========

to

==========
Add some test strings to registry_controlled_domain_unittest.cc

These strings will be interesting values if we reverse the lookup order,
which occurs in the next patch set of this series.

BUG=678334
==========

[ncarter (slow), 2017-02-16 20:46:27]

PTAL

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
File net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc
(right):

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:186:
EXCLUDE_UNKNOWN_REGISTRIES));  // 1
On 2017/01/25 22:05:24, Ryan Sleevi wrote:
> What are you denoting with the trailing comments? That's not as clear - and
> digging through the blame doesn't really shed light on it.

These numbers seemed to correspond to which rules (in the gperf file) are
potential matches for input strings. They are 1-indexed starting from the first
rule.

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:189:
EXPECT_EQ(0U, GetRegistryLengthFromURL("http://.baz.bar.jp",
On 2017/01/25 22:05:24, Ryan Sleevi wrote:
> This is an invalid URL - so what's actually being tested here?

What's being tested is the code in GetRegistryLengthImpl that deals with with
edge cases around multiple leading and trailing dots. And, an empty label in the
middle of a hostname is a corner case that the new implementation will need to
handle.

To my surprise, GURL canonicalization actually doesn't handle any of these cases
-- the following unittest passes:

TEST(RCD, EmptyLabelsNotCanonicalizedByGURL) {
  EXPECT_EQ(".baz.bar.jp", GURL("http://.baz.bar.jp").host());
  EXPECT_EQ("..baz.bar.jp", GURL("http://..baz.bar.jp").host());
  EXPECT_EQ("baz..bar.jp", GURL("http://baz..bar.jp").host());
  EXPECT_EQ("baz.bar.jp..", GURL("http://baz.bar.jp..").host());
  EXPECT_EQ("baz.bar.jp.", GURL("http://baz.bar.jp.").host());
  EXPECT_EQ("...a..b..c..d...", GURL("http://...a..b..c..d...").host());

  EXPECT_TRUE(GURL("http://.baz.bar.jp").is_valid());
  EXPECT_TRUE(GURL("http://..baz.bar.jp").is_valid());
  EXPECT_TRUE(GURL("http://baz..bar.jp").is_valid());
  EXPECT_TRUE(GURL("http://baz.bar.jp..").is_valid());
  EXPECT_TRUE(GURL("http://baz.bar.jp.").is_valid());
  EXPECT_TRUE(GURL("http://...a..b..c..d...").is_valid());
}

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:191:
EXPECT_EQ(0U, GetRegistryLengthFromURL("http://..baz.bar.jp",
On 2017/01/25 22:05:24, Ryan Sleevi wrote:
> Ditto

See above.

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:193:
EXPECT_EQ(11U, GetRegistryLengthFromURL("http://foo..baz.bar.jp",
On 2017/01/25 22:05:24, Ryan Sleevi wrote:
> Ditto

See above.

https://codereview.chromium.org/2648213003/diff/20001/net/base/registry_contr...
net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc:211:
EXPECT_EQ(6U, GetRegistryLengthFromURL("http://baz..pref.bar.jp",
On 2017/01/25 22:05:24, Ryan Sleevi wrote:
> Also invalid

See above.

[Ryan Sleevi, 2017-02-16 20:57:22]

lgtm

[ncarter (slow), 2017-02-16 23:41:43]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2017-02-16 23:43:07]

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-17 01:12:56]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487288503095550,
"parent_rev": "7c72af3f79ee04521ea3aadb0a8448bb0644ca0f", "commit_rev":
"6b6710cc340138ce79eba7a6e8d371eeacf58f56"}

[commit-bot: I haz the power, 2017-02-17 01:14:16]

Description was changed from

==========
Add some test strings to registry_controlled_domain_unittest.cc

These strings will be interesting values if we reverse the lookup order,
which occurs in the next patch set of this series.

BUG=678334
==========

to

==========
Add some test strings to registry_controlled_domain_unittest.cc

These strings will be interesting values if we reverse the lookup order,
which occurs in the next patch set of this series.

BUG=678334

Review-Url: https://codereview.chromium.org/2648213003
Cr-Commit-Position: refs/heads/master@{#451165}
Committed:
https://chromium.googlesource.com/chromium/src/+/6b6710cc340138ce79eba7a6e8d3...
==========

[commit-bot: I haz the power, 2017-02-17 01:14:18]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/6b6710cc340138ce79eba7a6e8d3...