[Hotword] Uninstall and reinstall the extension upon language change.

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/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/pending_extension_manager.h"
+#include "chrome/browser/extensions/updater/extension_updater.h"
+#include "chrome/browser/extensions/webstore_startup_installer.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/hotword_service_factory.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/common/webplugininfo.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/one_shot_event.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 // The whole file relies on the extension systems but this file is built on
 // some non-extension supported platforms and including an API header will cause
 // a compile error since it depends on header files generated by .idl.
 // TODO(mukai): clean up file dependencies and remove this clause.
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/api/hotword_private/hotword_private_api.h"
 #endif
@@ skipping 101 matching lines @@
 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;
 }
 
+// static
+std::string GetCurrentLocale(Profile* profile) {
+  std::string locale =
+#if defined(OS_CHROMEOS)
+      // On ChromeOS locale is per-profile.
+      profile->GetPrefs()->GetString(prefs::kApplicationLocale);
+#else
+      g_browser_process->GetApplicationLocale();
+#endif
+  return locale;
+}
+
 }  // namespace
 
 namespace hotword_internal {
 // Constants for the hotword field trial.
 const char kHotwordFieldTrialName[] = "VoiceTrigger";
 const char kHotwordFieldTrialDisabledGroupName[] = "Disabled";
 // Old preference constant.
 const char kHotwordUnusablePrefName[] = "hotword.search_enabled";
 }  // namespace hotword_internal
 
 // static
 bool HotwordService::DoesHotwordSupportLanguage(Profile* profile) {
-  std::string locale =
-#if defined(OS_CHROMEOS)
-      // On ChromeOS locale is per-profile.
-      profile->GetPrefs()->GetString(prefs::kApplicationLocale);
-#else
-      g_browser_process->GetApplicationLocale();
-#endif
-  std::string normalized_locale = l10n_util::NormalizeLocale(locale);
+  std::string normalized_locale =
+      l10n_util::NormalizeLocale(GetCurrentLocale(profile));
   StringToLowerASCII(&normalized_locale);
 
   for (size_t i = 0; i < arraysize(kSupportedLocales); i++) {
     if (normalized_locale.compare(0, 2, kSupportedLocales[i]) == 0)
       return true;
   }
   return false;
 }
 
 HotwordService::HotwordService(Profile* profile)
     : profile_(profile),
+      extension_registry_observer_(this),
       client_(NULL),
-      error_message_(0) {
+      error_message_(0),
+      uninstall_pending_(false),
+      weak_factory_(this) {
+  extension_registry_observer_.Add(extensions::ExtensionRegistry::Get(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. However, this should only be done if auto-install
     // is enabled which is gated through the IsHotwordAllowed check.
     if (IsHotwordAllowed())
       DisableHotwordExtension(GetExtensionService(profile_));
   }
   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)));
 
   registrar_.Add(this,
-                 chrome::NOTIFICATION_EXTENSION_INSTALLED_DEPRECATED,
-                 content::Source<Profile>(profile_));
-  registrar_.Add(this,
                  chrome::NOTIFICATION_BROWSER_WINDOW_READY,
                  content::NotificationService::AllSources());
 
