[Hotword] Ok Google options disappear if search provider is changed from

chrome/browser/search/hotword_service.cc

 // Copyright 2013 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/search/hotword_service.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/i18n/case_conversion.h"
@@ skipping 725 matching lines @@
 
 HotwordAudioHistoryHandler* HotwordService::GetAudioHistoryHandler() {
   return audio_history_handler_.get();
 }
 
 void HotwordService::SetAudioHistoryHandler(
     HotwordAudioHistoryHandler* handler) {
   audio_history_handler_.reset(handler);
 }
 
+void HotwordService::DisableHotwordPreferences() {
+  if (IsSometimesOnEnabled()) {

[Anand Mistry (off Chromium), 2014/12/05 03:56:51]

This will also disable hotwording if the user navigates to google.com. Is this
really what we want? From the user's perspective, it's understandable that
hotwording isn't going to work on non-google searches, but if they explicitly go
to google.com, why wouldn't that work?

[rpetterson, 2014/12/05 04:57:33]

So if I understand correctly, you're saying only make it disappear for
always-on, but not sometimes on? I think that's reasonable as well.

+somas so he can chime in

[Anand Mistry (off Chromium), 2014/12/05 05:23:52]

I think it's more nuanced than that. I think it should be disabled for the
launcher search, which means always-on and the launcher integration with
sometimes-on. NTP doesn't matter since you don't get an in-page search box. But
a user can visit google.com.

But now this is getting complicated, so disabling everything is a good
compromise (hence the lgtm). I'm just asking what the UX should be in an ideal
world based on reasonable user expectations.

+    if (profile_->GetPrefs()->HasPrefPath(prefs::kHotwordSearchEnabled))
+      profile_->GetPrefs()->SetBoolean(prefs::kHotwordSearchEnabled, false);
+  } else if (IsAlwaysOnEnabled()) {
+    if (profile_->GetPrefs()->HasPrefPath(
+            prefs::kHotwordAlwaysOnSearchEnabled)) {
+      profile_->GetPrefs()->SetBoolean(prefs::kHotwordAlwaysOnSearchEnabled,
+                                       false);
+    }
+  }
+}
+
 void HotwordService::OnHotwordSearchEnabledChanged(
     const std::string& pref_name) {
   DCHECK_EQ(pref_name, std::string(prefs::kHotwordSearchEnabled));
 
   ExtensionService* extension_service = GetExtensionService(profile_);
   if (profile_->GetPrefs()->GetBoolean(prefs::kHotwordSearchEnabled))
     EnableHotwordExtension(extension_service);
   else
     DisableHotwordExtension(extension_service);
 }
@@ skipping 58 matching lines @@
   HotwordPrivateEventService* event_service =
       BrowserContextKeyedAPIFactory<HotwordPrivateEventService>::Get(profile_);
   // "enabled" isn't being changed, but piggy-back off the notification anyway.
   if (event_service)
     event_service->OnEnabledChanged(prefs::kHotwordSearchEnabled);
 }
 
 bool HotwordService::UserIsActive() {
   return profile_ == ProfileManager::GetActiveUserProfile();
 }