predictors: Don't record URLs with a port.

predictors: Don't record URLs with a port.

BUG=631966
Review-Url: https://codereview.chromium.org/2749583003
Cr-Commit-Position: refs/heads/master@{#457406}
Committed: https://chromium.googlesource.com/chromium/src/+/0b6891ca2968c288af67584dd2ce758ef12fd699

[Benoit L, 2017-03-14 16:06:25]

lizeb@chromium.org changed reviewers:
+ alexilin@chromium.org

[Benoit L, 2017-03-14 16:07:13]

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

[commit-bot: I haz the power, 2017-03-14 16:07:56]

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-03-14 16:50:08]

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

[commit-bot: I haz the power, 2017-03-14 16:50:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Benoit L, 2017-03-15 13:39:41]

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

[commit-bot: I haz the power, 2017-03-15 13:39:56]

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-03-15 14:31:21]

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

[commit-bot: I haz the power, 2017-03-15 14:31:22]

Dry run: This issue passed the CQ dry run.

[Benoit L, 2017-03-15 16:06:59]

Now with 100% less failing tests.
PTAL, thanks.

[alexilin, 2017-03-15 17:56:26]

https://codereview.chromium.org/2749583003/diff/40001/chrome/browser/predicto...
File chrome/browser/predictors/resource_prefetch_predictor.cc (right):

https://codereview.chromium.org/2749583003/diff/40001/chrome/browser/predicto...
chrome/browser/predictors/resource_prefetch_predictor.cc:55: bool
g_allow_port_in_urls = false;
Why global flag and static setter instead of member field and method?
Having non const global is additional mental load for me.

[alexilin, 2017-03-15 18:02:54]

Discussed offline: static methods (IsHandled*) need this flag. We don't want to
make them class methods because that way it'd be harder to test them.

lgtm, thanks!

[Benoit L, 2017-03-16 09:13:58]

The CQ bit was checked by lizeb@chromium.org

[commit-bot: I haz the power, 2017-03-16 09:14:18]

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-03-16 09:14:19]

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

[commit-bot: I haz the power, 2017-03-16 09:14:20]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[Benoit L, 2017-03-16 09:17:35]

lizeb@chromium.org changed reviewers:
+ droger@chromium.org

[droger, 2017-03-16 12:02:27]

lgtm, mostly rubberstamping

https://codereview.chromium.org/2749583003/diff/40001/chrome/browser/predicto...
File chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
(right):

https://codereview.chromium.org/2749583003/diff/40001/chrome/browser/predicto...
chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc:340:
ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(false);
Nit, optional: Could we use ScopedClosureRunner to automatically reset this at
destruction?

[Benoit L, 2017-03-16 12:20:28]

The CQ bit was checked by lizeb@chromium.org

[commit-bot: I haz the power, 2017-03-16 12:20:44]

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-03-16 12:25:58]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489666828219920,
"parent_rev": "58f641e2daf00d18ce899adc30fa92abaaa26e5b", "commit_rev":
"0b6891ca2968c288af67584dd2ce758ef12fd699"}

[commit-bot: I haz the power, 2017-03-16 12:26:42]

Description was changed from

==========
predictors: Don't record URLs with a port.

BUG=631966
==========

to

==========
predictors: Don't record URLs with a port.

BUG=631966

Review-Url: https://codereview.chromium.org/2749583003
Cr-Commit-Position: refs/heads/master@{#457406}
Committed:
https://chromium.googlesource.com/chromium/src/+/0b6891ca2968c288af67584dd2ce...
==========

[commit-bot: I haz the power, 2017-03-16 12:26:44]

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