Omnibox: Add Field Trial to Make Shortcuts More Aggressive

Omnibox: Add Field Trial to Make Shortcuts More Aggressive

Change ShortcutsProvider so it can assign high relevance scores.
Simultaneously, make ShortcutsProvider explicitly enforce the rule
that it should not return a result with an inlineable score (>=1200).

Finally, create a field trial that will allow the server to specify
the maximum relevance score ShortcutsProvider can assign.  Setting this
to 1199 will maintain current behavior.  Also, if the field trial is
not enabled through the variations server, Chrome will maintain the
current behavior.

BUG=252032
TEST=enter omnibox inputs that you've used before to get places (and hence shortcuts will have data about them) into about:omnibox and look at the resulting scores.  Try again with --force-fieldtrials=OmniboxShortcutsScoring/MaxRelevance_XXX/ (where XXX is a number) and see how the scores change.

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

[Mark P, 2013-06-19 23:56:28]

Hi Peter,

Please take a look when you get a chance.  I'm finally giving Shortcuts some
love.

thanks,
mark

[Peter Kasting, 2013-06-20 00:06:28]

* CL description: bug number missing, test line cryptic
* The idea of this seems a little odd: allow higher scores, but then clamp to
the same max as before.  If things show improvement, do we design a different
scoring curve that doesn't fall off as quickly, or just make this logic be how
the shortcuts provider does things?  In either case, we'll probably need to test
everything again if/when we finally allow the shortcut provider to inline.

LGTM

https://codereview.chromium.org/17261012/diff/4001/chrome/browser/autocomplet...
File chrome/browser/autocomplete/shortcuts_provider.cc (right):

https://codereview.chromium.org/17261012/diff/4001/chrome/browser/autocomplet...
chrome/browser/autocomplete/shortcuts_provider.cc:185: max_relevance--;
Tiny nit: Use preincrement wherever you don't need postincrement

[Mark P, 2013-06-20 00:23:16]

> * CL description: bug number missing, test line cryptic

Created bug and added link to it.

Expanded on TEST= line.

> * The idea of this seems a little odd: allow higher scores, but then clamp
> to the same max as before.  If things show improvement, do we design
> a different scoring curve that doesn't fall off as quickly, or just make
> this logic be how the shortcuts provider does things?  In either case,
> we'll probably need to test everything again if/when we finally allow
> the shortcut provider to inline.

We were pretty much never hitting the top of scoring curve before.  We'd rarely
end up assigning a score in the 1100 - 1199 range.  This change drags the
scoring curve upward, then enforces the constraint later.  I think this makes
sense to me: determine the best estimate of the relevance score that you can,
and only later enforce constraints.  I don't like the idea of baking constraints
into the inner workings of scoring functions.

If this turns out positive, I would simply turn it on.  Later I can do further
tuning of the scoring function as desired.

I want to talk to you in general about inlineability and its connection to
scores and how we enforce this.  I realized we have this conversation
periodically, and it's making me anxious again so I want to talk about it again.
 I'll try to grab you tomorrow.

--mark

https://codereview.chromium.org/17261012/diff/4001/chrome/browser/autocomplet...
File chrome/browser/autocomplete/shortcuts_provider.cc (right):

https://codereview.chromium.org/17261012/diff/4001/chrome/browser/autocomplet...
chrome/browser/autocomplete/shortcuts_provider.cc:185: max_relevance--;
On 2013/06/20 00:06:28, Peter Kasting wrote:
> Tiny nit: Use preincrement wherever you don't need postincrement

Done.

[Peter Kasting, 2013-06-20 00:31:37]

On 2013/06/20 00:23:16, Mark P wrote:
> We were pretty much never hitting the top of scoring curve before.  We'd
rarely
> end up assigning a score in the 1100 - 1199 range.

There are two ways to address this.  One is to boost the top score amount like
you're doing here until results seem to arrive in the right range.  The other is
to change the way we calculate the scores more fundamentally so that we're more
likely to give good matches "high" scores (where "high" just means, near the top
of our scoring range).  In practice these turn out fairly similarly.  The
distinction I'm basically getting at is that the latter modification assumes
that maybe more is wrong with the scoring curve than just that the end results
don't come out high enough.

I'm not really complaining about this patch, I guess I'm just saying that we may
need a lot more examination and tweaking of the scoring algorithm, since IIRC
there was pretty much zero data collection or rigorous thought put into it.  But
that's probably so obvious that it's not worth me having written this :P

[commit-bot: I haz the power, 2013-06-20 14:39:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/17261012/9001

[commit-bot: I haz the power, 2013-06-20 15:03:53]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-20 18:52:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/17261012/27001

[commit-bot: I haz the power, 2013-06-21 17:32:42]

Change committed as 207872