All platforms don't support same locales at the same time. Make locale check...

All platforms don't support same locales at the same time. Make locale check for extensions less strict (we log a warning now, but let extension load).

TEST=Create extension with _locales/xxx_yyy and see that it loads.

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

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

chrome/common/extensions/extension_file_util.cc

@@ -411,57 +411,51 @@ static bool ValidateLocaleInfo(const Extension& extension, std::string* error) {
   std::string default_locale = extension.default_locale();
 
   // If both default locale and _locales folder are empty, skip verification.
-  if (!default_locale.empty() || path_exists) {
-    if (default_locale.empty() && path_exists) {
-      *error = errors::kLocalesNoDefaultLocaleSpecified;
-      return false;
-    } else if (!default_locale.empty() && !path_exists) {
-      *error = errors::kLocalesTreeMissing;
-      return false;
-    }
-
-    // Treat all folders under _locales as valid locales.
-    file_util::FileEnumerator locales(path,
-                                      false,
-                                      file_util::FileEnumerator::DIRECTORIES);
+  if (default_locale.empty() && !path_exists)
+    return true;
 
-    FilePath locale_path = locales.Next();
-    if (locale_path.empty()) {
-      *error = errors::kLocalesTreeMissing;
-      return false;
-    }
+  if (default_locale.empty() && path_exists) {
+    *error = errors::kLocalesNoDefaultLocaleSpecified;
+    return false;
+  } else if (!default_locale.empty() && !path_exists) {
+    *error = errors::kLocalesTreeMissing;
+    return false;
+  }
 
-    const FilePath default_locale_path = path.AppendASCII(default_locale);
-    bool has_default_locale_message_file = false;
-    do {
-      // Skip any strings with '.'. This happens sometimes, for example with
-      // '.svn' directories.
-      FilePath relative_path;
-      if (!extension.path().AppendRelativePath(locale_path, &relative_path))
-        NOTREACHED();
-      std::wstring subdir(relative_path.ToWStringHack());
-      if (std::find(subdir.begin(), subdir.end(), L'.') != subdir.end())
-        continue;
+  // Treat all folders under _locales as valid locales.
+  file_util::FileEnumerator locales(path,
+                                    false,
+                                    file_util::FileEnumerator::DIRECTORIES);
 
-      FilePath messages_path =
-        locale_path.Append(Extension::kMessagesFilename);
+  std::set<std::string> all_locales;
+  extension_l10n_util::GetAllLocales(&all_locales);
+  const FilePath default_locale_path = path.AppendASCII(default_locale);
+  bool has_default_locale_message_file = false;
 
-      if (!file_util::PathExists(messages_path)) {
-        *error = StringPrintf(
-            "%s %s", errors::kLocalesMessagesFileMissing,
-            WideToUTF8(messages_path.ToWStringHack()).c_str());
-        return false;
-      }
+  FilePath locale_path;
+  while (!(locale_path = locales.Next()).empty()) {
+    if (extension_l10n_util::ShouldSkipValidation(path, locale_path,
+                                                  all_locales))
+      continue;
 
-      if (locale_path == default_locale_path)
-        has_default_locale_message_file = true;
-    } while (!(locale_path = locales.Next()).empty());
+    FilePath messages_path =
+        locale_path.Append(Extension::kMessagesFilename);
 
-    // Only message file for default locale has to exist.
-    if (!has_default_locale_message_file) {
-      *error = errors::kLocalesNoDefaultMessages;
+    if (!file_util::PathExists(messages_path)) {
+      *error = StringPrintf(
+          "%s %s", errors::kLocalesMessagesFileMissing,
+          WideToUTF8(messages_path.ToWStringHack()).c_str());
       return false;
     }
+
+    if (locale_path == default_locale_path)
+      has_default_locale_message_file = true;
+  }
+
+  // Only message file for default locale has to exist.
+  if (!has_default_locale_message_file) {
+    *error = errors::kLocalesNoDefaultMessages;
+    return false;
   }
 
   return true;

chrome/common/extensions/extension_l10n_util.cc

@@ -11,6 +11,7 @@
 #include "app/l10n_util.h"
 #include "base/file_util.h"
 #include "base/linked_ptr.h"
+#include "base/logging.h"
 #include "base/string_util.h"
 #include "base/values.h"
 #include "chrome/common/extensions/extension.h"
@@ -147,10 +148,11 @@ bool AddLocale(const std::set<std::string>& chrome_locales,
   if (locale_name.find(".") == 0)
     return true;
   if (chrome_locales.find(locale_name) == chrome_locales.end()) {
-    // Fail if there is an extension locale that's not in the Chrome list.
-    *error = StringPrintf("Supplied locale %s is not supported.",
-                          locale_name.c_str());
-    return false;
+    // Warn if there is an extension locale that's not in the Chrome list,
+    // but don't fail.
+    LOG(WARNING) << StringPrintf("Supplied locale %s is not supported.",
+                                 locale_name.c_str());
+    return true;
   }
   // Check if messages file is actually present (but don't check content).
   if (file_util::PathExists(
@@ -196,9 +198,7 @@ void GetParentLocales(const std::string& current_locale,
   }
 }
 
-// Extends list of Chrome locales to them and their parents, so we can do
-// proper fallback.
-static void GetAllLocales(std::set<std::string>* all_locales) {
+void GetAllLocales(std::set<std::string>* all_locales) {
   const std::vector<std::string>& available_locales =
       l10n_util::GetAvailableLocales();
   // Add all parents of the current locale to the available locales set.
@@ -240,7 +240,6 @@ bool GetValidLocales(const FilePath& locale_path,
 
   return true;
 }
-
 // Loads contents of the messages file for given locale. If file is not found,
 // or there was parsing error we return NULL and set |error|.
 // Caller owns the returned object.
@@ -293,4 +292,23 @@ ExtensionMessageBundle* LoadMessageCatalogs(
   return ExtensionMessageBundle::Create(catalogs, error);
 }
 
+bool ShouldSkipValidation(const FilePath& locales_path,
+                          const FilePath& locale_path,
+                          const std::set<std::string>& all_locales) {
+  // Since we use this string as a key in a DictionaryValue, be paranoid about
+  // skipping any strings with '.'. This happens sometimes, for example with
+  // '.svn' directories.
+  FilePath relative_path;
+  if (!locales_path.AppendRelativePath(locale_path, &relative_path))
+    NOTREACHED();
+  std::wstring subdir(relative_path.ToWStringHack());
+  if (std::find(subdir.begin(), subdir.end(), L'.') != subdir.end())
+    return true;
+
+  if (all_locales.find(WideToASCII(subdir)) == all_locales.end())
+    return true;
+
+  return false;
+}
+
 }  // namespace extension_l10n_util

chrome/common/extensions/extension_l10n_util.h

@@ -72,6 +72,10 @@ std::string CurrentLocaleOrDefault();
 void GetParentLocales(const std::string& current_locale,
                       std::vector<std::string>* parent_locales);
 
+// Extends list of Chrome locales to them and their parents, so we can do
+// proper fallback.
+void GetAllLocales(std::set<std::string>* all_locales);
+
 // Adds valid locales to the extension.
 // 1. Do nothing if _locales directory is missing (not an error).
 // 2. Get list of Chrome locales.
@@ -95,6 +99,15 @@ ExtensionMessageBundle* LoadMessageCatalogs(
     const std::set<std::string>& valid_locales,
     std::string* error);
 
+// Returns true if directory has "." in the name (for .svn) or if it doesn't
+// belong to Chrome locales.
+// |locales_path| is extension_id/_locales
+// |locale_path| is extension_id/_locales/xx
+// |all_locales| is a set of all valid Chrome locales.
+bool ShouldSkipValidation(const FilePath& locales_path,
+                          const FilePath& locale_path,
+                          const std::set<std::string>& all_locales);
+
 }  // namespace extension_l10n_util
 
 #endif  // CHROME_COMMON_EXTENSIONS_EXTENSION_L10N_UTIL_H_

chrome/common/extensions/extension_l10n_util_unittest.cc

@@ -56,6 +56,33 @@ TEST(ExtensionL10nUtil, GetValidLocalesWithValidLocaleNoMessagesFile) {
   EXPECT_TRUE(locales.empty());
 }
 
+TEST(ExtensionL10nUtil, GetValidLocalesWithUnsupportedLocale) {
+  ScopedTempDir temp;
+  ASSERT_TRUE(temp.CreateUniqueTempDir());
+
+  FilePath src_path = temp.path().Append(Extension::kLocaleFolder);
+  ASSERT_TRUE(file_util::CreateDirectory(src_path));
+  // Supported locale.
+  FilePath locale_1 = src_path.AppendASCII("sr");
+  ASSERT_TRUE(file_util::CreateDirectory(locale_1));
+  std::string data("whatever");
+  ASSERT_TRUE(file_util::WriteFile(
+      locale_1.Append(Extension::kMessagesFilename),
+      data.c_str(), data.length()));
+  // Unsupported locale.
+  ASSERT_TRUE(file_util::CreateDirectory(src_path.AppendASCII("xxx_yyy")));
+
+  std::string error;
+  std::set<std::string> locales;
+  EXPECT_TRUE(extension_l10n_util::GetValidLocales(src_path,
+                                                   &locales,
+                                                   &error));
+
+  EXPECT_FALSE(locales.empty());
+  EXPECT_TRUE(locales.find("sr") != locales.end());
+  EXPECT_FALSE(locales.find("xxx_yyy") != locales.end());
+}
+
 TEST(ExtensionL10nUtil, GetValidLocalesWithValidLocalesAndMessagesFile) {
   FilePath install_dir;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));

chrome/common/extensions/extension_unpacker.cc

@@ -15,6 +15,7 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_file_util.h"
+#include "chrome/common/extensions/extension_l10n_util.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/url_constants.h"
@@ -115,22 +116,17 @@ bool ExtensionUnpacker::ReadAllMessageCatalogs(
   FilePath locales_path =
     temp_install_dir_.Append(Extension::kLocaleFolder);
 
-  // Treat all folders under _locales as valid locales.
+  // Not all folders under _locales have to be valid locales.
   file_util::FileEnumerator locales(locales_path,
                                     false,
                                     file_util::FileEnumerator::DIRECTORIES);
 
-  FilePath locale_path = locales.Next();
-  do {
-    // Since we use this string as a key in a DictionaryValue, be paranoid about
-    // skipping any strings with '.'. This happens sometimes, for example with
-    // '.svn' directories.
-    FilePath relative_path;
-    // message_path was created from temp_install_dir. This should never fail.
-    if (!temp_install_dir_.AppendRelativePath(locale_path, &relative_path))
-      NOTREACHED();
-    std::wstring subdir(relative_path.ToWStringHack());
-    if (std::find(subdir.begin(), subdir.end(), L'.') != subdir.end())
+  std::set<std::string> all_locales;
+  extension_l10n_util::GetAllLocales(&all_locales);
+  FilePath locale_path;
+  while (!(locale_path = locales.Next()).empty()) {
+    if (extension_l10n_util::ShouldSkipValidation(locales_path, locale_path,
+                                                  all_locales))
       continue;
 
     FilePath messages_path =
@@ -138,7 +134,7 @@ bool ExtensionUnpacker::ReadAllMessageCatalogs(
 
     if (!ReadMessageCatalog(messages_path))
       return false;
-  } while (!(locale_path = locales.Next()).empty());
+  }
 
   return true;
 }

chrome/common/extensions/extension_unpacker_unittest.cc

@@ -100,7 +100,7 @@ TEST_F(ExtensionUnpackerTest, MissingMessagesFile) {
 TEST_F(ExtensionUnpackerTest, NoLocaleData) {
   SetupUnpacker("no_locale_data.crx");
   EXPECT_FALSE(unpacker_->Run());
-  EXPECT_EQ(errors::kLocalesTreeMissing, unpacker_->error_message());
+  EXPECT_EQ(errors::kLocalesNoDefaultMessages, unpacker_->error_message());
 }
 
 TEST_F(ExtensionUnpackerTest, GoodL10n) {