Binds prefetching to the predictive networking privacy option.

Binds prefetching to the predictive networking privacy option.

Adds browser tests to check that both the privacy option and field
trial control prefetching.

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

[jkarlin, 2014-01-27 12:06:00]

PTAL

[tburkard, 2014-01-27 12:26:34]

lgtm

https://codereview.chromium.org/142473003/diff/1/chrome/browser/prefetch/pref...
File chrome/browser/prefetch/prefetch.h (right):

https://codereview.chromium.org/142473003/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch.h:14: bool
IsPrefetchEnabled(content::ResourceContext* resource_context);
This is intended to only run on the IO thread, correct?

Since it is accessing ProfileIOData?

I'd add a comment, and verify it at the beginning of the function (that it is
called on the right thread).

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

https://codereview.chromium.org/142473003/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_field_trial.cc:15: std::string experiment =
base::FieldTrialList::FindFullName("Prefetch");
I guess this does not have to be called on the UI thread, but can be called on
any thread. So this should be fine.

[jkarlin, 2014-01-27 13:48:59]

https://codereview.chromium.org/142473003/diff/1/chrome/browser/prefetch/pref...
File chrome/browser/prefetch/prefetch.h (right):

https://codereview.chromium.org/142473003/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch.h:14: bool
IsPrefetchEnabled(content::ResourceContext* resource_context);
On 2014/01/27 12:26:34, tburkard wrote:
> This is intended to only run on the IO thread, correct?
> 
> Since it is accessing ProfileIOData?
> 
> I'd add a comment, and verify it at the beginning of the function (that it is
> called on the right thread).

Done.

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

https://codereview.chromium.org/142473003/diff/1/chrome/browser/prefetch/pref...
chrome/browser/prefetch/prefetch_field_trial.cc:15: std::string experiment =
base::FieldTrialList::FindFullName("Prefetch");
On 2014/01/27 12:26:34, tburkard wrote:
> I guess this does not have to be called on the UI thread, but can be called on
> any thread. So this should be fine.

Looking at field_trial.h it says that it should only be called on the UI thread,
though I see locks and no thread checks in the implementation. Should the
comment be updated? Adding jar@

[jkarlin, 2014-01-27 13:50:55]

jar@: Note the use of a field trial in prefetch.cc on the IO thread.  Are field
trials thread safe?  They're used in IO code elsewhere but field_trial.h says
that all calls are from UI.  Should that comment be removed or is it actually
not thread safe?  Thanks!

[jkarlin, 2014-01-27 13:51:30]

On 2014/01/27 13:50:55, jkarlin wrote:
> jar@: Note the use of a field trial in prefetch.cc on the IO thread.  Are
field
> trials thread safe?  They're used in IO code elsewhere but field_trial.h says
> that all calls are from UI.  Should that comment be removed or is it actually
> not thread safe?  Thanks!

Sorry, look in prefetch_field_trial.cc not prefetch.cc

[jkarlin, 2014-01-27 13:52:24]

darin@chromium.org: Please review changes in profiles/ and render_host/

[darin (slow to review), 2014-01-27 21:51:26]

LGTM

[jkarlin, 2014-01-28 14:58:04]

Ping jar@, is it safe to call field trial code from IO thread like
prefetch_field_trial.cc does?

[jkarlin, 2014-01-29 16:03:06]

On 2014/01/28 14:58:04, jkarlin wrote:
> Ping jar@, is it safe to call field trial code from IO thread like
> prefetch_field_trial.cc does?

Note that the thread issue is not related to this CL, will commit regardless.

[commit-bot: I haz the power, 2014-01-29 16:57:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jkarlin@chromium.org/142473003/120001

[jar (doing other things), 2014-01-29 17:28:20]

On 2014/01/28 14:58:04, jkarlin wrote:
> Ping jar@, is it safe to call field trial code from IO thread like
> prefetch_field_trial.cc does?

Histograms, and field trial inquiries, may be called from any thread, at any
time.

The code has locking "when it is needed," and the histogram macros may use some
fancy lock-free code at times to enable use of efficient atomic statics, etc.

[jkarlin, 2014-01-29 17:35:01]

On 2014/01/29 17:28:20, jar wrote:
> On 2014/01/28 14:58:04, jkarlin wrote:
> > Ping jar@, is it safe to call field trial code from IO thread like
> > prefetch_field_trial.cc does?
> 
> Histograms, and field trial inquiries, may be called from any thread, at any
> time.
> 
> The code has locking "when it is needed," and the histogram macros may use
some
> fancy lock-free code at times to enable use of efficient atomic statics, etc.

Thanks!

[commit-bot: I haz the power, 2014-01-29 18:31:07]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-29 18:44:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jkarlin@chromium.org/142473003/120001

[commit-bot: I haz the power, 2014-01-29 20:44:02]

Change committed as 247738