Optimization: not sending answer server requests for voice queries.

chrome/browser/ui/app_list/app_list_view_delegate.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/app_list/app_list_view_delegate.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 263 matching lines @@
 app_list::SpeechUIModel* AppListViewDelegate::GetSpeechUI() {
   return speech_ui_.get();
 }
 
 void AppListViewDelegate::StartSearch() {
   if (search_controller_) {
     search_controller_->Start(is_voice_query_);
     controller_->OnSearchStarted();
   }
   if (search_answer_delegate_)
-    search_answer_delegate_->Update();
+    search_answer_delegate_->Update(is_voice_query_);
 }
 
 void AppListViewDelegate::StopSearch() {
   if (search_controller_)
     search_controller_->Stop();
 }
 
 void AppListViewDelegate::OpenSearchResult(
     app_list::SearchResult* result,
     bool auto_launch,
     int event_flags) {
   if (auto_launch)
     base::RecordAction(base::UserMetricsAction("AppList_AutoLaunched"));
   search_controller_->OpenResult(result, event_flags);
   is_voice_query_ = false;
+  model_->search_box()->SetText(base::string16());

[xiyuan, 2017/04/27 21:59:30]

Is this necessary? This would trigger another StartSearch call because query is
changed.

Can we optimize by comparing whether request url is actually changed in
SearchAnswerWebContentsDelegate::Update() ?

[vadimt, 2017/04/27 22:14:37]

This is a fundamentally correct thing to do.
We should not find ourselves in a state when there is a non-empty query as-if
entered from keyboard.

This won't cause extra queries to the answer server because it doesn't send
queries for an empty text.

Also, I'd be surprised if we sent empty strings to autocomplete server (even
though I didn't thoroughly verify this).

[xiyuan, 2017/04/27 22:29:20]

It will cause searches in other search providers. E.g. AppSearchProvider would
re-index and generate a recommendation list.

Not this CL's fault, but maybe |is_voice_query_| should be part of
SearchboxModel instead of a member in AppListViewDelegate. Then we can have a
ClearVoiceSearch flag to clear the flag with the query without notifying model
observers to avoid unnecessary searches. If we want to clear the query, maybe we
should do that.

[vadimt, 2017/04/29 00:48:28]

Done.
Moved is_voice_query_ from AppListViewDelegate to SearchBoxModel.
Switched to a lazy model where the query doesn’t get cleared (which would cause
unnecessary searches) after a search even if the launcher closes. It will be
cleared and trigger a search when launcher reopens.
Fixed a bug when the launcher started search for the old query string upon
reopening.
Added more optimizations.
At the end, I traced expensive search operations during reopening, keyboard and
voice searches, and now they are invoked exactly when and as many times as
needed.

 }
 
 void AppListViewDelegate::InvokeSearchResultAction(
     app_list::SearchResult* result,
     int action_index,
     int event_flags) {
   search_controller_->InvokeResultAction(result, action_index, event_flags);
 }
 
 base::TimeDelta AppListViewDelegate::GetAutoLaunchTimeout() {
   return auto_launch_timeout_;
 }
 
 void AppListViewDelegate::AutoLaunchCanceled() {
   if (is_voice_query_) {
     base::RecordAction(base::UserMetricsAction("AppList_AutoLaunchCanceled"));
     // Cancelling the auto launch means we are no longer in a voice query.
     is_voice_query_ = false;
+    model_->search_box()->SetText(base::string16());
   }
   auto_launch_timeout_ = base::TimeDelta();
 }
 
 void AppListViewDelegate::ViewInitialized() {
   app_list::StartPageService* service =
       app_list::StartPageService::Get(profile_);
   if (service) {
     service->AppListShown();
     if (service->HotwordEnabled()) {
@@ skipping 196 matching lines @@
   TemplateURLService* template_url_service =
       TemplateURLServiceFactory::GetForProfile(profile_);
   const TemplateURL* default_provider =
       template_url_service->GetDefaultSearchProvider();
   bool is_google =
       default_provider->GetEngineType(
           template_url_service->search_terms_data()) ==
       SEARCH_ENGINE_GOOGLE;
 
   model_->SetSearchEngineIsGoogle(is_google);
-  search_answer_delegate_->Update();
+  search_answer_delegate_->Update(is_voice_query_);
 
   app_list::StartPageService* start_page_service =
       app_list::StartPageService::Get(profile_);
   if (start_page_service)
     start_page_service->set_search_engine_is_google(is_google);
 }
 
 void AppListViewDelegate::Observe(int type,
                                   const content::NotificationSource& source,
                                   const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_APP_TERMINATING, type);
 
   SetProfile(nullptr);  // Ensures launcher page web contents are torn down.
 }