[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

[Jered, 2014/06/18 15:04:19]

delete this comment

[rpetterson, 2014/06/18 20:48:33]

Done.

+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),
+      reinstall_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::MaybeReinstallHotwordExtension),
+                 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_ != Profile::FromBrowserContext(browser_context) ||
+      !GetExtensionService(profile_))
+    return;
+
+  // If the extension wasn't uninstalled due to language change, don't try to
+  // reinstall it.
+  if (!reinstall_pending_)
+    return;
+
+  InstallHotwordExtensionFromWebstore();
+  SetPreviousLocalePref();
+}
+
+void HotwordService::InstallHotwordExtensionFromWebstore() {
+  installer_ = new extensions::WebstoreStartupInstaller(
+      extension_misc::kHotwordExtensionId,
+      profile_,
+      false,
+      extensions::WebstoreStandaloneInstaller::Callback());
+  installer_->BeginInstall();
+}
+
+void HotwordService::OnExtensionInstalled(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension) {
+
+  if (extension->id() != extension_misc::kHotwordExtensionId ||
+      profile_ != Profile::FromBrowserContext(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 MaybeReinstallHotwordExtension 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

[Jered, 2014/06/18 15:04:19]

her -> here

[rpetterson, 2014/06/18 20:48:33]

Done.

+  // that case |reinstall_pending_| will be true) then we know install
+  // has completed and we can reset |reinstall_pending_|.
+  if (!reinstall_pending_)
+    MaybeReinstallHotwordExtension();

[Jered, 2014/06/18 15:04:19]

Huh? Why would we ever want to reinstall the hotword extension just after it got
installed?

[Yoyo Zhou, 2014/06/18 19:10:31]

I believe this is for the scenario where the locale changes and the extension
updates at the same time. We hit the IsIdPending at startup, so we wait for the
update to finish. When it has done so, we get here.

[rpetterson, 2014/06/18 20:48:33]

That's correct. Typically, no, we would not. If we've just installed it, chances
are good that we installed it in the correct language. However, this callback is
also triggered when the extension has been installed from an update. So here's a
hypothetical situation:

- Select new language
- Restart Chrome to let new language take effect
- Upon Restart, external component loader decides it's time to update the
extensions
- Hotword has an update so we download the new one
- Meanwhile, the extension system says it's ready so we do a check
- Our correct language check tells us that the hotword extension is pending
(because it's downloading) so we'll wait until the OnExtensionInstalled trigger
- OnExtensionInstalled triggers and we realize that the updated extension is the
wrong language
- Proceed to uninstall and reinstall in the correct language

Would it be simpler to do this check before the external component loader does
it's updates so that we don't download the extension twice sometimes? Yes.
However, that requires more complicated hooks into the extension system.
Eventually the extension folks plan to build in forced updates which will serve
our purpose as well, but until then this is less invasive. We're covering edge
cases with this check that should rarely happen so extra downloads in general
shouldn't be an issue.

[Jered, 2014/06/18 21:06:04]

Ok, thanks for explaining.

+  else
+    reinstall_pending_ = false;

[Jered, 2014/06/18 15:04:19]

Just unconditionally set this to false.

[Yoyo Zhou, 2014/06/18 19:10:31]

I don't think that works with the previous.

[rpetterson, 2014/06/18 20:48:33]

I thought about this, but it needs to be in an else. Otherwise it might be set
to false before the OnExtensionUninstalled triggers which puts it to false
inside that call and that would be incorrect (I've seen this in manual testing).
Only after we know the reinstall has completed should it be set back to false.

I'm also not sure it's "better" since without the else we go through all the
ShouldReinstallHotwordExtension checks after the extension is reinstalled rather
than just checking the one boolean.

[Jered, 2014/06/18 21:06:04]

sgtm.

+
+  // 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() &&
+      !profile_->GetPrefs()->GetBoolean(prefs::kHotwordSearchEnabled)) {
+    DisableHotwordExtension(GetExtensionService(profile_));
+  }
+}
+
+bool HotwordService::MaybeReinstallHotwordExtension() {
+  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 (reinstall_pending_)
+    return false;
+
+  // Check if the current locale matches the previous. If they don't match,
+  // uninstall the extension.
+  if (!ShouldReinstallHotwordExtension())
+    return false;
+
+  reinstall_pending_ = true;

[Jered, 2014/06/18 15:04:19]

Should this be set to the return value of UninstallHotwordExtension, and then be
returned?

[Yoyo Zhou, 2014/06/18 19:10:31]

That seems right.

[rpetterson, 2014/06/18 20:48:33]

Sorta. I still need to set reinstall_pending_ to true before the call to
UninstallHotwordExtension. Otherwise it's still false when we get into
OnExtensionUninstalled and we never reinstall since it might be the result of a
different kind of uninstall. But I can set the ultimate value of
reinstall_pending_ to be the return value of UninstallHotwordExtension and
return that.

+  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() {

[Jered, 2014/06/18 15:04:19]

nit: The pref is called previous language, so should this be previous language
rather than previous locale?

[rpetterson, 2014/06/18 20:48:33]

Done.

+  profile_->GetPrefs()->SetString(prefs::kHotwordPreviousLanguage,
+                                  GetCurrentLocale(profile_));
+}
+
+bool HotwordService::ShouldReinstallHotwordExtension() {
+  // If there is no previous locale pref, then this is the first install
+  // so no need to uninstall first.
+  if (!profile_->GetPrefs()->HasPrefPath(prefs::kHotwordPreviousLanguage))
+    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_);
+}