Make BrowserProcess::GetApplicationLocale thread safe and migrate

callers of l10n_util::GetApplicationLocale to use this instead.

In the browser process, it's wrong to call
l10n_util::GetApplicationLocale with an empty string because then
it won't consider the user pref value when resolving the locale.

On Linux, it's also wrong to call l10n_util::GetApplicationLocale
after startup because the call touches disk and on Linux, we assume
that all of the program files can be deleted after startup (so
updates in place can work).

Review URL: http://codereview.chromium.org/476002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34206 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 52 insertions, 105 deletions

diff --git a/app/l10n_util.h b/app/l10n_util.h
index 4675156..0c65934 100644
--- a/app/l10n_util.h
+++ b/app/l10n_util.h
@@ -27,7 +27,7 @@
 #include "unicode/uchar.h"
 
 #if defined(OS_MACOSX)
-#include "app/l10n_util.h"
+#include "app/l10n_util_mac.h"
 #endif  // OS_MACOSX
 
 class FilePath;
diff --git a/app/resource_bundle.cc b/app/resource_bundle.cc
index e3174d8..4c13e01 100644
--- a/app/resource_bundle.cc
+++ b/app/resource_bundle.cc
@@ -31,11 +31,12 @@ const SkColor ResourceBundle::toolbar_separator_color =
      SkColorSetRGB(182, 186, 192);
 
 /* static */