+  extensions::ExtensionSystem::Get(profile_)->ready().Post(
+      FROM_HERE,
+      base::Bind(base::IgnoreResult(
+          &HotwordService::MaybeUninstallHotwordExtension),
+                 weak_factory_.GetWeakPtr()));
+
   // Clear the old user pref because it became unusable.
   // TODO(rlp): Remove this code per crbug.com/358789.
   if (profile_->GetPrefs()->HasPrefPath(
           hotword_internal::kHotwordUnusablePrefName)) {
     profile_->GetPrefs()->ClearPref(hotword_internal::kHotwordUnusablePrefName);
   }
 }
 
 HotwordService::~HotwordService() {
 }
 
 void HotwordService::Observe(int type,
                              const content::NotificationSource& source,
                              const content::NotificationDetails& details) {
-  if (type == chrome::NOTIFICATION_EXTENSION_INSTALLED_DEPRECATED) {
-    const extensions::Extension* extension =
-        content::Details<const extensions::InstalledExtensionInfo>(details)
-              ->extension;
-    // Disabling the extension automatically on install should only occur
-    // if the user is in the field trial for auto-install which is gated
-    // by the IsHotwordAllowed check.
-    if (IsHotwordAllowed() &&
-        extension->id() == extension_misc::kHotwordExtensionId &&
-        !profile_->GetPrefs()->GetBoolean(prefs::kHotwordSearchEnabled)) {
-      DisableHotwordExtension(GetExtensionService(profile_));
-      // Once the extension is disabled, it will not be enabled until the
-      // user opts in at which point the pref registrar will take over
-      // enabling and disabling.
-      registrar_.Remove(this,
-                        chrome::NOTIFICATION_EXTENSION_INSTALLED_DEPRECATED,
-                        content::Source<Profile>(profile_));
-    }
-  } else if (type == chrome::NOTIFICATION_BROWSER_WINDOW_READY) {
+  if (type == chrome::NOTIFICATION_BROWSER_WINDOW_READY) {
     // The microphone monitor must be initialized as the page is loading
     // so that the state of the microphone is available when the page
     // loads. The Ok Google Hotword setting will display an error if there
     // is no microphone but this information will not be up-to-date unless
     // the monitor had already been started. Furthermore, the pop up to
     // opt in to hotwording won't be available if it thinks there is no
     // microphone. There is no hard guarantee that the monitor will actually
     // be up by the time it's needed, but this is the best we can do without
     // starting it at start up which slows down start up too much.
     // The content/media for microphone uses the same observer design and
     // makes use of the same audio device monitor.
     HotwordServiceFactory::GetInstance()->UpdateMicrophoneState();
   }
 }
 
+void HotwordService::OnExtensionUninstalled(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  if (extension->id() != extension_misc::kHotwordExtensionId ||
+      profile_ != static_cast<Profile*>(browser_context) ||

[Yoyo Zhou, 2014/06/18 00:33:21]

Profile::FromBrowserContext

[rpetterson, 2014/06/18 02:08:44]

Done.

+      !GetExtensionService(profile_))
+    return;
+
+  // Verify that the extension was uninstalled because of a locale mismatch.
+  // If that's the case, reinstall and reset the saved locale.
+  if (!ShouldUninstallHotwordExtension())

[Yoyo Zhou, 2014/06/18 00:33:21]

Does uninstall_pending_ serve as well here?

[rpetterson, 2014/06/18 02:08:44]

You mean just replace the check ShouldUninstallHotwordExtension() with
uninstall_pending_? Yes, that should work since uninstall_pending_ should be set
if we've decided an uninstall is needed.

I've updated it.

+    return;
+
+  InstallHotwordExtensionFromWebstore();
+  SetPreviousLocalePref();
+}
+
+void HotwordService::InstallHotwordExtensionFromWebstore() {
+  extensions::WebstoreStandaloneInstaller::Callback callback =
+      base::Bind(&HotwordService::OnHotwordExtensionInstalled,
+                 base::Unretained(this));
+  installer_ = new extensions::WebstoreStartupInstaller(

[Yoyo Zhou, 2014/06/18 00:33:21]

I'm not confident that this is the right choice of installer (there are too many
to choose from =( ), but if it installs silently without prompting the user,
then I think that's what you need.

[rpetterson, 2014/06/18 02:08:44]

This is the same one used by ChromeOS to install the hotword extension for the
app_list which is why I used it since it's already been in production in a
similar context. It does silently install without prompting the user from what I
can tell.

+      extension_misc::kHotwordExtensionId,
+      profile_,
+      false,
+      callback);
+  installer_->BeginInstall();
+}
+
+void HotwordService::OnExtensionInstalled(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension) {
+
+  if (extension->id() != extension_misc::kHotwordExtensionId ||
+      profile_ != static_cast<Profile*>(browser_context))
+    return;
+
+  // If the previous locale pref has never been set, set it now since
+  // the extension has been installed.
+  if (!profile_->GetPrefs()->HasPrefPath(prefs::kHotwordPreviousLanguage))
+    SetPreviousLocalePref();
+
+  // If MaybeUninstallHotwordExtension already triggered an uninstall, we
+  // don't want to loop and trigger another uninstall-install cycle.
+  // However, if we arrived her via an uninstall-triggered-install (and in
+  // that case |uninstall_pending_| will be true) then we know install
+  // has completed and we can reset |uninstall_pending_|.
+  if (!uninstall_pending_)

[Yoyo Zhou, 2014/06/18 00:33:21]

What do you think about calling this reinstall_pending_ (that would make more
sense in the context here), and MaybeReinstallHotwordExtension?

It also should be possible to put most of this reinstalling logic into a
separate helper class if HotwordService is getting unwieldy. Up to you.

[rpetterson, 2014/06/18 02:08:44]

sgtm. done.
I'd rather do that as a separate CL for now. I've added a tracking bug for it:
crbug.com/385969

+    MaybeUninstallHotwordExtension();
+  else
+    uninstall_pending_ = false;
+
+  // Now that the extension is installed, if the user has not selected
+  // the preference on, make sure it is turned off.
+  //
+  // Disabling the extension automatically on install should only occur
+  // if the user is in the field trial for auto-install which is gated
+  // by the IsHotwordAllowed check. The check for IsHotwordAllowed() here
+  // can be removed once it's known that few people have manually
+  // installed extension.
+  if (IsHotwordAllowed() &&

[Yoyo Zhou, 2014/06/18 00:33:21]

This check/disable looks repeated from the constructor.

[rpetterson, 2014/06/18 02:08:44]

Yes, but we have to repeat it because the HotwordService constructor can finish
before the external component loader finishes installing the extensions. In that
case, we catch it here and disable it. 

+      !profile_->GetPrefs()->GetBoolean(prefs::kHotwordSearchEnabled)) {
+    DisableHotwordExtension(GetExtensionService(profile_));
+  }
+}
+
+bool HotwordService::MaybeUninstallHotwordExtension() {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  ExtensionService* extension_service = GetExtensionService(profile_);
+  if (!extension_service)
+    return false;
+
+  const extensions::Extension* extension = extension_service->GetExtensionById(
+      extension_misc::kHotwordExtensionId, true);
+  if (!extension)
+    return false;
+
+  // If the extension is currently pending, return and we'll check again
+  // after the install is finished.
+  extensions::PendingExtensionManager* pending_manager =
+      extension_service->pending_extension_manager();
+  if (pending_manager->IsIdPending(extension->id()))
+    return false;
+
+  // If there is already a pending request from HotwordService, don't try
+  // to uninstall either.
+  if (uninstall_pending_)
+    return false;
+
+  // Check if the current locale matches the previous. If they don't match,
+  // uninstall the extension.
+  if (!ShouldUninstallHotwordExtension())
+    return false;
+
+  uninstall_pending_ = true;
+  return UninstallHotwordExtension(extension_service);
+}
+
+bool HotwordService::UninstallHotwordExtension(
+    ExtensionService* extension_service) {
+  base::string16 error;
+  if (!extension_service->UninstallExtension(
+          extension_misc::kHotwordExtensionId, true, &error)) {
+    LOG(WARNING) << "Cannot uninstall extension with id "
+                 << extension_misc::kHotwordExtensionId
+                 << ": " << error;
+    return false;
+  }
+  return true;
+}
+
 bool HotwordService::IsServiceAvailable() {
   error_message_ = 0;
 
   // Determine if the extension is available.
   extensions::ExtensionSystem* system =
       extensions::ExtensionSystem::Get(profile_);
   ExtensionService* service = system->extension_service();
   // Include disabled extensions (true parameter) since it may not be enabled
   // if the user opted out.
   const extensions::Extension* extension =
@@ skipping 94 matching lines @@
 
   DCHECK(client_ == client);
 
   client_ = NULL;
   HotwordPrivateEventService* event_service =
       BrowserContextKeyedAPIFactory<HotwordPrivateEventService>::Get(profile_);
   if (event_service)
     event_service->OnHotwordSessionStopped();
 #endif
 }
+
+void HotwordService::SetPreviousLocalePref() {
+  profile_->GetPrefs()->SetString(prefs::kHotwordPreviousLanguage,
+                                  GetCurrentLocale(profile_));
+}
+
+bool HotwordService::ShouldUninstallHotwordExtension() {
+  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.
+  // If there is no previous locale pref, then this is the first install
+  // so no need to uninstall first.
+  return locale != previous_locale &&
+      profile_->GetPrefs()->HasPrefPath(prefs::kHotwordPreviousLanguage) &&

[Yoyo Zhou, 2014/06/18 00:33:21]

nit: seems like the no-previous-locale check should come at the beginning of
this function (though I think it works either way)

[rpetterson, 2014/06/18 02:08:44]

Done.

+      HotwordService::DoesHotwordSupportLanguage(profile_);
+}
+
+void HotwordService::OnHotwordExtensionInstalled(bool success,

[Yoyo Zhou, 2014/06/18 00:33:21]

This callback doesn't really do anything. Is it needed?

[rpetterson, 2014/06/18 02:08:44]

I agree. Removed.

+                                                 const std::string& error) {
+  if (!success)
+    LOG(WARNING) << "Cannot install extension with id "
+                 << extension_misc::kHotwordExtensionId
+                 << ": " << error;
+}