Field trial to investigate higher weightings for hits when calculating Omnibox prerender confidence.

Field trial to investigate higher weightings for hits when calculating Omnibox prerender confidence.

BUG=111069
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=120133

[dominich, 2012-01-27 22:28:31]

pkasting: autocomplete
tburkard: prerender
estade: resources and ui

Thanks!

[Peter Kasting, 2012-01-27 22:30:39]

LGTM

http://codereview.chromium.org/9298032/diff/1/chrome/browser/autocomplete/net...
File chrome/browser/autocomplete/network_action_predictor.h (right):

http://codereview.chromium.org/9298032/diff/1/chrome/browser/autocomplete/net...
chrome/browser/autocomplete/network_action_predictor.h:121: static double
hit_weight_;
Nit: Add comments about what this means, who uses it, and why it's static.

[Evan Stade, 2012-01-28 03:30:48]

webui lgtm

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/resources/net...
File
chrome/browser/resources/network_action_predictor/network_action_predictor.js
(right):

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/resources/net...
chrome/browser/resources/network_action_predictor/network_action_predictor.js:18:
* including the database as a flattened list, a boolean indicating if the
indent text 4 spaces

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/resources/net...
chrome/browser/resources/network_action_predictor/network_action_predictor.js:36:
* including the database as a flattened list, a boolean indicating if the
indent text 4 spaces

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/ui/webui/netw...
File
chrome/browser/ui/webui/network_action_predictor/network_action_predictor_dom_handler.cc
(right):

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/ui/webui/netw...
chrome/browser/ui/webui/network_action_predictor/network_action_predictor_dom_handler.cc:31:
const bool enabled = (network_action_predictor_ != NULL);
no need for parens

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/ui/webui/netw...
chrome/browser/ui/webui/network_action_predictor/network_action_predictor_dom_handler.cc:32:

remove \n

[dominich, 2012-01-31 16:05:48]

tburkard: PTAL

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/resources/net...
File
chrome/browser/resources/network_action_predictor/network_action_predictor.js
(right):

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/resources/net...
chrome/browser/resources/network_action_predictor/network_action_predictor.js:18:
* including the database as a flattened list, a boolean indicating if the
On 2012/01/28 03:30:48, Evan Stade wrote:
> indent text 4 spaces

Done.

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/resources/net...
chrome/browser/resources/network_action_predictor/network_action_predictor.js:36:
* including the database as a flattened list, a boolean indicating if the
On 2012/01/28 03:30:48, Evan Stade wrote:
> indent text 4 spaces

Done.

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/ui/webui/netw...
File
chrome/browser/ui/webui/network_action_predictor/network_action_predictor_dom_handler.cc
(right):

http://codereview.chromium.org/9298032/diff/6001/chrome/browser/ui/webui/netw...
chrome/browser/ui/webui/network_action_predictor/network_action_predictor_dom_handler.cc:32:

On 2012/01/28 03:30:48, Evan Stade wrote:
> remove \n

Done.

[tburkard, 2012-02-01 09:15:54]

LGTM

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_field_trial.cc (right):

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_field_trial.cc:69: const int
kExperiment1Group = trial->kDefaultGroupNumber;
Great Cleanup!  Removing the unnecessary Prerender from all variables definitely
makes this easier to read & to maintain.

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_histograms.cc (right):

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_histograms.cc:51:
NetworkActionPredictor::get_hit_weight();
I personally would strongly prefer doing something like this:
return ComposeHistogramName(StringPrintf("omnibox_...", ...).c_str(), name);
I think it's more concise and easier to maintain.

[dominich, 2012-02-01 17:21:17]

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_field_trial.cc (right):

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_field_trial.cc:69: const int
kExperiment1Group = trial->kDefaultGroupNumber;
On 2012/02/01 09:15:54, tburkard wrote:
> Great Cleanup!  Removing the unnecessary Prerender from all variables
definitely
> makes this easier to read & to maintain.

Done.

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
File chrome/browser/prerender/prerender_histograms.cc (right):

http://codereview.chromium.org/9298032/diff/7001/chrome/browser/prerender/pre...
chrome/browser/prerender/prerender_histograms.cc:51:
NetworkActionPredictor::get_hit_weight();
On 2012/02/01 09:15:54, tburkard wrote:
> I personally would strongly prefer doing something like this:
> return ComposeHistogramName(StringPrintf("omnibox_...", ...).c_str(), name);
> I think it's more concise and easier to maintain.

Done.

[commit-bot: I haz the power, 2012-02-01 21:33:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominich@chromium.org/9298032/14001

[commit-bot: I haz the power, 2012-02-01 23:54:18]

Change committed as 120133