-void ResourceBundle::InitSharedInstance(const std::wstring& pref_locale) {
+std::string ResourceBundle::InitSharedInstance(
+    const std::wstring& pref_locale) {
   DCHECK(g_shared_instance_ == NULL) << "ResourceBundle initialized twice";
   g_shared_instance_ = new ResourceBundle();
 
-  g_shared_instance_->LoadResources(pref_locale);
+  return g_shared_instance_->LoadResources(pref_locale);
 }
 
 /* static */
diff --git a/app/resource_bundle.h b/app/resource_bundle.h
index 550fb72..0273494 100644
--- a/app/resource_bundle.h
+++ b/app/resource_bundle.h
@@ -63,10 +63,11 @@ class ResourceBundle {
     LargeFont,
   };
 
-  // Initialize the ResourceBundle for this process.
+  // Initialize the ResourceBundle for this process.  Returns the language
+  // selected.
   // NOTE: Mac ignores this and always loads up resources for the language
   // defined by the Cocoa UI (ie-NSBundle does the langange work).
-  static void InitSharedInstance(const std::wstring& pref_locale);
+  static std::string InitSharedInstance(const std::wstring& pref_locale);
 
   // Delete the ResourceBundle for this process if it exists.
   static void CleanupSharedInstance();
@@ -169,8 +170,8 @@ class ResourceBundle {
 #endif
 
   // Try to load the main resources and the locale specific strings from an
-  // external data module.
-  void LoadResources(const std::wstring& pref_locale);
+  // external data module.  Returns the locale that is loaded.
+  std::string LoadResources(const std::wstring& pref_locale);
 
   // Initialize all the gfx::Font members if they haven't yet been initialized.
   void LoadFontsIfNecessary();
@@ -183,7 +184,7 @@ class ResourceBundle {
 
   // Returns the full pathname of the locale file to load.  May return an empty
   // string if no locale data files are found.
-  static FilePath GetLocaleFilePath(const std::wstring& pref_locale);
+  static FilePath GetLocaleFilePath(const std::string& app_locale);
 
   // Returns a handle to bytes from the resource |module|, without doing any
   // processing or interpretation of the resource. Returns whether we
diff --git a/app/resource_bundle_dummy.cc b/app/resource_bundle_dummy.cc
index 77479f6..1dd65ed 100644
--- a/app/resource_bundle_dummy.cc
+++ b/app/resource_bundle_dummy.cc
@@ -24,9 +24,11 @@ namespace gfx {
 
 
 /* static */
-void ResourceBundle::InitSharedInstance(const std::wstring& pref_locale) {
+std::string ResourceBundle::InitSharedInstance(
+    const std::wstring& pref_locale) {
   DCHECK(g_shared_instance_ == NULL) << "ResourceBundle initialized twice";
   g_shared_instance_ = new ResourceBundle();
+  return std::string();
 }
 
 /* static */
diff --git a/app/resource_bundle_linux.cc b/app/resource_bundle_linux.cc
index e8c53d3..95d5ceb 100644
--- a/app/resource_bundle_linux.cc
+++ b/app/resource_bundle_linux.cc
@@ -74,12 +74,11 @@ FilePath ResourceBundle::GetResourcesFilePath() {
 }
 
 // static
-FilePath ResourceBundle::GetLocaleFilePath(const std::wstring& pref_locale) {
+FilePath ResourceBundle::GetLocaleFilePath(const std::string& app_locale) {
   FilePath locale_file_path;
   PathService::Get(app::DIR_LOCALES, &locale_file_path);
   if (locale_file_path.empty())
     return locale_file_path;
-  const std::string app_locale = l10n_util::GetApplicationLocale(pref_locale);
   if (app_locale.empty())
     return FilePath();
   locale_file_path = locale_file_path.AppendASCII(app_locale + ".pak");
diff --git a/app/resource_bundle_mac.mm b/app/resource_bundle_mac.mm
index 94ef31a..9d14440 100644
--- a/app/resource_bundle_mac.mm
+++ b/app/resource_bundle_mac.mm
@@ -29,8 +29,8 @@ FilePath ResourceBundle::GetResourcesFilePath() {
 }
 
 // static
-FilePath ResourceBundle::GetLocaleFilePath(const std::wstring& pref_locale) {
-  DLOG_IF(WARNING, !pref_locale.empty())
+FilePath ResourceBundle::GetLocaleFilePath(const std::string& app_locale) {
+  DLOG_IF(WARNING, !app_locale.empty())
       << "ignoring requested locale in favor of NSBundle's selection";
   return GetResourcesPakFilePath(@"locale");
 }
diff --git a/app/resource_bundle_posix.cc b/app/resource_bundle_posix.cc
index ec6893a..044bc3d 100644
--- a/app/resource_bundle_posix.cc
+++ b/app/resource_bundle_posix.cc
@@ -5,6 +5,7 @@
 #include "app/resource_bundle.h"
 
 #include "app/gfx/font.h"
+#include "app/l10n_util.h"
 #include "base/data_pack.h"
 #include "base/logging.h"
 #include "base/string16.h"
@@ -79,7 +80,7 @@ string16 ResourceBundle::GetLocalizedString(int message_id) {
   return msg;
 }
 
-void ResourceBundle::LoadResources(const std::wstring& pref_locale) {
+std::string ResourceBundle::LoadResources(const std::wstring& pref_locale) {
   DCHECK(!resources_data_) << "chrome.pak already loaded";
   FilePath resources_file_path = GetResourcesFilePath();
   DCHECK(!resources_file_path.empty()) << "chrome.pak not found";
@@ -87,12 +88,14 @@ void ResourceBundle::LoadResources(const std::wstring& pref_locale) {
   DCHECK(resources_data_) << "failed to load chrome.pak";
 
   DCHECK(!locale_resources_data_) << "locale.pak already loaded";
-  FilePath locale_file_path = GetLocaleFilePath(pref_locale);
+  std::string app_locale = l10n_util::GetApplicationLocale(pref_locale);
+  FilePath locale_file_path = GetLocaleFilePath(app_locale);
   if (locale_file_path.empty()) {
     // It's possible that there is no locale.pak.
     NOTREACHED();
-    return;
+    return std::string();
   }
   locale_resources_data_ = LoadResourcesDataPak(locale_file_path);
   DCHECK(locale_resources_data_) << "failed to load locale.pak";
+  return app_locale;
 }
diff --git a/app/resource_bundle_win.cc b/app/resource_bundle_win.cc
index 9a0f7eb..ae420b8 100644
--- a/app/resource_bundle_win.cc
+++ b/app/resource_bundle_win.cc
@@ -38,17 +38,18 @@ ResourceBundle::~ResourceBundle() {
   }
 }
 
-void ResourceBundle::LoadResources(const std::wstring& pref_locale) {
+std::string ResourceBundle::LoadResources(const std::wstring& pref_locale) {
   // As a convenience, set resources_data_ to the current resource module.
   resources_data_ = _AtlBaseModule.GetResourceInstance();
 
   DCHECK(NULL == locale_resources_data_) << "locale dll already loaded";
-  const FilePath& locale_path = GetLocaleFilePath(pref_locale);
+  std::string app_locale = l10n_util::GetApplicationLocale(pref_locale);
+  const FilePath& locale_path = GetLocaleFilePath(app_locale);
   if (locale_path.value().empty()) {
     // It's possible that there are no locale dlls found, in which case we just
     // return.
     NOTREACHED();
-    return;
+    return std::string();
   }
 
   // The dll should only have resources, not executable code.
@@ -56,14 +57,14 @@ void ResourceBundle::LoadResources(const std::wstring& pref_locale) {
                                          GetDataDllLoadFlags());
   DCHECK(locale_resources_data_ != NULL) <<
       "unable to load generated resources";
+  return app_locale;
 }
 
 // static
-FilePath ResourceBundle::GetLocaleFilePath(const std::wstring& pref_locale) {
+FilePath ResourceBundle::GetLocaleFilePath(const std::string& app_locale) {
   FilePath locale_path;
   PathService::Get(app::DIR_LOCALES, &locale_path);
 
-  const std::string app_locale = l10n_util::GetApplicationLocale(pref_locale);
   if (app_locale.empty())
     return FilePath();
 
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 766fb91..9878994 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -559,15 +559,20 @@ int BrowserMain(const MainFunctionParams& parameters) {
 
   // If we're running tests (ui_task is non-null), then the ResourceBundle
   // has already been initialized.
-  // Mac starts it earlier in Platform::WillInitializeMainMessageLoop (because
-  // it is needed when loading the MainMenu.nib and the language doesn't depend
-  // on anything since it comes from Cocoa.
-#if !defined(OS_MACOSX)
-  if (!parameters.ui_task) {
-    ResourceBundle::InitSharedInstance(
+  if (parameters.ui_task) {
+    g_browser_process->set_application_locale("en-US");
+  } else {
+    // Mac starts it earlier in Platform::WillInitializeMainMessageLoop (because
+    // it is needed when loading the MainMenu.nib and the language doesn't depend
+    // on anything since it comes from Cocoa.
+#if defined(OS_MACOSX)
+    g_browser_process->set_application_locale(l10n_util::GetLocaleOverride());
+#else
+    std::string app_locale = ResourceBundle::InitSharedInstance(
         local_state->GetString(prefs::kApplicationLocale));
-  }
+    g_browser_process->set_application_locale(app_locale);
 #endif  // !defined(OS_MACOSX)
+  }
 
 #if defined(OS_LINUX)
   gtk_util::SetDefaultWindowIcon();
diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h
index 3283318..d826321 100644
--- a/chrome/browser/browser_process.h
+++ b/chrome/browser/browser_process.h
@@ -125,6 +125,7 @@ class BrowserProcess {
 
   // Returns the locale used by the application.
   virtual const std::string& GetApplicationLocale() = 0;
+  virtual void set_application_locale(const std::string& locale) = 0;
 
   DownloadRequestManager* download_request_manager();
 
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 9b110a65..54d7858 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -290,15 +290,6 @@ printing::PrintJobManager* BrowserProcessImpl::print_job_manager() {
   return print_job_manager_.get();
 }
 
-const std::string& BrowserProcessImpl::GetApplicationLocale() {
-  DCHECK(CalledOnValidThread());
-  if (locale_.empty()) {
-    locale_ = l10n_util::GetApplicationLocale(
-        local_state()->GetString(prefs::kApplicationLocale));
-  }
-  return locale_;
-}
-
 void BrowserProcessImpl::CreateResourceDispatcherHost() {
   DCHECK(!created_resource_dispatcher_host_ &&
          resource_dispatcher_host_.get() == NULL);
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 3d1bbcd..f4b0e7d 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -194,7 +194,13 @@ class BrowserProcessImpl : public BrowserProcess, public NonThreadSafe {
     return google_url_tracker_.get();
   }
 
-  virtual const std::string& GetApplicationLocale();
+  virtual const std::string& GetApplicationLocale() {
+    DCHECK(!locale_.empty());
+    return locale_;
+  }
+  virtual void set_application_locale(const std::string& locale) {
+    locale_ = locale;
+  }
 
   virtual base::WaitableEvent* shutdown_event() {
     return shutdown_event_.get();
diff --git a/chrome/browser/extensions/extension_file_util.cc b/chrome/browser/extensions/extension_file_util.cc
index a1cace7..0c16ecd 100644
--- a/chrome/browser/extensions/extension_file_util.cc
+++ b/chrome/browser/extensions/extension_file_util.cc
@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_util.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/privacy_blacklist/blacklist.h"
 #include "chrome/browser/privacy_blacklist/blacklist_io.h"
@@ -414,9 +415,7 @@ ExtensionMessageBundle* LoadLocaleInfo(const FilePath& extension_path,
     return NULL;
   }
 
-  // We can't call g_browser_process->GetApplicationLocale() since we are not
-  // on the main thread.
-  static std::string app_locale = l10n_util::GetApplicationLocale(L"");
+  std::string app_locale = g_browser_process->GetApplicationLocale();
   if (locales.find(app_locale) == locales.end())
     app_locale = "";
   ExtensionMessageBundle* message_bundle =
diff --git a/chrome/browser/utility_process_host.cc b/chrome/browser/utility_process_host.cc
index 9b03634..76d24f2 100644
--- a/chrome/browser/utility_process_host.cc
+++ b/chrome/browser/utility_process_host.cc
@@ -74,9 +74,7 @@ bool UtilityProcessHost::StartProcess(const FilePath& exposed_dir) {
                                   switches::kUtilityProcess);
   cmd_line->AppendSwitchWithValue(switches::kProcessChannelID,
                                   ASCIIToWide(channel_id()));
-  // Pass on the browser locale.  TODO(tony): This touches the disk and
-  // checks for locale dlls/pak files.  It shouldn't need to.
-  std::string locale = l10n_util::GetApplicationLocale(L"");
+  std::string locale = g_browser_process->GetApplicationLocale();
   cmd_line->AppendSwitchWithValue(switches::kLang, locale);
 
   SetCrashReporterCommandLine(cmd_line);
diff --git a/chrome/common/extensions/extension_l10n_util.cc b/chrome/common/extensions/extension_l10n_util.cc
index 0095227..4157083 100644
--- a/chrome/common/extensions/extension_l10n_util.cc
+++ b/chrome/common/extensions/extension_l10n_util.cc
@@ -192,24 +192,4 @@ ExtensionMessageBundle* LoadMessageCatalogs(
   return ExtensionMessageBundle::Create(catalogs, error);
 }
 
-void GetL10nRelativePaths(const FilePath& relative_resource_path,
-                          std::vector<FilePath>* l10n_paths) {
-  DCHECK(NULL != l10n_paths);
-
-  std::string* current_locale = GetProcessLocale();
-  if (current_locale->empty())
-    *current_locale = l10n_util::GetApplicationLocale(L"");
-
-  std::vector<std::string> locales;
-  GetParentLocales(*current_locale, &locales);
-
-  FilePath locale_relative_path;
-  for (size_t i = 0; i < locales.size(); ++i) {
-    l10n_paths->push_back(locale_relative_path
-        .AppendASCII(Extension::kLocaleFolder)
-        .AppendASCII(locales[i])
-        .Append(relative_resource_path));
-  }
-}
-
 }  // namespace extension_l10n_util
diff --git a/chrome/common/extensions/extension_l10n_util.h b/chrome/common/extensions/extension_l10n_util.h
index 87ad43f..7be62912 100644
--- a/chrome/common/extensions/extension_l10n_util.h
+++ b/chrome/common/extensions/extension_l10n_util.h
@@ -71,12 +71,6 @@ ExtensionMessageBundle* LoadMessageCatalogs(
     const std::set<std::string>& valid_locales,
     std::string* error);
 
-// Returns relative l10n paths to the resource.
-// Returned vector starts with more specific locale path, and ends with the
-// least specific one.
-void GetL10nRelativePaths(const FilePath& relative_resource_path,
-                          std::vector<FilePath>* l10n_paths);
-
 }  // namespace extension_l10n_util
 
 #endif  // CHROME_COMMON_EXTENSIONS_EXTENSION_L10N_UTIL_H_
diff --git a/chrome/common/extensions/extension_l10n_util_unittest.cc b/chrome/common/extensions/extension_l10n_util_unittest.cc
index 2825a4c..da46b78 100644
--- a/chrome/common/extensions/extension_l10n_util_unittest.cc
+++ b/chrome/common/extensions/extension_l10n_util_unittest.cc
@@ -193,39 +193,4 @@ TEST(ExtensionL10nUtil, GetParentLocales) {
   EXPECT_EQ("sr", locales[2]);
 }
 
-bool PathsAreEqual(const FilePath& path1, const FilePath& path2) {
-  FilePath::StringType path1_str = path1.value();
-  std::replace(path1_str.begin(), path1_str.end(), '\\', '/');
-
-  FilePath::StringType path2_str = path2.value();
-  std::replace(path2_str.begin(), path2_str.end(), '\\', '/');
-
-  if (path1_str == path2_str) {
-    return true;
-  } else {
-    return false;
-  }
-}
-
-TEST(ExtensionL10nUtil, GetL10nRelativePath) {
-  static std::string current_locale =
-    extension_l10n_util::NormalizeLocale(l10n_util::GetApplicationLocale(L""));
-
-  std::vector<FilePath> l10n_paths;
-  extension_l10n_util::GetL10nRelativePaths(
-      FilePath(FILE_PATH_LITERAL("foo/bar.js")), &l10n_paths);
-  ASSERT_FALSE(l10n_paths.empty());
-
-  std::vector<std::string> locales;
-  extension_l10n_util::GetParentLocales(current_locale, &locales);
-  ASSERT_EQ(locales.size(), l10n_paths.size());
-
-  for (size_t i = 0; i < locales.size(); ++i) {
-    FilePath tmp;
-    tmp = tmp.AppendASCII(Extension::kLocaleFolder)
-      .AppendASCII(locales[i]).AppendASCII("foo/bar.js");
-    EXPECT_TRUE(PathsAreEqual(tmp, l10n_paths[i]));
-  }
-}
-
 }  // namespace
diff --git a/chrome/test/testing_browser_process.h b/chrome/test/testing_browser_process.h
index 070c865..2131ed7 100644
--- a/chrome/test/testing_browser_process.h
+++ b/chrome/test/testing_browser_process.h
@@ -139,6 +139,7 @@ class TestingBrowserProcess : public BrowserProcess {
       value = new std::string("en");
     return *value;
   }
+  virtual void set_application_locale(const std::string& app_locale) {}
 
   virtual base::WaitableEvent* shutdown_event() {
     return shutdown_event_.get();