Remove NETWORK_PREDICTION_UNSET.

Remove NETWORK_PREDICTION_UNSET.

Since PrefService provides a consistent and convenient way to check if a user
value has ever been saved for a preference, there is no need for
NETWORK_PREDICTION_UNSET in enum chrome_browser_net::NetworkPredictionOptions.
The default value is NETWORK_PREDICTION_WIFI_ONLY, this is reported by
PrefService if there is no user setting, policy, etc.

Note that since MigrateNetworkPredictionUserPrefs was introduced in
https://codereview.chromium.org/421653006/diff/160002/chrome/browser/net/prediction_options.cc,
there is no need to check the old preference NetworkPredictionEnabled in
CanPrefetchAndPrerender* or CanPreresolveAndPreconnect*.

Also note that Chromium at no point did save NETWORK_PREDICTION_UNSET to disk,
it only used to be provided as a default option.  This guarantees that this CL
does not introduce functional change.

TODO:
 * Remove network_prediction_enabled from profile_io_data.
 * Remove PrerenderCondition classes.

As this CL will also fix UI bug https://crbug.com/404794, I prefer to land it
first, and deal with the other two clean-up items on a separate CL.

BUG=406583
Committed: https://crrev.com/c1a560b6e1f7548bc952467d01e3bed610a4e280
Cr-Commit-Position: refs/heads/master@{#292613}

[Bence, 2014-08-27 15:25:33]

bnc@chromium.org changed reviewers:
+ gavinp@chromium.org, mattm@chromium.org

[Bence, 2014-08-27 15:25:33]

gavinp: Please review changes in prefetch.
mattm: Please review changes in net, prerender.

[Bence, 2014-08-27 18:45:37]

Trybots are happy now, PTAL.

[mattm, 2014-08-27 19:30:10]

I might not be the best person to review prerender stuff. Did you mean mmenke?

[Bence, 2014-08-27 19:36:25]

On 2014/08/27 19:30:10, mattm wrote:
> I might not be the best person to review prerender stuff. Did you mean mmenke?

I am sorry about that, yes I did.

[Bence, 2014-08-27 19:36:34]

bnc@chromium.org changed reviewers:
+ mmenke@chromium.org
- mattm@chromium.org

[gavinp, 2014-08-28 13:34:11]

prefetch.cc LGTM.

The rest LGTM as well, although I think the thread checking comment is of the
two, the one most worth looking at.

https://codereview.chromium.org/516443002/diff/20001/chrome/browser/net/predi...
File chrome/browser/net/prediction_options.cc (right):

https://codereview.chromium.org/516443002/diff/20001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:19: bool CanPrefetchAndPrerender(int
network_prediction_options) {
Hmm, should the type be chrome_browser_net::NetworkPredictionOptions rather than
an int? The disadvantage is that there's a need for a static cast on the result
of GetValue() below, which I guess is a bit weird.

mmenke, wdyt?

https://codereview.chromium.org/516443002/diff/20001/chrome/browser/prerender...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/516443002/diff/20001/chrome/browser/prerender...
chrome/browser/prerender/prerender_manager.cc:1862:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
I think this change doesn't belong here.

I do think a separate CL that removes base::NonThreadSafe from
PrerenderManager's inheritance, and puts really explicit threading DCHECKs
everywhere that CalledOnValidThread() is checked is probably a really good idea
though... What do you think?

[Bence, 2014-08-28 14:11:55]

mmenke: PTAL.

https://codereview.chromium.org/516443002/diff/20001/chrome/browser/prerender...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/516443002/diff/20001/chrome/browser/prerender...
chrome/browser/prerender/prerender_manager.cc:1862:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
You are right.  It is stated in prerender_manager.h that all methods must be
called on the UI thread, I just thought we might want to enforce it.

Done.

[mmenke, 2014-08-28 14:34:49]

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
File chrome/browser/net/prediction_options.cc (right):

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:40: namespace chrome_browser_net {
Suggest moving this up to include the anonymous namespace, to make code a little
cleaner up there.

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:46:
chrome_browser_net::NETWORK_PREDICTION_DEFAULT,
nit:  chrome_browser_net not needed

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/prerender...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/prerender...
chrome/browser/prerender/prerender_manager.cc:1864: if (!enabled_)
You're going to get rid of this, and the logic in net_pref_observer to modify it
in another CL?

[Bence, 2014-08-28 14:58:20]

mmenke: PTAL.

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
File chrome/browser/net/prediction_options.cc (right):

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:40: namespace chrome_browser_net {
CanPrefetchAndPrerender and CanPreresolveAndPreconnect are not intended to be
called from outside this file, I prefer not to litter the namespace with them. 
I could add "using chrome_browser_net::NETWORK_PREDICTION_ALWAYS" and such if
you prefer.

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:46:
chrome_browser_net::NETWORK_PREDICTION_DEFAULT,
On 2014/08/28 14:34:48, mmenke wrote:
> nit:  chrome_browser_net not needed

Done.

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/prerender...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/prerender...
chrome/browser/prerender/prerender_manager.cc:1864: if (!enabled_)
Yes, I plan to do that soon.

[mmenke, 2014-08-28 15:12:16]

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
File chrome/browser/net/prediction_options.cc (right):

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:40: namespace chrome_browser_net {
On 2014/08/28 14:58:20, Bence wrote:
> CanPrefetchAndPrerender and CanPreresolveAndPreconnect are not intended to be
> called from outside this file, I prefer not to litter the namespace with them.

> I could add "using chrome_browser_net::NETWORK_PREDICTION_ALWAYS" and such if
> you prefer.

You can net an anonymous namespace inside chrome_browser_net, and you won't be
cluttering anything.  It's done extensively in Chrome and net.

[Bence, 2014-08-28 15:46:17]

PTAL.

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
File chrome/browser/net/prediction_options.cc (right):

https://codereview.chromium.org/516443002/diff/40001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:40: namespace chrome_browser_net {
On 2014/08/28 15:12:16, mmenke wrote:
> On 2014/08/28 14:58:20, Bence wrote:
> > CanPrefetchAndPrerender and CanPreresolveAndPreconnect are not intended to
be
> > called from outside this file, I prefer not to litter the namespace with
them.
> 
> > I could add "using chrome_browser_net::NETWORK_PREDICTION_ALWAYS" and such
if
> > you prefer.
> 
> You can net an anonymous namespace inside chrome_browser_net, and you won't be
> cluttering anything.  It's done extensively in Chrome and net.

Oh, I didn't realize one can do that, sorry.

Done.

[mmenke, 2014-08-28 17:03:38]

LGTM

https://codereview.chromium.org/516443002/diff/80001/chrome/browser/net/predi...
File chrome/browser/net/prediction_options.cc (right):

https://codereview.chromium.org/516443002/diff/80001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:27: default:
Optional:  Maybe DCHECK_EQ(NETWORK_PREDICTION_WIFI_ONLY,
network_prediction_options);

[Bence, 2014-08-29 12:03:21]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2014-08-29 12:04:13]

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

[commit-bot: I haz the power, 2014-08-29 12:54:29]

Committed patchset #6 (id:100001) as 01300a78c7e9c1c35144c6fb762aa86862a250d3

[Bence, 2014-08-29 13:22:43]

mmenke:  Thanks for your help.

https://codereview.chromium.org/516443002/diff/80001/chrome/browser/net/predi...
File chrome/browser/net/prediction_options.cc (right):

https://codereview.chromium.org/516443002/diff/80001/chrome/browser/net/predi...
chrome/browser/net/prediction_options.cc:27: default:
On 2014/08/28 17:03:37, mmenke wrote:
> Optional:  Maybe DCHECK_EQ(NETWORK_PREDICTION_WIFI_ONLY,
> network_prediction_options);

Done.

[commit-bot: I haz the power, 2014-09-10 03:07:54]

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