[Hotword] Making enabling/disabling the setting enable/disable the hotword extension.

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/i18n/case_conversion.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
+#include "base/threading/worker_pool.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/api/hotword_private/hotword_private_api.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
+#include "extensions/browser/browser_context_keyed_api_factory.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/extension.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace {
 const int kMaxTimesToShowOptInPopup = 10;
 
 // Enum describing the state of the hotword preference.
 // This is used for UMA stats -- do not reorder or delete items; only add to
 // the end.
@@ skipping 60 matching lines @@
 HotwordService::HotwordService(Profile* profile)
     : profile_(profile) {
   // This will be called during profile initialization which is a good time
   // to check the user's hotword state.
   HotwordEnabled enabled_state = UNSET;
   if (profile_->GetPrefs()->HasPrefPath(prefs::kHotwordSearchEnabled)) {
     if (profile_->GetPrefs()->GetBoolean(prefs::kHotwordSearchEnabled))
       enabled_state = ENABLED;
     else
       enabled_state = DISABLED;
+  } else {
+    // If the preference has not been set the hotword extension should
+    // not be running.
+    DisableHotwordExtension();
   }
   UMA_HISTOGRAM_ENUMERATION("Hotword.Enabled", enabled_state,
                             NUM_HOTWORD_ENABLED_METRICS);
+
+  pref_registrar_.Init(profile_->GetPrefs());
+  pref_registrar_.Add(
+      prefs::kHotwordSearchEnabled,
+      base::Bind(&HotwordService::OnHotwordSearchEnabledChanged,
+                 base::Unretained(this)));
 }
 
 HotwordService::~HotwordService() {
 }
 
 bool HotwordService::ShouldShowOptInPopup() {
   if (profile_->IsOffTheRecord())
     return false;
 
   // Profile is not off the record.
@@ skipping 14 matching lines @@
       prefs::kHotwordOptInPopupTimesShown);
   profile_->GetPrefs()->SetInteger(prefs::kHotwordOptInPopupTimesShown,
                                    ++number_shown);
   // TODO(rlp): actually show opt in popup when linked up to extension.
 }
 
 bool HotwordService::IsServiceAvailable() {
   extensions::ExtensionSystem* system =
       extensions::ExtensionSystem::Get(profile_);
   ExtensionService* service = system->extension_service();
-  // Do not include disabled extension (false parameter) because if the
-  // extension is disabled, it's not available.
+  // Include disabled extensions (true parameter) since it may not be enabled
+  // if the user opted out.
   const extensions::Extension* extension =
-      service->GetExtensionById(extension_misc::kHotwordExtensionId, false);
+      service->GetExtensionById(extension_misc::kHotwordExtensionId, true);
 
   RecordAvailabilityMetrics(service, extension);
 
   return extension && IsHotwordAllowed();
 }
 
 bool HotwordService::IsHotwordAllowed() {
   std::string group = base::FieldTrialList::FindFullName(
       hotword_internal::kHotwordFieldTrialName);
   return !group.empty() &&
       group != hotword_internal::kHotwordFieldTrialDisabledGroupName &&
       DoesHotwordSupportLanguage(profile_);
 }
 
 bool HotwordService::RetryHotwordExtension() {
   CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   extensions::ExtensionSystem* extension_system =
       extensions::ExtensionSystem::Get(profile_);
   if (!extension_system || !extension_system->extension_service())
     return false;
   ExtensionService* extension_service = extension_system->extension_service();
 
   extension_service->ReloadExtension(extension_misc::kHotwordExtensionId);
   return true;
 }
+
+void HotwordService::DisableHotwordExtension() {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  extensions::ExtensionSystem* extension_system =
+      extensions::ExtensionSystem::Get(profile_);
+  if (!extension_system || !extension_system->extension_service())
+    return;
+  ExtensionService* extension_service = extension_system->extension_service();
+
+  extension_service->DisableExtension(
+      extension_misc::kHotwordExtensionId,
+      extensions::Extension::DISABLE_USER_ACTION);

[miket_OOO, 2014/03/12 21:54:46]

I agree this isn't sufficiently distinct from a typical user-initiated disable
action to enumerate it separately.

+}
+
+void HotwordService::EnableHotwordExtension() {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  extensions::ExtensionSystem* extension_system =
+      extensions::ExtensionSystem::Get(profile_);
+  if (!extension_system || !extension_system->extension_service())
+    return;
+  ExtensionService* extension_service = extension_system->extension_service();
+
+  extension_service->EnableExtension(extension_misc::kHotwordExtensionId);
+}
+
+void HotwordService::OnHotwordSearchEnabledChanged(
+    const std::string& pref_name) {
+  DCHECK_EQ(pref_name, std::string(prefs::kHotwordSearchEnabled));
+
+  if (profile_->GetPrefs()->GetBoolean(prefs::kHotwordSearchEnabled))
+    EnableHotwordExtension();

[miket_OOO, 2014/03/12 21:54:46]

It would be way cooler to refactor the work to grab ExtensionService into this
method. Otherwise it's needless duplication above.

[rpetterson, 2014/03/12 23:27:32]

Done. But factored into a separate method since DisableHotwordExtension is used
in the constructor as well. But I was also able to pull it out of
RetryHotwordExtension so cleaner overall.

+  else
+    DisableHotwordExtension();
+}