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.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=41046

[rafaelw, 2010-03-09 00:09:14]

http://codereview.chromium.org/671011/diff/2001/3004
File chrome/common/extensions/extension_file_util.cc (right):

http://codereview.chromium.org/671011/diff/2001/3004#newcode411
chrome/common/extensions/extension_file_util.cc:411: if (!default_locale.empty()
|| path_exists) {
Can you change this to exit early if default locale and _locales are both empty?
i.e.

if (default_locale.empty() && !path_exists)
  return true;

http://codereview.chromium.org/671011/diff/2001/3004#newcode435
chrome/common/extensions/extension_file_util.cc:435: do {
it seems like this should either be a for loop, or you should DCHECK immediately
before that locales contains at least 1 item.

http://codereview.chromium.org/671011/diff/2001/3003
File chrome/common/extensions/extension_l10n_util.h (right):

http://codereview.chromium.org/671011/diff/2001/3003#newcode107
chrome/common/extensions/extension_l10n_util.h:107: bool
ShouldSkipDirectory(const FilePath& locales_path,
How about ShouldSkipValidation?

http://codereview.chromium.org/671011/diff/2001/3005
File chrome/common/extensions/extension_unpacker.cc (right):

http://codereview.chromium.org/671011/diff/2001/3005#newcode127
chrome/common/extensions/extension_unpacker.cc:127: do {
it seems like this should either be a for loop, or you should DCHECK immediately
before that locales contains at least 1 item.

[Nebojša Ćirić, 2010-03-09 00:44:53]

http://codereview.chromium.org/671011/diff/2001/3004
File chrome/common/extensions/extension_file_util.cc (right):

http://codereview.chromium.org/671011/diff/2001/3004#newcode411
chrome/common/extensions/extension_file_util.cc:411: if (!default_locale.empty()
|| path_exists) {
On 2010/03/09 00:09:14, rafaelw wrote:
> Can you change this to exit early if default locale and _locales are both
empty?
> i.e.
> 
> if (default_locale.empty() && !path_exists)
>   return true;
> 
> 

Done.

http://codereview.chromium.org/671011/diff/2001/3004#newcode435
chrome/common/extensions/extension_file_util.cc:435: do {
On 2010/03/09 00:09:14, rafaelw wrote:
> it seems like this should either be a for loop, or you should DCHECK
immediately
> before that locales contains at least 1 item.

Done.

http://codereview.chromium.org/671011/diff/2001/3005
File chrome/common/extensions/extension_unpacker.cc (right):

http://codereview.chromium.org/671011/diff/2001/3005#newcode127
chrome/common/extensions/extension_unpacker.cc:127: do {
On 2010/03/09 00:09:14, rafaelw wrote:
> it seems like this should either be a for loop, or you should DCHECK
immediately
> before that locales contains at least 1 item.

Done.

[rafaelw, 2010-03-09 00:50:18]

lgtm w/ nit.

http://codereview.chromium.org/671011/diff/10001/11005
File chrome/common/extensions/extension_unpacker.cc (right):

http://codereview.chromium.org/671011/diff/10001/11005#newcode128
chrome/common/extensions/extension_unpacker.cc:128: if
(extension_l10n_util::ShouldSkipValidation(
nit: looks like you can still fit at least one of these args on the first line.

[Nebojša Ćirić, 2010-03-09 00:52:46]

Done.

On 2010/03/09 00:50:18, rafaelw wrote:
> lgtm w/ nit.
> 
> http://codereview.chromium.org/671011/diff/10001/11005
> File chrome/common/extensions/extension_unpacker.cc (right):
> 
> http://codereview.chromium.org/671011/diff/10001/11005#newcode128
> chrome/common/extensions/extension_unpacker.cc:128: if
> (extension_l10n_util::ShouldSkipValidation(
> nit: looks like you can still fit at least one of these args on the first
line.