Launch HQP & HUP score changes.

Launch HQP+HUP score changes in chrome binary. We already launched it through finch configs, now just making the binary change. Also adding an additional option for HUP experimental scoring to run finch experiments with additional decay factors.

More details can be found in the bug.

BUG=306198
Committed: https://crrev.com/e3cfb233f6d7689d96fdb52a87caa3ada346666d
Cr-Commit-Position: refs/heads/master@{#347029}

[Ashok vardhan, 2015-08-26 22:47:38]

ashokvardhan@chromium.org changed reviewers:
+ bartn@chromium.org, mpearson@chromium.org

[Bart N., 2015-08-26 23:01:26]

Just a few minor comments. Please add Peter Kasting to R.

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:134: if
(score_buckets.use_decay_factor()) {
This is invariant, so I suggest taking it out of the loop.

Something like:
const double factor = (score_buckets.use_decay_factor() ? decay_factor :
decay_count);

and then use 'factor' inside the loop.

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:159:
!scoring_params.default_scoring_enabled) {
I don't understand why we need default_scoring_enabled.

Can you simply make default_scoring params initialized before applying
experiment overrides? In this approach, defaults are always there and can be
optionally overridden.

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:80: it =
params.find(use_decay_factor_param);
Can you add a unit test to omnibox_field_trial_test.cc and test the new param?

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:80: bool use_decay_factor_ =
false;
= false will most likely not compile on some platforms. Can you move it to the
constructor?

[Ashok vardhan, 2015-08-27 00:30:46]

Addressed comments. PTAL.

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:134: if
(score_buckets.use_decay_factor()) {
On 2015/08/26 23:01:25, Bart N. wrote:
> This is invariant, so I suggest taking it out of the loop.
> 
> Something like:
> const double factor = (score_buckets.use_decay_factor() ? decay_factor :
> decay_count);
> 
> and then use 'factor' inside the loop.

Done.

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:159:
!scoring_params.default_scoring_enabled) {
On 2015/08/26 23:01:25, Bart N. wrote:
> I don't understand why we need default_scoring_enabled.
> 
> Can you simply make default_scoring params initialized before applying
> experiment overrides? In this approach, defaults are always there and can be
> optionally overridden.

Done.

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:80: it =
params.find(use_decay_factor_param);
On 2015/08/26 23:01:25, Bart N. wrote:
> Can you add a unit test to omnibox_field_trial_test.cc and test the new param?

Acknowledged.

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/1286093006/diff/120001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:80: bool use_decay_factor_ =
false;
On 2015/08/26 23:01:25, Bart N. wrote:
> = false will most likely not compile on some platforms. Can you move it to the
> constructor?

Done.

[Ashok vardhan, 2015-08-27 00:31:02]

ashokvardhan@chromium.org changed reviewers:
+ pkasting@chromium.org

[Peter Kasting, 2015-08-27 00:50:45]

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:130: // decay factor, then
use it correspondingly.
Nit: After updating the comments in the .h file (see my comment there), come
back to this and see if this can be removed entirely, or else re-worded a bit
less awkwardly (i.e. not structured like "This uses counts.  Oh, wait, it might
optionally use factors instead.").

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:84:
score_buckets->set_use_decay_factor((use_decay_factor != 0));
Nit: Extra parens

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:286:
"1.0:1413,0.97:1390,0.93:1360,0.85:1340,0.72:1320,0.50:1250,0.0:1203";
These values are incredibly magic.  Can there at least be a comment as to where
they come from?

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:293: const std::string&
visit_count_score_buckets_str = "4.0:790,0.5:590,0.0:100";
Same.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:304: &params)) {
Nit: Don't add {}

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:25: // maximum relevance score
in the corresponding bucket list.
I think you should update this comment and the comments below (e.g. the one on
the typedef) to either be more vague, e.g. talk about a "threshold" without
saying what the units are, or else describe the two choices for what the values
mean and how the caller knows what to use.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial_unittest.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial_unittest.cc:452:
params[std::string(OmniboxFieldTrial::kHUPNewScoringEnabledParam)] = "1";
Do the std::string() calls here need to be explicitly made?

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/scored_history_match.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/scored_history_match.cc:117:
"0.0:400,1.5:600,5.0:900,10.5:1203,15.0:1300,20.0:1399";
Another cryptically-magic string.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/scored_history_match.h (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/scored_history_match.h:191: static std::string
hqp_relevance_buckets_str_;
For this and the other variables here, if we only need to refer to these in the
.cc file, make them file-scoped instead of private statics.

Also, static class members of non-POD type are banned.  You'd need to use a
char[] instead, or some other solution.

[Ashok vardhan, 2015-08-27 19:09:27]

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:130: // decay factor, then
use it correspondingly.
On 2015/08/27 00:50:44, Peter Kasting wrote:
> Nit: After updating the comments in the .h file (see my comment there), come
> back to this and see if this can be removed entirely, or else re-worded a bit
> less awkwardly (i.e. not structured like "This uses counts.  Oh, wait, it
might
> optionally use factors instead.").

Done.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:84:
score_buckets->set_use_decay_factor((use_decay_factor != 0));
On 2015/08/27 00:50:44, Peter Kasting wrote:
> Nit: Extra parens

Done.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:286:
"1.0:1413,0.97:1390,0.93:1360,0.85:1340,0.72:1320,0.50:1250,0.0:1203";
On 2015/08/27 00:50:44, Peter Kasting wrote:
> These values are incredibly magic.  Can there at least be a comment as to
where
> they come from?

Done.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:293: const std::string&
visit_count_score_buckets_str = "4.0:790,0.5:590,0.0:100";
On 2015/08/27 00:50:44, Peter Kasting wrote:
> Same.

Done.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:304: &params)) {
On 2015/08/27 00:50:44, Peter Kasting wrote:
> Nit: Don't add {}

Done.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:25: // maximum relevance score
in the corresponding bucket list.
On 2015/08/27 00:50:45, Peter Kasting wrote:
> I think you should update this comment and the comments below (e.g. the one on
> the typedef) to either be more vague, e.g. talk about a "threshold" without
> saying what the units are, or else describe the two choices for what the
values
> mean and how the caller knows what to use.

Done.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial_unittest.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial_unittest.cc:452:
params[std::string(OmniboxFieldTrial::kHUPNewScoringEnabledParam)] = "1";
On 2015/08/27 00:50:45, Peter Kasting wrote:
> Do the std::string() calls here need to be explicitly made?

I think so, to convert const char to string. Followed the existing tests.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/scored_history_match.cc (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/scored_history_match.cc:117:
"0.0:400,1.5:600,5.0:900,10.5:1203,15.0:1300,20.0:1399";
On 2015/08/27 00:50:45, Peter Kasting wrote:
> Another cryptically-magic string.

Done.

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
File components/omnibox/browser/scored_history_match.h (right):

https://codereview.chromium.org/1286093006/diff/140001/components/omnibox/bro...
components/omnibox/browser/scored_history_match.h:191: static std::string
hqp_relevance_buckets_str_;
On 2015/08/27 00:50:45, Peter Kasting wrote:
> For this and the other variables here, if we only need to refer to these in
the
> .cc file, make them file-scoped instead of private statics.
> 
> Also, static class members of non-POD type are banned.  You'd need to use a
> char[] instead, or some other solution.

Most of them are referenced in unit-tests also. I changed to char[] instead of
string.

[Peter Kasting, 2015-08-27 19:21:41]

Thanks, this is better.  Some ideas for further comment improvement below.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:130: const double factor =
(score_buckets.use_decay_factor() ? decay_factor :
Nit: Break after ? rather than :

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:50: // Clear the buckets.
Nit: Wrong indent

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:286: // 1-day older give score
of 1390, 3 days older 0.93 and so on. Values are
Nit: Omit the sentence about "If url is 1-day older" as this just duplicates the
numbers below; it's the motivation for these numbers we're interested in.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:288: // number of days back
they visited.
Nit: This last sentence is kind of vague.  It's sort of clear from the general
idea of these buckets that we're demoting URLs with lower counts like this,
what's more interesting is how we obtained these numbers.  For example, "The
values here are based on the results of field trials to determine what maximized
overall result quality." (If that's actually true.)

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:298: // (which practically
means to drop the urls on to floor).
Similar comment applies here.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:30: //
<decayed_count/decay-factor,max_relevance) pair.
Nit: How about just:

Stores the max relevance at each count/decay factor threshold.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:73: // decayed_count.
Nit: How about just:

The relevance score caps at successively decreasing threshold values.  The
thresholds are either decayed counts or decay factors, depending on the value of
|use_decay_factor_|.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:84: // Specify decay factor in
buckets rather than actual score.
Nit: How about:

True when the bucket thresholds are decay factors rather than counts.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial_unittest.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial_unittest.cc:452:
params[std::string(OmniboxFieldTrial::kHUPNewScoringEnabledParam)] = "1";
At least test locally whether std::string() can be omitted; I really would have
thought that the implicit string constructor would kick in here, but maybe the
fact that map is templated makes the compiler unable to figure this out.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/scored_history_match.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/scored_history_match.cc:118: //
GetFinalRelevancyScore() for more details.
Again, I'm not so much interested in a comment restating the numbers as I'm
interested in how/why we picked these particular numbers.

[Bart N., 2015-08-27 19:22:33]

LGTM

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial_unittest.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial_unittest.cc:471: ASSERT_EQ(true,
scoring_params.typed_count_buckets.use_decay_factor());
ASSERT_TRUE instead?

[Ashok vardhan, 2015-08-27 19:51:38]

Thanks. I am bad at some commenting language. Addressed. Please take a look.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:130: const double factor =
(score_buckets.use_decay_factor() ? decay_factor :
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Nit: Break after ? rather than :

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:50: // Clear the buckets.
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Nit: Wrong indent

Acknowledged.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:286: // 1-day older give score
of 1390, 3 days older 0.93 and so on. Values are
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Nit: Omit the sentence about "If url is 1-day older" as this just duplicates
the
> numbers below; it's the motivation for these numbers we're interested in.

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:288: // number of days back
they visited.
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Nit: This last sentence is kind of vague.  It's sort of clear from the general
> idea of these buckets that we're demoting URLs with lower counts like this,
> what's more interesting is how we obtained these numbers.  For example, "The
> values here are based on the results of field trials to determine what
maximized
> overall result quality." (If that's actually true.)

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:298: // (which practically
means to drop the urls on to floor).
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Similar comment applies here.

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:30: //
<decayed_count/decay-factor,max_relevance) pair.
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Nit: How about just:
> 
> Stores the max relevance at each count/decay factor threshold.

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:73: // decayed_count.
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Nit: How about just:
> 
> The relevance score caps at successively decreasing threshold values.  The
> thresholds are either decayed counts or decay factors, depending on the value
of
> |use_decay_factor_|.

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.h:84: // Specify decay factor in
buckets rather than actual score.
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Nit: How about:
> 
> True when the bucket thresholds are decay factors rather than counts.

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial_unittest.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial_unittest.cc:452:
params[std::string(OmniboxFieldTrial::kHUPNewScoringEnabledParam)] = "1";
On 2015/08/27 19:21:40, Peter Kasting wrote:
> At least test locally whether std::string() can be omitted; I really would
have
> thought that the implicit string constructor would kick in here, but maybe the
> fact that map is templated makes the compiler unable to figure this out.

Its working on linux and can be omitted. I made the change. Thanks for that.!
Let me know if it doesn't work for other platforms.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial_unittest.cc:471: ASSERT_EQ(true,
scoring_params.typed_count_buckets.use_decay_factor());
On 2015/08/27 19:22:33, Bart N. wrote:
> ASSERT_TRUE instead?

Done.

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/scored_history_match.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/scored_history_match.cc:118: //
GetFinalRelevancyScore() for more details.
On 2015/08/27 19:21:40, Peter Kasting wrote:
> Again, I'm not so much interested in a comment restating the numbers as I'm
> interested in how/why we picked these particular numbers.

Done.

[Peter Kasting, 2015-08-27 19:55:01]

You don't seem to have uploaded a new patch.

[Ashok vardhan, 2015-08-27 20:11:35]

done

[Peter Kasting, 2015-08-27 20:29:31]

LGTM

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial_unittest.cc (right):

https://codereview.chromium.org/1286093006/diff/160001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial_unittest.cc:452:
params[std::string(OmniboxFieldTrial::kHUPNewScoringEnabledParam)] = "1";
On 2015/08/27 19:51:38, Ashok vardhan wrote:
> On 2015/08/27 19:21:40, Peter Kasting wrote:
> > At least test locally whether std::string() can be omitted; I really would
> have
> > thought that the implicit string constructor would kick in here, but maybe
the
> > fact that map is templated makes the compiler unable to figure this out.
> 
> Its working on linux and can be omitted. I made the change. Thanks for that.!
> Let me know if it doesn't work for other platforms.

The gold standard is "does it pass the trybots".  If it does, then bonus points
if you fix the similar instances elsewhere in this file.

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:131: decay_factor :
decayed_count);
Nit: Only indent this line 4

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:287: // maximized overall
result quality.
This is true, right?  That these numbers are the outcome of the various field
trial tests?

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:299: // (which practically
means to drop the urls on to floor).
Nit: How about:

