Show PhysicalWebProvider suggestions with omnibox input

Show PhysicalWebProvider suggestions with omnibox input

Add support for "Query Suggest" in the Physical Web omnibox provider.
Previously, the omnibox would only display Physical Web omnibox suggestions
when the user had not yet entered any text in the omnibox ("Zero Suggest").
With this CL, Physical Web suggestions may be displayed if the URL and page
title match all the terms in the omnibox query string.

BUG=630769
Review-Url: https://codereview.chromium.org/2591053002
Cr-Commit-Position: refs/heads/master@{#442098}
Committed: https://chromium.googlesource.com/chromium/src/+/a04fff0b8cd393873fa9c1864a98525035519866

[mattreynolds, 2016-12-20 20:50:50]

mattreynolds@chromium.org changed reviewers:
+ mpearson@chromium.org

[mattreynolds, 2016-12-20 20:50:50]

Hi Mark, PTAL

This CL depends on https://codereview.chromium.org/2583763003/
"Issue 2583763003: Factor out AutocompleteMatch creation from BookmarkProvider"

[Mark P, 2016-12-21 18:57:50]

I have concerns about scoring and launch/eval.

* What sort of thought have you given to the score of as-you-type suggestions? 
Right now, first, you're using a fixed score, which I find a bit surprising. 
And, second,you're using the same fixed score which we picked arbitrarily so it
works with the other zero suggest providers.  Now you have to interact well with
other providers.
  This fixed score might happen to be at a values that good and defensible for
as-you-typed suggestions, if a bit conservative.  I'm more curious about what
you've thought about how prominently and when you these suggestions displayed as
the user types.

* How do you plan to roll out / launch this?  And on which platforms, when?
  For iOS in zero suggest, when you're not competing with anything else, I am
okay with a minimal eval, not necessarily even a field trial--just look as you
launch at at the impressions for zero suggest physical web and the number of
clicks and see if the ratio is plausible or if you're taking up too much screen
real estate.  Ideally, I'd also look to see if it takes longer before a user
starts type in the omnibox after focussing it (Omnibox.FocusToEditTime
histogram).
  For Android in zero suggest, I'd require a field trial because you're compete
with another very popular provider (zero suggest most visited).  We'd need a
full eval.  To make this work with the other omnibox field trials and analysis
pipeline, you'd have to gate adding the physical web provider on a parameter. 
You'll need to add a function to omnibox_field_trial.h.  (If you're doing this
though, you might as well do it in a platform independent-way, thus setting up
iOS physical web provider only when the field trial enables it.)
  For either platforms for as-you-type suggestions, you'll also need a field
trial.  I prefer for you to set this up separately from the enable zero suggest
so we can evaluate the as-you-type scoring independently from the zero suggest
situation.
  In other words, please add to omnibox_field_trial.{h,cc} two functions,
something like EnablePhysicalWebProvider() and
EnablePhysicalWebAsYouTypeMatches().

