Add prefetch behavior test.

Add prefetch behavior test.

Add test to verify prefetch behavior under different user preference
settings and network connections.

BUG=334602
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290295

[Bence, 2014-08-11 20:16:39]

Gavin: please review.

Josh, Matt: this is the test we have been talking about, in person or at
https://codereview.chromium.org/443413002/#msg6.  Also, this would have cought
the bug that Josh fixed at https://codereview.chromium.org/419833003/.  Please
take a look and let me know if this is what you had in mind.

[jkarlin, 2014-08-13 11:18:28]

Thanks for tackling this Bence.

There is some confusion about the meaning of the field trial being
enabled/disabled.

Also, I'm confused about how NotifyObserversOfConnectionTypeChangeForTests
changes the result of the prediction function since that relies on
GetConnectionType(), not notifications.  See comments below.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
File chrome/browser/prefetch/prefetch_browsertest.cc (right):

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:26: using
chrome_browser_net::NetworkPredictionOptions;
Swap lines 25 and 26 for alphabetical?  Not sure if necessary but might as well.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:56:
NetworkChangeNotifier::NotifyObserversOfConnectionTypeChangeForTests(type);
NetworkChangeNotifier::NotifyObserversOfConnectionTypeChangeForTests(type) sends
notifications of type change but does not change the result of
NetworkChangeNotifier::GetConnectionType(), which the prediction logic relies
on.  

Given that, I have no idea how the tests below are passing.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:76: class
PrefetchBrowserTestPredictionExpDisabled
A few weeks ago the meaning of the Finch trial changed with prefetch becoming
default on.

Now, by default (experiment off) prefetch should work (according to predictive
preference).  When the experiment is on, prefetch should never work.  Please
update the names, comments, and tests to reflect that.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:140: // Experiment is enabled. 
Prefetch should allowed depending on preference.
s/should allowed/should be allowed/

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:146:
SetConnectionType(NetworkChangeNotifier::CONNECTION_4G);
I don't understand how this is working given my comment above.  Can you try
running the CONNECTION_4G experiment first and then WIFI and see if that order
also works?

[Bence, 2014-08-13 14:31:45]

jkarlin: PTAL.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
File chrome/browser/prefetch/prefetch_browsertest.cc (right):

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:26: using
chrome_browser_net::NetworkPredictionOptions;
On 2014/08/13 11:18:28, jkarlin wrote:
> Swap lines 25 and 26 for alphabetical?  Not sure if necessary but might as
well.

Done.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:56:
NetworkChangeNotifier::NotifyObserversOfConnectionTypeChangeForTests(type);
On 2014/08/13 11:18:27, jkarlin wrote:
> NetworkChangeNotifier::NotifyObserversOfConnectionTypeChangeForTests(type)
sends
> notifications of type change but does not change the result of
> NetworkChangeNotifier::GetConnectionType(), which the prediction logic relies
> on.  
> 
> Given that, I have no idea how the tests below are passing.

They were not supposed to.  Turns out overriding SetUp() resulted in
EXPECT_TRUE(false) not failing. (!?)

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:76: class
PrefetchBrowserTestPredictionExpDisabled
On 2014/08/13 11:18:28, jkarlin wrote:
> A few weeks ago the meaning of the Finch trial changed with prefetch becoming
> default on.
> 
> Now, by default (experiment off) prefetch should work (according to predictive
> preference).  When the experiment is on, prefetch should never work.  Please
> update the names, comments, and tests to reflect that.

Done.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:140: // Experiment is enabled. 
Prefetch should allowed depending on preference.
On 2014/08/13 11:18:28, jkarlin wrote:
> s/should allowed/should be allowed/

Done.

https://codereview.chromium.org/459933002/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_browsertest.cc:146:
SetConnectionType(NetworkChangeNotifier::CONNECTION_4G);
On 2014/08/13 11:18:28, jkarlin wrote:
> I don't understand how this is working given my comment above.  Can you try
> running the CONNECTION_4G experiment first and then WIFI and see if that order
> also works?

It wasn't working this way, but I found another way to fake connection types.

[jkarlin, 2014-08-13 14:56:08]

This is great. Though gavinp will have to approve it.

[gavinp, 2014-08-18 14:24:40]

lgtm (halfway rubber stamp based on jkarlin's good words, which I confirmed were
an lgtm by another name)

[Bence, 2014-08-18 14:26:19]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2014-08-18 14:27:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bnc@chromium.org/459933002/40001

[commit-bot: I haz the power, 2014-08-18 17:48:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-18 17:59:37]

Committed patchset #3 (40001) as 290295