[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"
@@ skipping 43 matching lines @@
     availability_state = PENDING_DOWNLOAD;
   } else if (!service->IsExtensionEnabled(
       extension_misc::kHotwordExtensionId)) {
     availability_state = DISABLED_EXTENSION;
   }
   UMA_HISTOGRAM_ENUMERATION("Hotword.HotwordExtensionAvailability",
                             availability_state,
                             NUM_HOTWORD_EXTENSION_AVAILABILITY_METRICS);
 }
 
+ExtensionService* GetExtensionService(Profile* profile) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  extensions::ExtensionSystem* extension_system =
+      extensions::ExtensionSystem::Get(profile);
+  if (extension_system)
+    return extension_system->extension_service();
+  return NULL;
+}
+
 }  // namespace
 
 namespace hotword_internal {
 // Constants for the hotword field trial.
 const char kHotwordFieldTrialName[] = "VoiceTrigger";
 const char kHotwordFieldTrialDisabledGroupName[] = "Disabled";
 }  // namespace hotword_internal
 
 // static
 bool HotwordService::DoesHotwordSupportLanguage(Profile* profile) {
@@ skipping 13 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())
+  ExtensionService* extension_service = GetExtensionService(profile_);
+  if (!extension_service)
     return false;
-  ExtensionService* extension_service = extension_system->extension_service();
 
   extension_service->ReloadExtension(extension_misc::kHotwordExtensionId);
   return true;
 }
+
+void HotwordService::DisableHotwordExtension() {

[miket_OOO, 2014/03/17 20:10:51]

Curious: why not pass in extension_service as a parameter to these two methods?
That's the final version of the refactoring that I had in mind. If it's
impossible for some reason, then leave it. But it seems like it's doable.

[rpetterson, 2014/03/17 21:14:29]

Fixed. I didn't originally pass it in because I still have to call the
GetExtensionService(..) function twice -- once here and once where I call it on
line 110 above.

+  ExtensionService* extension_service = GetExtensionService(profile_);
+  if (extension_service) {
+    extension_service->DisableExtension(
+        extension_misc::kHotwordExtensionId,
+        extensions::Extension::DISABLE_USER_ACTION);
+  }
+}
+
+void HotwordService::EnableHotwordExtension() {
+  ExtensionService* extension_service = GetExtensionService(profile_);
+  if (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();
+  else
+    DisableHotwordExtension();
+}