Buckets based on visit count.  Like the typed count buckets above, the values
here were chosen based on field trials.  Note that when a URL hasn't been
visited in the last 30 days, we clamp its score to 100, which basically demotes
it below any other results in the dropdown.

[Ashok vardhan, 2015-08-27 21:18:15]

Thanks for the review.

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
File components/omnibox/browser/history_url_provider.cc (right):

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
components/omnibox/browser/history_url_provider.cc:131: decay_factor :
decayed_count);
On 2015/08/27 20:29:30, Peter Kasting wrote:
> Nit: Only indent this line 4

Done.

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:287: // maximized overall
result quality.
On 2015/08/27 20:29:31, Peter Kasting wrote:
> This is true, right?  That these numbers are the outcome of the various field
> trial tests?

Ho yes.. more details in the bug.!! Result of various experiments from long
time.!

https://codereview.chromium.org/1286093006/diff/180001/components/omnibox/bro...
components/omnibox/browser/omnibox_field_trial.cc:299: // (which practically
means to drop the urls on to floor).
On 2015/08/27 20:29:31, Peter Kasting wrote:
> Nit: How about:
> 
> Buckets based on visit count.  Like the typed count buckets above, the values
> here were chosen based on field trials.  Note that when a URL hasn't been
> visited in the last 30 days, we clamp its score to 100, which basically
demotes
> it below any other results in the dropdown.

