Omnibox - Warm Up PSuggest on Focus

Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest server (even before the user types anything).  This request will trigger the suggest server to load the per-user models into memory (if appropriate and necessary).  Thus, when the user actually starts typing, the models will be available to help provide personalized suggestions.

Guard this behavior using a field trial.

For these on-focus requests, the reply from the suggest server (if any) is ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For example, when creating a new tab page, we send a request, but then several SearchProvider::Start() calls happen with from_omnibox_focus=false, and that leads us to cancel the request so we never receive whatever response would've come.  This seems okay to me because we don't care about the reply--the important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search provider.  I.e., normal as-you-type logic applies.  The only possible negative consequence of this code is that the throttling to prevent queries to the suggest server from happening too often still applies.  (We don't send requests more often than once per 100ms.)  This means that if the user focuses and immediately starts typing, we might not send a request on the first keystroke.  This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of omnibox interactions involve focus + first keystroke is tapped within 100ms.  I imagine it's less on Android.  (We don't have metrics for Android.)  In any case, this is small enough that I'm not inclined to worry about it / come up with a new throttling strategy in the presence of this warm-up request in order to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that the focus code is getting called the same way on Windows as on Linux.  Thus, it works on Windows.

I also tested it on Android.

I didn't bother testing on Mac.  (Even if Mac doesn't handle focus events well, Mac will change soon enough with MacViews anywhere.)  It would not likely be worth the trouble to fix current Mac issues if there are any.

=== Testing Details ===

Here are the test cases I tried on Linux:
1. staring Chrome which opens a new tab page by default (which puts focus in the omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on Android, it loses all edits-in-progress.  There's no nothing about switching back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a attached, I think the omnibox gets focus by default on new tab pages.  Normally it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075
Review-Url: https://codereview.chromium.org/2717893002
Cr-Commit-Position: refs/heads/master@{#454419}
Committed: https://chromium.googlesource.com/chromium/src/+/e52518a3a6faa15b49d85a4eeb278300f4bcdaa1

[Mark P, 2017-02-25 23:02:49]

Description was changed from

==========
Omnibox - Warm Up PSuggest on Focus

BUG=676075
==========

to

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  In any case, this is small enough that I'm not
inclined to worry about it / come up with a new throttling strategy in the
presence of this warm-up request in order to avoid it.

This code is expected to work in general.  There are number of caveats however
because the omnibox does not currently send a on-focus request to the providers
under certain conditions.  On Linux these conditions include:
1. creating a new tab page (which puts focus in the omnibox by default)
2. starting Chrome and having the new tab page open by default
3. pressing control-k to put focus in the omnibox with a "Search google.com:"
chip
4. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.
5. having focus in the omnibox (possibly with text or without), switching tabs,
then switching back.
6. when on a real web page (not the NTP), having an edit in progress in the
omnibox (i.e., the omnibox is not the page's URL), focusing on the web content,
then focussing back in the omnibox.

I hope to fix #1 and #2 before submitting this.  (This is a pre-existing issue.)
 #5 and #6 almost work; the provider gets an on-focus call but doesn't send a
warm-up request.  (This is a bug with my code.)

The following work (i.e., send a warm-up request) on Linux:
1. when on a real web page (not the NTP) and an unmodified omnibox, with the web
content in focus, then focus in the omnibox via control-l or clicking.

I still need to test other platforms, especially Windows and Android.

BUG=676075
==========

[Mark P, 2017-02-27 20:47:48]

Description was changed from

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  In any case, this is small enough that I'm not
inclined to worry about it / come up with a new throttling strategy in the
presence of this warm-up request in order to avoid it.

This code is expected to work in general.  There are number of caveats however
because the omnibox does not currently send a on-focus request to the providers
under certain conditions.  On Linux these conditions include:
1. creating a new tab page (which puts focus in the omnibox by default)
2. starting Chrome and having the new tab page open by default
3. pressing control-k to put focus in the omnibox with a "Search google.com:"
chip
4. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.
5. having focus in the omnibox (possibly with text or without), switching tabs,
then switching back.
6. when on a real web page (not the NTP), having an edit in progress in the
omnibox (i.e., the omnibox is not the page's URL), focusing on the web content,
then focussing back in the omnibox.

I hope to fix #1 and #2 before submitting this.  (This is a pre-existing issue.)
 #5 and #6 almost work; the provider gets an on-focus call but doesn't send a
warm-up request.  (This is a bug with my code.)

The following work (i.e., send a warm-up request) on Linux:
1. when on a real web page (not the NTP) and an unmodified omnibox, with the web
content in focus, then focus in the omnibox via control-l or clicking.

I still need to test other platforms, especially Windows and Android.

BUG=676075
==========

to

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  In any case, this is small enough that I'm not
inclined to worry about it / come up with a new throttling strategy in the
presence of this warm-up request in order to avoid it.

This code is expected to work in general.  There are number of caveats however
because the omnibox does not currently send a on-focus request to the providers
under certain conditions.

On Linux these conditions include:
1. creating a new tab page (which puts focus in the omnibox by default)
2. starting Chrome and having the new tab page open by default
3. pressing control-k to put focus in the omnibox with a "Search google.com:"
chip
4. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.
5. having focus in the omnibox (possibly with text or without), switching tabs,
then switching back.
6. when on a real web page (not the NTP), having an edit in progress in the
omnibox (i.e., the omnibox is not the page's URL), focusing on the web content,
then focussing back in the omnibox.

I hope to fix #1 and #2 before submitting this.  (This is a pre-existing issue.)
 #5 and #6 almost work; the provider gets an on-focus call but doesn't send a
warm-up request.  (This is a bug with my code.)

The following work (i.e., send a warm-up request) on Linux:
1. when on a real web page (not the NTP) and an unmodified omnibox, with the web
content in focus, then focus in the omnibox via control-l or clicking.

On Android, everything seems to work correctly.  (Tested on a Nexus 7.)  I
didn't test a phone, though I do not expect anything different.  I also did not
test using an auxiliary keyboard.  (When one of those is a attached, I think the
omnibox gets focus by default on new tab pages.  Normally it does not.)  That's
such a minor case that I'm not worrying about it.

BUG=676075
==========

[Mark P, 2017-02-28 00:27:46]

Description was changed from

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  In any case, this is small enough that I'm not
inclined to worry about it / come up with a new throttling strategy in the
presence of this warm-up request in order to avoid it.

This code is expected to work in general.  There are number of caveats however
because the omnibox does not currently send a on-focus request to the providers
under certain conditions.

On Linux these conditions include:
1. creating a new tab page (which puts focus in the omnibox by default)
2. starting Chrome and having the new tab page open by default
3. pressing control-k to put focus in the omnibox with a "Search google.com:"
chip
4. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.
5. having focus in the omnibox (possibly with text or without), switching tabs,
then switching back.
6. when on a real web page (not the NTP), having an edit in progress in the
omnibox (i.e., the omnibox is not the page's URL), focusing on the web content,
then focussing back in the omnibox.

I hope to fix #1 and #2 before submitting this.  (This is a pre-existing issue.)
 #5 and #6 almost work; the provider gets an on-focus call but doesn't send a
warm-up request.  (This is a bug with my code.)

The following work (i.e., send a warm-up request) on Linux:
1. when on a real web page (not the NTP) and an unmodified omnibox, with the web
content in focus, then focus in the omnibox via control-l or clicking.

On Android, everything seems to work correctly.  (Tested on a Nexus 7.)  I
didn't test a phone, though I do not expect anything different.  I also did not
test using an auxiliary keyboard.  (When one of those is a attached, I think the
omnibox gets focus by default on new tab pages.  Normally it does not.)  That's
such a minor case that I'm not worrying about it.

BUG=676075
==========

to

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For
example, when creating a new tab page, we send a request, but then several
SearchProvider::Start() calls happen with from_omnibox_focus=false, and that
leads us to cancel the request so we never receive whatever response would've
come.  This seems okay to me because we don't care about the reply--the
important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  (We don't have metrics for Android.)  In any
case, this is small enough that I'm not inclined to worry about it / come up
with a new throttling strategy in the presence of this warm-up request in order
to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that
the focus code is getting called the same way on Windows as on Linux.  Thus, it
works on Linux.

I also tested it on Android.

Here are the test cases I tried:
1. staring Chrome which opens a new tab page by default (which puts focus in the
omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or
similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the
focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to
put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty
text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on
Android, it loses all edits-in-progress.  There's no nothing about switching
back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a
attached, I think the omnibox gets focus by default on new tab pages.  Normally
it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075
==========

[Mark P, 2017-02-28 00:35:49]

Description was changed from

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For
example, when creating a new tab page, we send a request, but then several
SearchProvider::Start() calls happen with from_omnibox_focus=false, and that
leads us to cancel the request so we never receive whatever response would've
come.  This seems okay to me because we don't care about the reply--the
important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  (We don't have metrics for Android.)  In any
case, this is small enough that I'm not inclined to worry about it / come up
with a new throttling strategy in the presence of this warm-up request in order
to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that
the focus code is getting called the same way on Windows as on Linux.  Thus, it
works on Linux.

I also tested it on Android.

Here are the test cases I tried:
1. staring Chrome which opens a new tab page by default (which puts focus in the
omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or
similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the
focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to
put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty
text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on
Android, it loses all edits-in-progress.  There's no nothing about switching
back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a
attached, I think the omnibox gets focus by default on new tab pages.  Normally
it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075
==========

to

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For
example, when creating a new tab page, we send a request, but then several
SearchProvider::Start() calls happen with from_omnibox_focus=false, and that
leads us to cancel the request so we never receive whatever response would've
come.  This seems okay to me because we don't care about the reply--the
important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  (We don't have metrics for Android.)  In any
case, this is small enough that I'm not inclined to worry about it / come up
with a new throttling strategy in the presence of this warm-up request in order
to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that
the focus code is getting called the same way on Windows as on Linux.  Thus, it
works on Windows.

I also tested it on Android.

I didn't bother testing on Mac.  (Even if Mac doesn't handle focus events well,
Mac will change soon enough with MacViews anywhere.)  It would not likely be
worth the trouble to fix current Mac issues if there are any.

=== Testing Details ===

Here are the test cases I tried on Linux:
1. staring Chrome which opens a new tab page by default (which puts focus in the
omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or
similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the
focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to
put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty
text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on
Android, it loses all edits-in-progress.  There's no nothing about switching
back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a
attached, I think the omnibox gets focus by default on new tab pages.  Normally
it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075
==========

[Mark P, 2017-02-28 00:41:14]

Description was changed from

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For
example, when creating a new tab page, we send a request, but then several
SearchProvider::Start() calls happen with from_omnibox_focus=false, and that
leads us to cancel the request so we never receive whatever response would've
come.  This seems okay to me because we don't care about the reply--the
important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  (We don't have metrics for Android.)  In any
case, this is small enough that I'm not inclined to worry about it / come up
with a new throttling strategy in the presence of this warm-up request in order
to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that
the focus code is getting called the same way on Windows as on Linux.  Thus, it
works on Windows.

I also tested it on Android.

I didn't bother testing on Mac.  (Even if Mac doesn't handle focus events well,
Mac will change soon enough with MacViews anywhere.)  It would not likely be
worth the trouble to fix current Mac issues if there are any.

=== Testing Details ===

Here are the test cases I tried on Linux:
1. staring Chrome which opens a new tab page by default (which puts focus in the
omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or
similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the
focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to
put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty
text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on
Android, it loses all edits-in-progress.  There's no nothing about switching
back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a
attached, I think the omnibox gets focus by default on new tab pages.  Normally
it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075
==========

to

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

Guard this behavior using a field trial.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For
example, when creating a new tab page, we send a request, but then several
SearchProvider::Start() calls happen with from_omnibox_focus=false, and that
leads us to cancel the request so we never receive whatever response would've
come.  This seems okay to me because we don't care about the reply--the
important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  (We don't have metrics for Android.)  In any
case, this is small enough that I'm not inclined to worry about it / come up
with a new throttling strategy in the presence of this warm-up request in order
to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that
the focus code is getting called the same way on Windows as on Linux.  Thus, it
works on Windows.

I also tested it on Android.

I didn't bother testing on Mac.  (Even if Mac doesn't handle focus events well,
Mac will change soon enough with MacViews anywhere.)  It would not likely be
worth the trouble to fix current Mac issues if there are any.

=== Testing Details ===

Here are the test cases I tried on Linux:
1. staring Chrome which opens a new tab page by default (which puts focus in the
omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or
similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the
focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to
put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty
text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on
Android, it loses all edits-in-progress.  There's no nothing about switching
back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a
attached, I think the omnibox gets focus by default on new tab pages.  Normally
it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075
==========

[Mark P, 2017-02-28 20:12:04]

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

[Mark P, 2017-02-28 20:12:05]

Hi Peter,

This is ready for review.  Can you please take a look?  I'm hoping to get this
polished and submitted by Friday.

thanks,
mark

[Peter Kasting, 2017-03-01 03:00:35]

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
File chrome/browser/autocomplete/search_provider_unittest.cc (right):

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:3551: // Make sure the
default providers suggest service was queried.
Nit: provider's

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:3556: // (though the
provider should now we done).
Nit: we -> be

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:3558:
fetcher->SetResponseString("[\"\",[\"a\", \"b\"],[],[],{}]");
Nit: Might be more readable with a raw string literal:

  fetcher->SetResponseString(R"(["",["a", "b"],[],[],{}])");

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

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:43: // in memory allows the
suggest server to respond more question with
Nit: more question -> to more questions

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

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:277: // typing).
Can we have had requests in-flight before this?  Going out of focus should have
closed the dropdown, which should have stopped all providers, which should have
stopped all requests, no?

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:301: // inputs.
Nit: Maybe second clause should be "these inputs should only be used to warm up
the suggest server".

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:410: // Ignore (i.e, don't
display) any suggestions for on-focus inputs.
Nit: , -> .

Might want to say why we should ignore these.

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:516: UpdateDone();
Nit: Maybe most of this function should be a helper so this could just be:

{
  // On-focus inputs display no suggestions, so we need not enforce
  // inlineability and default match constraints.  (BTW, does doing so actually
  // break stuff?  If so say what.  Maybe the thing we have to avoid is the
  // PersistTopSuggestions() calls?  Maybe those also make the name I choose
  // below incorrect?)
  if (!input_.from_omnibox_focus())
    EnforceConstraints();

  UpdateDone();
}

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:761: // isn't sent to the server.
Nit: While here: This comment seems like it belongs as part of a declaration
comment or something, not here.

And the comment below starts with "Next", which looks like there used to be code
here but now there isn't?  Would be nice to fix this stuff up.

[Mark P, 2017-03-01 20:02:33]

Please take another look, thanks.

--mark

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
File chrome/browser/autocomplete/search_provider_unittest.cc (right):

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:3551: // Make sure the
default providers suggest service was queried.
On 2017/03/01 03:00:34, Peter Kasting wrote:
> Nit: provider's

Done.

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:3556: // (though the
provider should now we done).
On 2017/03/01 03:00:34, Peter Kasting wrote:
> Nit: we -> be

Done.

https://codereview.chromium.org/2717893002/diff/80001/chrome/browser/autocomp...
chrome/browser/autocomplete/search_provider_unittest.cc:3558:
fetcher->SetResponseString("[\"\",[\"a\", \"b\"],[],[],{}]");
On 2017/03/01 03:00:34, Peter Kasting wrote:
> Nit: Might be more readable with a raw string literal:
> 
>   fetcher->SetResponseString(R"(["",["a", "b"],[],[],{}])");

Indeed, it is.  I didn't know those existed. :-)

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

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/omnibox_field_trial.cc:43: // in memory allows the
suggest server to respond more question with
On 2017/03/01 03:00:34, Peter Kasting wrote:
> Nit: more question -> to more questions

Actually, I intended "quickly". Done.

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

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:277: // typing).
On 2017/03/01 03:00:35, Peter Kasting wrote:
> Can we have had requests in-flight before this?  Going out of focus should
have
> closed the dropdown, which should have stopped all providers, which should
have
> stopped all requests, no?

Probably...?

But there are so many edge cases.  E.g., control-l to focus, dropdown does not
focus, click on web content, control-l to focus.  Do we cancel requests on loss
of focus even if the dropdown was not open?  Revised the comment; please the
StopSuggest() call.

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:301: // inputs.
On 2017/03/01 03:00:35, Peter Kasting wrote:
> Nit: Maybe second clause should be "these inputs should only be used to warm
up
> the suggest server".

Okay.  Done.

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:410: // Ignore (i.e, don't
display) any suggestions for on-focus inputs.
On 2017/03/01 03:00:35, Peter Kasting wrote:
> Nit: , -> .

Did -> .,

> Might want to say why we should ignore these.

Done, though I'm not sure how much light my new comment adds.

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:516: UpdateDone();
On 2017/03/01 03:00:35, Peter Kasting wrote:
> Nit: Maybe most of this function should be a helper so this could just be:
> 
> {
>   // On-focus inputs display no suggestions, so we need not enforce
>   // inlineability and default match constraints.  (BTW, does doing so
actually
>   // break stuff?  If so say what.  Maybe the thing we have to avoid is the
>   // PersistTopSuggestions() calls?  Maybe those also make the name I choose
>   // below incorrect?)
>   if (!input_.from_omnibox_focus())
>     EnforceConstraints();
> 
>   UpdateDone();
> }

Refactored to make this cleaner.

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:761: // isn't sent to the server.
On 2017/03/01 03:00:35, Peter Kasting wrote:
> Nit: While here: This comment seems like it belongs as part of a declaration
> comment or something, not here.

Done.  (Moved a slightly revised version of the comment to the header.)

> And the comment below starts with "Next", which looks like there used to be
code
> here but now there isn't?  Would be nice to fix this stuff up.

Removed "Next".

[Mark P, 2017-03-02 05:43:52]

If you lgtm this and are willing to accept additional "cleanup" in a later
changelist, please CQ it.

thanks,
mark

[Mark P, 2017-03-02 05:43:58]

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

[commit-bot: I haz the power, 2017-03-02 05:44:35]

Dry run: 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-03-02 05:47:57]

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

[commit-bot: I haz the power, 2017-03-02 05:47:58]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mark P, 2017-03-02 05:57:46]

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

[commit-bot: I haz the power, 2017-03-02 05:58:41]

Dry run: 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-03-02 06:04:05]

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

[commit-bot: I haz the power, 2017-03-02 06:04:06]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Mark P, 2017-03-02 06:18:46]

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

[commit-bot: I haz the power, 2017-03-02 06:19:35]

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

[Mark P, 2017-03-02 06:44:42]

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

[commit-bot: I haz the power, 2017-03-02 06:45:33]

Dry run: 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-03-02 08:11:37]

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

[commit-bot: I haz the power, 2017-03-02 08:11:38]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-03-02 22:50:43]

CQing for now, followup can happen in other CLs.

LGTM

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

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:277: // typing).
On 2017/03/01 20:02:32, Mark P wrote:
> On 2017/03/01 03:00:35, Peter Kasting wrote:
> > Can we have had requests in-flight before this?  Going out of focus should
> have
> > closed the dropdown, which should have stopped all providers, which should
> have
> > stopped all requests, no?
> 
> Probably...?
> 
> But there are so many edge cases.  E.g., control-l to focus, dropdown does not
> focus, click on web content, control-l to focus.  Do we cancel requests on
loss
> of focus even if the dropdown was not open?

Yes.  Otherwise, the dropdown could pop open later when the not-stopped requests
return results back.  So I think this is an invariant that we cannot have
requests in flight here.

I would aim to remove this and the accompanying comment bits.

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
File components/omnibox/browser/search_provider.cc (right):

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.cc:275: // requests here (there
likely aren't yet, though it doesn't hurt to be safe
Nit: Missing paren

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
File components/omnibox/browser/search_provider.h (right):

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.h:199: // Check constraints that may
be violated by suggested relevances and revises/
Nit: Check -> Checks

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.h:200: // rolls back the suggested
relevance scores to make all constraints old.
Nit: old -> hold

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.h:203: // Record the top suggestion
(if any) for future use.  SearchProvider tries
Nit: Record -> Records

[Peter Kasting, 2017-03-02 22:50:48]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2017-03-02 22:51:22]

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-03-02 22:58:55]

CQ is committing da patch.

Bot data: {"patchset_id": 190009, "attempt_start_ts": 1488495048252040,
"parent_rev": "21f26e967e50357e2e251249a51b37920dff6236", "commit_rev":
"e52518a3a6faa15b49d85a4eeb278300f4bcdaa1"}

[commit-bot: I haz the power, 2017-03-02 22:59:41]

Description was changed from

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

Guard this behavior using a field trial.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For
example, when creating a new tab page, we send a request, but then several
SearchProvider::Start() calls happen with from_omnibox_focus=false, and that
leads us to cancel the request so we never receive whatever response would've
come.  This seems okay to me because we don't care about the reply--the
important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  (We don't have metrics for Android.)  In any
case, this is small enough that I'm not inclined to worry about it / come up
with a new throttling strategy in the presence of this warm-up request in order
to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that
the focus code is getting called the same way on Windows as on Linux.  Thus, it
works on Windows.

I also tested it on Android.

I didn't bother testing on Mac.  (Even if Mac doesn't handle focus events well,
Mac will change soon enough with MacViews anywhere.)  It would not likely be
worth the trouble to fix current Mac issues if there are any.

=== Testing Details ===

Here are the test cases I tried on Linux:
1. staring Chrome which opens a new tab page by default (which puts focus in the
omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or
similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the
focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to
put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty
text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on
Android, it loses all edits-in-progress.  There's no nothing about switching
back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a
attached, I think the omnibox gets focus by default on new tab pages.  Normally
it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075
==========

to

==========
Omnibox - Warm Up PSuggest on Focus

When the omnibox gains focus, have SearchProvider send a request to the suggest
server (even before the user types anything).  This request will trigger the
suggest server to load the per-user models into memory (if appropriate and
necessary).  Thus, when the user actually starts typing, the models will be
available to help provide personalized suggestions.

Guard this behavior using a field trial.

For these on-focus requests, the reply from the suggest server (if any) is
ignored.  Sometimes the omnibox code will cancel a request-in-progress.  For
example, when creating a new tab page, we send a request, but then several
SearchProvider::Start() calls happen with from_omnibox_focus=false, and that
leads us to cancel the request so we never receive whatever response would've
come.  This seems okay to me because we don't care about the reply--the
important thing is that we sent the request.

This warm-up request should not affect the regular behavior of the search
provider.  I.e., normal as-you-type logic applies.  The only possible negative
consequence of this code is that the throttling to prevent queries to the
suggest server from happening too often still applies.  (We don't send requests
more often than once per 100ms.)  This means that if the user focuses and
immediately starts typing, we might not send a request on the first keystroke. 
This will affect ~2% of omnibox interactions on Windows.  That is, only 2% of
omnibox interactions involve focus + first keystroke is tapped within 100ms.  I
imagine it's less on Android.  (We don't have metrics for Android.)  In any
case, this is small enough that I'm not inclined to worry about it / come up
with a new throttling strategy in the presence of this warm-up request in order
to avoid it.

This code is expected to work in general.

I tested it on Linux.

Also, sky@ has confirmed (both via code understanding and experimentally) that
the focus code is getting called the same way on Windows as on Linux.  Thus, it
works on Windows.

I also tested it on Android.

I didn't bother testing on Mac.  (Even if Mac doesn't handle focus events well,
Mac will change soon enough with MacViews anywhere.)  It would not likely be
worth the trouble to fix current Mac issues if there are any.

=== Testing Details ===

Here are the test cases I tried on Linux:
1. staring Chrome which opens a new tab page by default (which puts focus in the
omnibox by default).
2. when focus is on page content (not in the omnibox!), press control-t (or
similar) to create a new tab (which puts focus in the omnibox by default).
3. when focus is on the page content, clicking or pressing control-l to put the
focus in the omnibox.
4. when focus is on page content (not in the omnibox!), pressing control-k to
put focus in the omnibox with a "Search google.com:" chip.
5. having an omnibox edit-in-progress (possibly with some text or with empty
text), switching tabs, then switching back.

It doesn't work in this case:
1. having an omnibox edit-in-progress, putting focus in the page content, then
pressing control-l or tapping to put focus back in the omnibox.


On Android, I tested the following (on a Nexus 7):
1. on the new tab page, tap to focus the omnibox
2. on the new tab page, tap to focus the box on the page (a.k.a. the "fakebox")
3. on an arbitrary web page, tap to focus the omnibox

Some other tests aren't applicable because when the omnibox loses focus on
Android, it loses all edits-in-progress.  There's no nothing about switching
back to an omnibox or a tab with an omnibox with edits-in-progress.

I didn't test a phone, though I do not expect anything different.

I also did not test using an auxiliary keyboard.  (When one of those is a
attached, I think the omnibox gets focus by default on new tab pages.  Normally
it does not.)  That's such a minor case that I'm not worrying about it.

BUG=676075

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

[commit-bot: I haz the power, 2017-03-02 22:59:42]

Committed patchset #11 (id:190009) as
https://chromium.googlesource.com/chromium/src/+/e52518a3a6faa15b49d85a4eeb27...

[Mark P, 2017-03-03 22:12:25]

Doing follow-up in
https://codereview.chromium.org/2725333004/

--mark

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

https://codereview.chromium.org/2717893002/diff/80001/components/omnibox/brow...
components/omnibox/browser/search_provider.cc:277: // typing).
On 2017/03/02 22:50:42, Peter Kasting wrote:
> On 2017/03/01 20:02:32, Mark P wrote:
> > On 2017/03/01 03:00:35, Peter Kasting wrote:
> > > Can we have had requests in-flight before this?  Going out of focus should
> > have
> > > closed the dropdown, which should have stopped all providers, which should
> > have
> > > stopped all requests, no?
> > 
> > Probably...?
> > 
> > But there are so many edge cases.  E.g., control-l to focus, dropdown does
not
> > focus, click on web content, control-l to focus.  Do we cancel requests on
> loss
> > of focus even if the dropdown was not open?
> 
> Yes.  Otherwise, the dropdown could pop open later when the not-stopped
requests
> return results back.  So I think this is an invariant that we cannot have
> requests in flight here.
> 
> I would aim to remove this and the accompanying comment bits.

Removed it and removed comment.  Added DCHECK to verify your claim.

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
File components/omnibox/browser/search_provider.cc (right):

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.cc:275: // requests here (there
likely aren't yet, though it doesn't hurt to be safe
On 2017/03/02 22:50:42, Peter Kasting wrote:
> Nit: Missing paren

Now moot.

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
File components/omnibox/browser/search_provider.h (right):

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.h:199: // Check constraints that may
be violated by suggested relevances and revises/
On 2017/03/02 22:50:42, Peter Kasting wrote:
> Nit: Check -> Checks

Done.

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.h:200: // rolls back the suggested
relevance scores to make all constraints old.
On 2017/03/02 22:50:42, Peter Kasting wrote:
> Nit: old -> hold

Done.

https://codereview.chromium.org/2717893002/diff/190009/components/omnibox/bro...
components/omnibox/browser/search_provider.h:203: // Record the top suggestion
(if any) for future use.  SearchProvider tries
On 2017/03/02 22:50:42, Peter Kasting wrote:
> Nit: Record -> Records

Done.