Hotword Audio Verification App: Adds to hotwordPrivate API

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 "base/command_line.h"
 #include "base/i18n/case_conversion.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
@@ skipping 434 matching lines @@
     ExtensionService* extension_service) {
   if (extension_service) {
     extension_service->DisableExtension(
         extension_misc::kHotwordExtensionId,
         extensions::Extension::DISABLE_USER_ACTION);
   }
 }
 
 void HotwordService::OnHotwordAlwaysOnSearchEnabledChanged(
     const std::string& pref_name) {
-  DCHECK_EQ(pref_name, std::string(prefs::kHotwordAlwaysOnSearchEnabled));
+    DCHECK_EQ(pref_name, std::string(prefs::kHotwordAlwaysOnSearchEnabled));

[benwells, 2014/09/09 20:57:30]

The indenting was correct before.

[kcarattini, 2014/09/10 08:14:28]

Done.

 
   ExtensionService* extension_service = GetExtensionService(profile_);
   if (!extension_service)
     return;
 
   const extensions::Extension* extension = extension_service->GetExtensionById(
       extension_misc::kHotwordAudioVerificationAppId, true);
   if (!extension)
     return;
 
   if (profile_->GetPrefs()->GetBoolean(prefs::kHotwordAlwaysOnSearchEnabled)) {
+    // TODO(kcarattini): Check if the app is already running. If so, bring it

[benwells, 2014/09/09 20:57:31]

This TODO is valid but should be in the app itself, as that's where you'd do
this. That is, you always send onLaunched. In the app's onLaunched handler it
can detect if it has a window visible and can focus the window if it does.

[kcarattini, 2014/09/10 08:14:28]

Done.

+    // to focus rather than launching it.
+    SetHotwordAudioVerificationLaunchState(pref_name);
     OpenApplication(AppLaunchParams(
         profile_, extension, extensions::LAUNCH_CONTAINER_WINDOW, NEW_WINDOW));
   }
 }
 
+void HotwordService::SetHotwordAudioVerificationLaunchState(
+    const std::string& pref_name) {
+  if (pref_name == std::string(prefs::kHotwordAlwaysOnSearchEnabled)) {
+    if (profile_->GetPrefs()->GetBoolean(prefs::kHotwordAudioLoggingEnabled)) {
+      hotword_audio_verification_launch_state_ = HOTWORD_ONLY;
+    } else {
+      hotword_audio_verification_launch_state_ = HOTWORD_AND_AUDIO_HISTORY;
+    }
+  } else if (pref_name == std::string(prefs::kHotwordAudioLoggingEnabled)){

[rpetterson, 2014/09/09 20:07:32]

Does this function (SetHotwordAudioVerificationLaunchState) get called
elsewhere? If not, then this condition will never be met since above we DCHECK
for pref_name == std::string(prefs::kHotwordAlwaysOnSearchEnabled.

[benwells, 2014/09/09 20:57:31]

I think it would be clearer if this function didn't exist, but the caller just
set hotword_audio_verification_launch_state to whatever it thought was
necessary. It is confusing setting this value based on a pref_name.

[kcarattini, 2014/09/10 08:14:28]

Yes, it will be called when the user retrains the model, and from anywhere we
might put a promo link.

The logic I had here doesn't make sense given the latest mocks, so I am removing
all of this and adding a TODO to follow up in a separate cl.

[kcarattini, 2014/09/10 08:14:28]

Refactored this. Please let me know if it makes more sense. The caller of the
launch function might be external to hotword_service.

+    hotword_audio_verification_launch_state_ = AUDIO_HISTORY_ONLY;
+  }
+}
+
+HotwordService::LaunchMode
+HotwordService::GetHotwordAudioVerificationLaunchState() {
+  return hotword_audio_verification_launch_state_;
+}
+
 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 35 matching lines @@
     return false;
 
   std::string previous_locale =
       profile_->GetPrefs()->GetString(prefs::kHotwordPreviousLanguage);
   std::string locale = GetCurrentLocale(profile_);
 
   // If it's a new locale, then the old extension should be uninstalled.
   return locale != previous_locale &&
       HotwordService::DoesHotwordSupportLanguage(profile_);
 }