cheers,
mark

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_node.cc (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_node.cc:17: if
(metadata_item.GetString(physical_web::kResolvedUrlKey, &url)) {
BTW, why do you use the resolved URL key here?  What's the difference between
this and the scanned URL key?  I searched the code base and cannot find an
explanation anyway.  (Please consider adding one somewhere if there isn't one.) 
The only thing I found is an example that indicates the resolved one is a "long"
url and the scanned one is "short".

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_provider.cc (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:44: }
style nit (I think): blank line before closing namespace brace

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:194:
bookmarks::TitledUrlIndex index(NULL);
Please either add a comment (in this changelist or another) to the constructor
of TitledUrlIndex that NULL is valid.  And generally explain what the passed
parameter means.

If you don't want to do that there (I might not understand why), then do it
here.

Also, nit: NULL -> nullptr

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:196: size_t metadata_count =
metadata_list->GetSize();
nit: const

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:200:
nodes.push_back(base::MakeUnique<PhysicalWebNode>(*metadata_item));
This seems roundabout to me.  Why not call index.Add() directly?  Why do you
need the nodes vector?

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_provider.h (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.h:47: // When the user has
focused the omnibox but not yet entered any text (ie,
nit: ie -> i.e.
ditto below

[Mark P, 2016-12-21 21:11:19]

Thanks calling to discuss my comments, short-circuiting what would've been a
possibly-long e-mail exchange.

One additional point that I forgot to mention:
I would like to see you add "counterfactual" logging.  This means logging
something in zero suggest provider when physical web suggestions are available
even if the user is in a control or experiment group that does not result in
them being displayed.  This will allow us to focus the omnibox eval--looking at
how users behave--on only the times when a physical web beacon was nearby.  In
other words, it lets us focus the eval on the situation where the ability to
display a physical web suggestion might've made a different.  This'll help us
find the signal amongst the large sea of noise.

To do this, I'll correct my earlier suggestion of having a flag that controls
whether to add the physical web provider to the list of providers and a flag
that controls whether to provide as-you-type suggestions.  Instead, always add
the physical web provider.  Have a flag to control whether you return physical
web zero suggest results and a separate flag for returning physical web
as-you-typed results.

Then, to add counterfactual logging--which I'm fine with you doing in a separate
changelist--, look at the code in
components/omnibox/browser/physical_web_provider.cc from patch set 2,
on line 82 (just after constructing zero suggest matches) do the following:
1. call a function AddProviderInfo().   This will record the counterfactual bit,
i.e., whether you actually had suggestions.
2. if the physical web zero suggest flag is not enabled, clear the list of
matches.

Ditto just after line 84: call AddProviderInfo, then clear the matches if you're
not supposed to be providing as-you-type matches.

Your AddProviderInfo() function should look something like this one:
https://cs.chromium.org/chromium/src/components/omnibox/browser/base_search_p...
just with
  if (field_trial_triggered_)
replaced with
  if (matches_.size() > 0)

I think you should also deal with the
  if (field_trial_triggered_in_session_)
bit too.  That'll require more bookkeeping.  (You'll have to keep track of
whether you had matches_.size() > 0 at any point from the last on-focus call to
the physical web provider.)

I hope this message is clear.

cheers,
mark

[mattreynolds, 2016-12-21 22:25:59]

I think it's not possible to call AddProviderInfo from the provider itself since
we won't have the ProvidersInfo object to append to.

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_node.cc (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_node.cc:17: if
(metadata_item.GetString(physical_web::kResolvedUrlKey, &url)) {
On 2016/12/21 18:57:50, Mark P wrote:
> BTW, why do you use the resolved URL key here?  What's the difference between
> this and the scanned URL key?  I searched the code base and cannot find an
> explanation anyway.  (Please consider adding one somewhere if there isn't
one.) 
> The only thing I found is an example that indicates the resolved one is a
"long"
> url and the scanned one is "short".

The scanned URL is the URL broadcast by the Physical Web beacon. Due to BLE
packet size limitations, the number of bytes available for the URL is very
constrained, so most Physical Web beacons broadcast a shortened URL. Instead of
displaying this shortened URL, the Physical Web service follows the redirect
chain and returns the final, resolved URL.

We actually have a CL up already that improves the documentation in
physical_web_data_source.h:

https://codereview.chromium.org/2561493002/patch/160001/170010

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_provider.cc (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:44: }
On 2016/12/21 18:57:50, Mark P wrote:
> style nit (I think): blank line before closing namespace brace

Done.

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:194:
bookmarks::TitledUrlIndex index(NULL);
On 2016/12/21 18:57:50, Mark P wrote:
> Please either add a comment (in this changelist or another) to the constructor
> of TitledUrlIndex that NULL is valid.  And generally explain what the passed
> parameter means.
> 
> If you don't want to do that there (I might not understand why), then do it
> here.

I'll add the comment here and comment the TitledUrlIndex constructor in a
separate CL.

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:196: size_t metadata_count =
metadata_list->GetSize();
On 2016/12/21 18:57:50, Mark P wrote:
> nit: const

Done.

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:200:
nodes.push_back(base::MakeUnique<PhysicalWebNode>(*metadata_item));
On 2016/12/21 18:57:50, Mark P wrote:
> This seems roundabout to me.  Why not call index.Add() directly?  Why do you
> need the nodes vector?

TitledUrlIndex doesn't take ownership of the PhysicalWebNodes we're creating. By
storing them in a vector of unique_ptrs we can ensure they are properly cleaned
up once the vector falls out of scope.

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_provider.h (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.h:47: // When the user has
focused the omnibox but not yet entered any text (ie,
On 2016/12/21 18:57:50, Mark P wrote:
> nit: ie -> i.e.
> ditto below

Done.

[Mark P, 2016-12-21 23:21:41]

>>>
I think it's not possible to call AddProviderInfo from the provider itself since
we won't have the ProvidersInfo object to append to.
>>>
Good point.  Instead, you'd want to have a private variable, something like
had_physical_web_suggestions_.  You'd set that where I told you to call
AddProviderInfo.

And you'd implement AddProviderInfo as I suggested.  It'll automatically get
called.

You can also add the internal variable
had_physical_web_suggestions_at_focus_or_later_ to get the "shown at any point
during this interaction" bit.

--mark

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_node.cc (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_node.cc:17: if
(metadata_item.GetString(physical_web::kResolvedUrlKey, &url)) {
On 2016/12/21 22:25:58, mattreynolds wrote:
> On 2016/12/21 18:57:50, Mark P wrote:
> > BTW, why do you use the resolved URL key here?  What's the difference
between
> > this and the scanned URL key?  I searched the code base and cannot find an
> > explanation anyway.  (Please consider adding one somewhere if there isn't
> one.) 
> > The only thing I found is an example that indicates the resolved one is a
> "long"
> > url and the scanned one is "short".
> 
> The scanned URL is the URL broadcast by the Physical Web beacon. Due to BLE
> packet size limitations, the number of bytes available for the URL is very
> constrained, so most Physical Web beacons broadcast a shortened URL. Instead
of
> displaying this shortened URL, the Physical Web service follows the redirect
> chain and returns the final, resolved URL.
> 
> We actually have a CL up already that improves the documentation in
> physical_web_data_source.h:
> 
> https://codereview.chromium.org/2561493002/patch/160001/170010

Nice!

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
File components/omnibox/browser/physical_web_provider.cc (right):

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:194:
bookmarks::TitledUrlIndex index(NULL);
On 2016/12/21 22:25:58, mattreynolds wrote:
> On 2016/12/21 18:57:50, Mark P wrote:
> > Please either add a comment (in this changelist or another) to the
constructor
> > of TitledUrlIndex that NULL is valid.  And generally explain what the passed
> > parameter means.
> > 
> > If you don't want to do that there (I might not understand why), then do it
> > here.
> 
> I'll add the comment here and comment the TitledUrlIndex constructor in a
> separate CL.

Acknowledged.

https://codereview.chromium.org/2591053002/diff/20001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:200:
nodes.push_back(base::MakeUnique<PhysicalWebNode>(*metadata_item));
On 2016/12/21 22:25:58, mattreynolds wrote:
> On 2016/12/21 18:57:50, Mark P wrote:
> > This seems roundabout to me.  Why not call index.Add() directly?  Why do you
> > need the nodes vector?
> 
> TitledUrlIndex doesn't take ownership of the PhysicalWebNodes we're creating.
By
> storing them in a vector of unique_ptrs we can ensure they are properly
cleaned
> up once the vector falls out of scope.

Acknowledged.

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:549: void
OmniboxFieldTrial::GetExperimentalPhysicalWebScoringParams(
minor comment: I think I find these three separate find calls harder to read
then three separate GetVariationParamValue() call.

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:562: if
(base::StringToInt(it->second, &enabled))
Enabling is an int parameter?

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.h:98: // The set of parameters
customizing PhysicalWebProvider relevance scoring.
This approach works but I feel it's a little heavy-handed.  (I see you copied it
from above.)  Plus, it's easy to make mistakes in configuration (the more
parameters, the more opportunities for mistakes.)  For example, your enabling
scoring code expects a "1" (or similar) to be enabled whereas your enabling
suggestions appearing code expected as "true".  I can easily imagine this biting
us at some point.  It's also the case that the current behavior is a problem if
we enable experimental scoring but don't, for example, specify one of your
current parameters (we'd try using the base relevance of 0).

Nowadays I prefer the approach where the scoring code isn't gated on a separate
flag (experimental_scoring_enabled).  Instead, simply have a function
GetPhysicalWebZeroSuggestBaseRelevance() that looks up the parameter directly
and returns it, returning a default value if the parameter doesn't exist.  This
will also let you get rid of the experiment gating test, making the
Get...Relevance function shorter (and more error-proof and maybe also easier to
read).  It'll also let you get rid of the default value in the physical web
code.  (Always get the value from the field trial code, regardless.  Make that
code have the default.)

This suggestion is optional.  I think it's cleaner.  Your approach is
nonetheless like others in this file so if you insist on this way, that's okay
too.

[mattreynolds, 2016-12-22 00:23:40]

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:549: void
OmniboxFieldTrial::GetExperimentalPhysicalWebScoringParams(
On 2016/12/21 23:21:41, Mark P wrote:
> minor comment: I think I find these three separate find calls harder to read
> then three separate GetVariationParamValue() call.

I switched the base relevance calls to use GetVariationParamValue as well.

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:562: if
(base::StringToInt(it->second, &enabled))
On 2016/12/21 23:21:41, Mark P wrote:
> Enabling is an int parameter?

Removed

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/2591053002/diff/40001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.h:98: // The set of parameters
customizing PhysicalWebProvider relevance scoring.
On 2016/12/21 23:21:41, Mark P wrote:
> This approach works but I feel it's a little heavy-handed.  (I see you copied
it
> from above.)  Plus, it's easy to make mistakes in configuration (the more
> parameters, the more opportunities for mistakes.)  For example, your enabling
> scoring code expects a "1" (or similar) to be enabled whereas your enabling
> suggestions appearing code expected as "true".  I can easily imagine this
biting
> us at some point.  It's also the case that the current behavior is a problem
if
> we enable experimental scoring but don't, for example, specify one of your
> current parameters (we'd try using the base relevance of 0).
> 
> Nowadays I prefer the approach where the scoring code isn't gated on a
separate
> flag (experimental_scoring_enabled).  Instead, simply have a function
> GetPhysicalWebZeroSuggestBaseRelevance() that looks up the parameter directly
> and returns it, returning a default value if the parameter doesn't exist. 
This
> will also let you get rid of the experiment gating test, making the
> Get...Relevance function shorter (and more error-proof and maybe also easier
to
> read).  It'll also let you get rid of the default value in the physical web
> code.  (Always get the value from the field trial code, regardless.  Make that
> code have the default.)
> 
> This suggestion is optional.  I think it's cleaner.  Your approach is
> nonetheless like others in this file so if you insist on this way, that's okay
> too.

Agreed, this is cleaner and less error-prone. Done.

[Mark P, 2016-12-22 04:58:16]

lgtm
I'd like to re-review / re-glance at this  after you rebase on top of
https://codereview.chromium.org/2583763003/
after that's been reviewed and submitted.  I don't think it's likely I'll have
any new comments though.

--mark

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:554: if (!param_value.empty()
&& base::StringToInt(param_value, &base_relevance)) {
nit: {} not needed
ditto below

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:573: // when the user typed a
query in the omnibox.
nit: typed -> has typed
or perhaps
is typing

also, remove "a query"
(It doesn't add anything, and it may confuse because users may be typing a URL.)

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.h:447: static const char
kPhysicalWebExperimentalScoringParam[];
nit: no longer needed

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
File components/omnibox/browser/physical_web_provider.cc (right):

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:62: if
(input.from_omnibox_focus()) {
nit: {} unnecessary

[mattreynolds, 2017-01-04 01:04:03]

Thanks Mark, I'll ping you again when this is rebased.

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.cc (right):

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:554: if (!param_value.empty()
&& base::StringToInt(param_value, &base_relevance)) {
On 2016/12/22 04:58:16, Mark P (away thru Jan 2 2017) wrote:
> nit: {} not needed
> ditto below

Done.

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:573: // when the user typed a
query in the omnibox.
On 2016/12/22 04:58:16, Mark P (away thru Jan 2 2017) wrote:
> nit: typed -> has typed
> or perhaps
> is typing
> 
> also, remove "a query"
> (It doesn't add anything, and it may confuse because users may be typing a
URL.)

Done.

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
File components/omnibox/browser/omnibox_field_trial.h (right):

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.h:447: static const char
kPhysicalWebExperimentalScoringParam[];
On 2016/12/22 04:58:16, Mark P (away thru Jan 2 2017) wrote:
> nit: no longer needed

Done.

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
File components/omnibox/browser/physical_web_provider.cc (right):

https://codereview.chromium.org/2591053002/diff/80001/components/omnibox/brow...
components/omnibox/browser/physical_web_provider.cc:62: if
(input.from_omnibox_focus()) {
On 2016/12/22 04:58:16, Mark P (away thru Jan 2 2017) wrote:
> nit: {} unnecessary

Done.

[mattreynolds, 2017-01-06 18:50:42]

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

https://codereview.chromium.org/2591053002/diff/120001/components/omnibox/bro...
components/omnibox/browser/physical_web_provider.cc:39: }
git cl format ate the newline I added here, so I think this is considered
style-correct

[Mark P, 2017-01-06 19:55:03]

still lgtm

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

https://codereview.chromium.org/2591053002/diff/120001/components/omnibox/bro...
components/omnibox/browser/physical_web_provider.cc:39: }
On 2017/01/06 18:50:42, mattreynolds wrote:
> git cl format ate the newline I added here, so I think this is considered
> style-correct

Acknowledged.

[mattreynolds, 2017-01-06 19:56:46]

Hi Mark, this should be ready for another look.

[mattreynolds, 2017-01-06 19:57:28]

On 2017/01/06 19:55:03, Mark P wrote:
> still lgtm
> 
>
https://codereview.chromium.org/2591053002/diff/120001/components/omnibox/bro...
> File components/omnibox/browser/physical_web_provider.cc (right):
> 
>
https://codereview.chromium.org/2591053002/diff/120001/components/omnibox/bro...
> components/omnibox/browser/physical_web_provider.cc:39: }
> On 2017/01/06 18:50:42, mattreynolds wrote:
> > git cl format ate the newline I added here, so I think this is considered
> > style-correct
> 
> Acknowledged.

Too quick for me... Thanks again!

[mattreynolds, 2017-01-06 19:57:59]

The CQ bit was checked by mattreynolds@chromium.org

[commit-bot: I haz the power, 2017-01-06 19:58:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 20:23:58]

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

[commit-bot: I haz the power, 2017-01-06 20:23:59]

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

[mattreynolds, 2017-01-06 22:00:38]

On 2017/01/06 20:23:59, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
> (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Hi Mark, could you take another look? I needed to set the experimental params to
fix the unit tests.

[Mark P, 2017-01-06 22:32:54]

still lgtm ...

--mark

[mattreynolds, 2017-01-06 22:35:35]

The CQ bit was checked by mattreynolds@chromium.org

[commit-bot: I haz the power, 2017-01-06 22:36:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 23:46:22]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1483742135082730,
"parent_rev": "dad848c2408d0fc99ab2d4c1ada2e336f88c7b3c", "commit_rev":
"a04fff0b8cd393873fa9c1864a98525035519866"}

[commit-bot: I haz the power, 2017-01-06 23:46:51]

Description was changed from

==========
Show PhysicalWebProvider suggestions with omnibox input

Add support for "Query Suggest" in the Physical Web omnibox provider.
Previously, the omnibox would only display Physical Web omnibox suggestions
when the user had not yet entered any text in the omnibox ("Zero Suggest").
With this CL, Physical Web suggestions may be displayed if the URL and page
title match all the terms in the omnibox query string.

BUG=630769
==========

to

==========
Show PhysicalWebProvider suggestions with omnibox input

Add support for "Query Suggest" in the Physical Web omnibox provider.
Previously, the omnibox would only display Physical Web omnibox suggestions
when the user had not yet entered any text in the omnibox ("Zero Suggest").
With this CL, Physical Web suggestions may be displayed if the URL and page
title match all the terms in the omnibox query string.

BUG=630769

Review-Url: https://codereview.chromium.org/2591053002
Cr-Commit-Position: refs/heads/master@{#442098}
Committed:
https://chromium.googlesource.com/chromium/src/+/a04fff0b8cd393873fa9c1864a98...
==========

[commit-bot: I haz the power, 2017-01-06 23:46:52]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/a04fff0b8cd393873fa9c1864a98...