App list voice searches now prioritize exact-match web results.

App list voice searches now prioritize exact-match web results.

This means that if you do a voice search, the first result will always
be a web search for exactly what you said (not some other result like an
app, or a search from your history). So you can now use the voice search
interface as a direct way to send queries to the web search.

BUG=436665
TBR=mukai@chromium.org

Committed: https://crrev.com/403458264d79f23f3b4e6b613ccabb654ebee499
Cr-Commit-Position: refs/heads/master@{#308518}

[Matt Giuca, 2014-12-15 02:43:00]

mgiuca@chromium.org changed reviewers:
+ calamity@chromium.org

[Matt Giuca, 2014-12-15 02:43:00]

OK so I looked into writing tests. There are three separate pieces of
functionality here which I think could be tested:
1. The AppListViewDelegate (which sets the voice query flag if it is a voice
query).
2. The OmniboxResult (which sets the voice result flag if it is an exact match
web search).
3. The Mixer (which promotes voice results if it is a voice query).

Unfortunately, I was only able to test the third one, because the first two do
not have any test suite set up at all. Do you think it's worth creating test
suites for both of them? In the case of OmniboxResult, this will also mean
splitting out the OmniboxResult class into its own file (it is currently a
private class of omnibox_provider.cc).

[calamity, 2014-12-16 01:50:04]

Maybe you can write an AppListViewUnittest that walks through the entire process
of doing a voice search.

https://codereview.chromium.org/796293002/diff/40001/chrome/browser/ui/app_li...
File chrome/browser/ui/app_list/app_list_view_delegate.h (right):

https://codereview.chromium.org/796293002/diff/40001/chrome/browser/ui/app_li...
chrome/browser/ui/app_list/app_list_view_delegate.h:172: // Determines whether
the current search was initiate by speech.
s/initiate/initiated/

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/app_list_mod...
File ui/app_list/app_list_model.h (right):

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/app_list_mod...
ui/app_list/app_list_model.h:197: scoped_ptr<SearchResult> auto_launch_result_;
Unused?

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/search_result.h
File ui/app_list/search_result.h (right):

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/search_resul...
ui/app_list/search_result.h:110: bool voice_result() const { return
voice_result_; }
So do we want to change this as discussed yesterday into voice_promoted (with
appropriate comment change)? I'm happy for you to do either.

[Matt Giuca, 2014-12-16 02:04:09]

Will do the test in a follow-up.

https://codereview.chromium.org/796293002/diff/40001/chrome/browser/ui/app_li...
File chrome/browser/ui/app_list/app_list_view_delegate.h (right):

https://codereview.chromium.org/796293002/diff/40001/chrome/browser/ui/app_li...
chrome/browser/ui/app_list/app_list_view_delegate.h:172: // Determines whether
the current search was initiate by speech.
On 2014/12/16 01:50:03, calamity wrote:
> s/initiate/initiated/

Done.

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/app_list_mod...
File ui/app_list/app_list_model.h (right):

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/app_list_mod...
ui/app_list/app_list_model.h:197: scoped_ptr<SearchResult> auto_launch_result_;
On 2014/12/16 01:50:03, calamity wrote:
> Unused?

Done.

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/search_result.h
File ui/app_list/search_result.h (right):

https://codereview.chromium.org/796293002/diff/40001/ui/app_list/search_resul...
ui/app_list/search_result.h:110: bool voice_result() const { return
voice_result_; }
As discussed, I think after the changes that the provider decides what is and
isn't included in particular types of search, it once again makes sense to call
this a "voice result" and keep its original semantics of "results that can be
automatically selected by a voice query".

[calamity, 2014-12-16 02:09:23]

lgtm

[Matt Giuca, 2014-12-16 02:11:55]

TBR mukai for mechanical athena change.

[Matt Giuca, 2014-12-16 02:12:25]

mgiuca@chromium.org changed reviewers:
+ mukai@chromium.org

[Matt Giuca, 2014-12-16 02:12:25]

Actually adding mukai@.

[Matt Giuca, 2014-12-16 02:12:31]

The CQ bit was checked by mgiuca@chromium.org

[Jun Mukai, 2014-12-16 02:13:30]

lgtm

[commit-bot: I haz the power, 2014-12-16 02:13:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/796293002/120001

[commit-bot: I haz the power, 2014-12-16 03:08:53]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2014-12-16 03:10:19]

Patchset 7 (id:??) landed as
https://crrev.com/403458264d79f23f3b4e6b613ccabb654ebee499
Cr-Commit-Position: refs/heads/master@{#308518}