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

chrome/browser/search/hotword_service.cc

Index: chrome/browser/search/hotword_service.cc
diff --git a/chrome/browser/search/hotword_service.cc b/chrome/browser/search/hotword_service.cc
index b9f4bfd7d7452beab3561f312bb697f9998091c5..c36c655422e4fc13751cd25c7eb918fbfd736186 100644
--- a/chrome/browser/search/hotword_service.cc
+++ b/chrome/browser/search/hotword_service.cc
@@ -8,12 +8,15 @@
 #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"
@@ -94,9 +97,19 @@ HotwordService::HotwordService(Profile* profile)
       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() {
@@ -131,10 +144,10 @@ 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);
 
@@ -161,3 +174,39 @@ bool HotwordService::RetryHotwordExtension() {
   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();
+}