system-web-apps: sync system-web-apps with locale

SystemWebAppManager::NeedsUpdate() will check whether current locale is
consistent with the locale that system web apps were installed with. If
not (i.e. when user changes it), request to reinstall apps in order to
keep their launcher localization (title, icons, etc.) synchronized to
current locale.

- Add a preference kSystemWebAppLastInstalledLocale to store locale at
  installation time.
- Check install results of apps before storing locale

Fixes: 1020442
Change-Id: I16f99a81fb5a06a634078914ff4fd9d745f5f73c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1898887Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712907}

chrome/browser/web_applications/extensions/system_web_app_manager_unittest.cc

@@ -312,6 +312,46 @@ TEST_F(SystemWebAppManagerTest, UpdateOnVersionChange) {
   EXPECT_TRUE(IsInstalled(kAppUrl3));
 }
 
+TEST_F(SystemWebAppManagerTest, UpdateOnLocaleChange) {
+  const std::vector<ExternalInstallOptions>& install_requests =
+      pending_app_manager()->install_requests();
+
+  system_web_app_manager()->SetUpdatePolicy(
+      SystemWebAppManager::UpdatePolicy::kOnVersionChange);
+
+  base::flat_map<SystemAppType, SystemAppInfo> system_apps;
+  system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
+  system_web_app_manager()->SetSystemApps(system_apps);
+
+  // Simulate first execution.
+  pending_app_manager()->SetInstallResultCode(
+      InstallResultCode::kSuccessNewInstall);
+  system_web_app_manager()->set_current_locale("en-US");
+  system_web_app_manager()->Start();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(1u, install_requests.size());
+  EXPECT_TRUE(IsInstalled(kAppUrl1));
+
+  // Change locale setting, should trigger reinstall.
+  pending_app_manager()->SetInstallResultCode(
+      InstallResultCode::kSuccessNewInstall);
+  system_web_app_manager()->set_current_locale("ja");
+  system_web_app_manager()->Start();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(2u, install_requests.size());
+  EXPECT_TRUE(install_requests[1].force_reinstall);
+  EXPECT_TRUE(IsInstalled(kAppUrl1));
+
+  // Do not reinstall because locale is not changed.
+  system_web_app_manager()->Start();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(3u, install_requests.size());
+  EXPECT_FALSE(install_requests[2].force_reinstall);
+}
+
 TEST_F(SystemWebAppManagerTest, InstallResultHistogram) {
   base::HistogramTester histograms;
   {

chrome/browser/web_applications/system_web_app_manager.cc

@@ -14,6 +14,7 @@
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/version.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
@@ -243,18 +244,27 @@ bool SystemWebAppManager::IsEnabled() {
 void SystemWebAppManager::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterStringPref(prefs::kSystemWebAppLastUpdateVersion, "");
+  registry->RegisterStringPref(prefs::kSystemWebAppLastInstalledLocale, "");
 }
 
 const base::Version& SystemWebAppManager::CurrentVersion() const {
   return version_info::GetVersion();
 }
 
+const std::string& SystemWebAppManager::CurrentLocale() const {
+  return g_browser_process->GetApplicationLocale();
+}
+
 void SystemWebAppManager::OnAppsSynchronized(
     std::map<GURL, InstallResultCode> install_results,
     std::map<GURL, bool> uninstall_results) {
   if (IsEnabled()) {
+    // TODO(qjw): Figure out where install_results come from, decide if
+    // installation failures need to be handled
     pref_service_->SetString(prefs::kSystemWebAppLastUpdateVersion,
                              CurrentVersion().GetString());
+    pref_service_->SetString(prefs::kSystemWebAppLastInstalledLocale,
+                             CurrentLocale());
   }
 
   RecordExternalAppInstallResultCode(kInstallResultHistogramName,
@@ -280,10 +290,20 @@ bool SystemWebAppManager::NeedsUpdate() const {
 
   base::Version last_update_version(
       pref_service_->GetString(prefs::kSystemWebAppLastUpdateVersion));
-  // This also updates if the version rolls back for some reason to ensure that
-  // the System Web Apps are always in sync with the Chrome version.
-  return !last_update_version.IsValid() ||
-         last_update_version != CurrentVersion();
+
+  const std::string& last_installed_locale(
+      pref_service_->GetString(prefs::kSystemWebAppLastInstalledLocale));
+
+  // If Chrome version rolls back for some reason, ensure System Web Apps are
+  // always in sync with Chrome version.
+  bool versionIsDifferent =
+      !last_update_version.IsValid() || last_update_version != CurrentVersion();
+
+  // If system language changes, ensure System Web Apps launcher localization
+  // are in sync with current language.
+  bool localeIsDifferent = last_installed_locale != CurrentLocale();
+
+  return versionIsDifferent || localeIsDifferent;
 }
 
 }  // namespace web_app

chrome/browser/web_applications/system_web_app_manager.h

@@ -129,6 +129,7 @@ class SystemWebAppManager {
 
  protected:
   virtual const base::Version& CurrentVersion() const;
+  virtual const std::string& CurrentLocale() const;
 
  private:
   void OnAppsSynchronized(std::map<GURL, InstallResultCode> install_results,

chrome/browser/web_applications/test/test_system_web_app_manager.cc

@@ -30,4 +30,8 @@ const base::Version& TestSystemWebAppManager::CurrentVersion() const {
   return current_version_;
 }
 
+const std::string& TestSystemWebAppManager::CurrentLocale() const {
+  return current_locale_;
+}
+
 }  // namespace web_app

chrome/browser/web_applications/test/test_system_web_app_manager.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_SYSTEM_WEB_APP_MANAGER_H_
 #define CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_SYSTEM_WEB_APP_MANAGER_H_
 
+#include <string>
 #include <vector>
 
 #include "base/macros.h"
@@ -29,11 +30,17 @@ class TestSystemWebAppManager : public SystemWebAppManager {
     current_version_ = version;
   }
 
+  void set_current_locale(const std::string& locale) {
+    current_locale_ = locale;
+  }
+
   // SystemWebAppManager:
   const base::Version& CurrentVersion() const override;
+  const std::string& CurrentLocale() const override;
 
  private:
   base::Version current_version_{"0.0.0.0"};
+  std::string current_locale_;
 
   DISALLOW_COPY_AND_ASSIGN(TestSystemWebAppManager);
 };

chrome/common/pref_names.cc

@@ -1783,6 +1783,11 @@ const char kWebAppsExtensionIDs[] = "web_apps.extension_ids";
 const char kSystemWebAppLastUpdateVersion[] =
     "web_apps.system_web_app_last_update";
 
+// A string representing the last locale that System Web Apps were installed in.
+// This is used to refresh System Web Apps i18n when the locale is changed.
+const char kSystemWebAppLastInstalledLocale[] =
+    "web_apps.system_web_app_last_installed_language";
+
 // The default audio capture device used by the Media content setting.
 const char kDefaultAudioCaptureDevice[] = "media.default_audio_capture_device";
 

chrome/common/pref_names.h

@@ -608,6 +608,7 @@ extern const char kWebAppInstallMetrics[];
 
 extern const char kWebAppsExtensionIDs[];
 extern const char kSystemWebAppLastUpdateVersion[];
+extern const char kSystemWebAppLastInstalledLocale[];
 
 extern const char kDefaultAudioCaptureDevice[];
 extern const char kDefaultVideoCaptureDevice[];