Done.

[Ashok vardhan, 2015-08-27 21:18:29]

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

[commit-bot: I haz the power, 2015-08-27 21:19:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286093006/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286093006/200001

[commit-bot: I haz the power, 2015-08-27 21:55:05]

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

[commit-bot: I haz the power, 2015-08-27 21:55:06]

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

[Ashok vardhan, 2015-09-01 17:47:01]

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

[commit-bot: I haz the power, 2015-09-01 17:47:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286093006/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286093006/200001

[commit-bot: I haz the power, 2015-09-01 18:14:10]

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

[commit-bot: I haz the power, 2015-09-01 18:14:11]

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

[Ashok vardhan, 2015-09-02 00:18:24]

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

[commit-bot: I haz the power, 2015-09-02 00:19:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286093006/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286093006/220001

[commit-bot: I haz the power, 2015-09-02 00:47:54]

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

[commit-bot: I haz the power, 2015-09-02 00:47:54]

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

[Ashok vardhan, 2015-09-02 01:11:03]

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

[commit-bot: I haz the power, 2015-09-02 01:11:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286093006/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286093006/240001

[commit-bot: I haz the power, 2015-09-02 01:49:15]

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

[commit-bot: I haz the power, 2015-09-02 01:49:16]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Ashok vardhan, 2015-09-02 17:26:16]

The CQ bit was checked by ashokvardhan@chromium.org

[Ashok vardhan, 2015-09-02 17:26:16]

The patchset sent to the CQ was uploaded after l-g-t-m from bartn@chromium.org,
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1286093006/#ps240001
(title: " ")

[commit-bot: I haz the power, 2015-09-02 17:27:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286093006/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286093006/240001

[Ashok vardhan, 2015-09-02 17:30:06]

The CQ bit was checked by ashokvardhan@chromium.org

[commit-bot: I haz the power, 2015-09-02 17:30:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286093006/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286093006/240001

[commit-bot: I haz the power, 2015-09-02 18:15:01]

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

[commit-bot: I haz the power, 2015-09-02 18:15:02]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Ashok vardhan, 2015-09-02 20:55:51]

The CQ bit was checked by ashokvardhan@chromium.org

[Ashok vardhan, 2015-09-02 20:55:52]

The patchset sent to the CQ was uploaded after l-g-t-m from bartn@chromium.org,
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1286093006/#ps260001
(title: " ")

[commit-bot: I haz the power, 2015-09-02 20:56:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286093006/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286093006/260001

[commit-bot: I haz the power, 2015-09-02 21:25:44]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2015-09-02 21:26:28]

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