Avoid adding English to the blocked languages on non-English UI

Avoid adding English to the blocked languages on non-English UI

Because of crbug/36182, English is translated even though English exists in the Accept language list on non-English loccale. English is always listed in the list by default whichever the user locale is. So, when the blocked language list is created, English should not be included in the list on non-English locale.

BUG=264155
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=218477

[hajimehoshi, 2013-08-19 08:01:19]

Can you take a look?  Thank you in advance.

[Takashi Toyoshima, 2013-08-19 12:53:46]

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
File chrome/browser/translate/translate_accept_languages.cc (right):

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_accept_languages.cc:94: printf("ui_lang:
%s\n", ui_lang.c_str());
?

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
File chrome/browser/translate/translate_prefs.cc (right):

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs.cc:381: const std::string& lang = *it;
It's not worth reassigning it.

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs.cc:426: std::string app_locale =
g_browser_process->GetApplicationLocale();
[optional] const std::string& is better?

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs.cc:435: // Regarding crbug/36182, even
though English exists in Accept language
To cite a crbug formatting it in http://crbug.com/number is encouraged.

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
File chrome/browser/translate/translate_prefs_unittest.cc (right):

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs_unittest.cc:21: virtual void SetUp()
override {
Don't use override directly. Chromium use OVERRIDE macro to avoid troubles in
gcc.
See base/compiler_specific.h.

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs_unittest.cc:34: std::string
original_locale_;
BTW, do you think to save and to restore the original locale are needed?

[hajimehoshi, 2013-08-20 01:31:24]

Thank you. PTAL

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
File chrome/browser/translate/translate_accept_languages.cc (right):

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_accept_languages.cc:94: printf("ui_lang:
%s\n", ui_lang.c_str());
Oops, sorry.

On 2013/08/19 12:53:46, Takashi Toyoshima (chromium) wrote:
> ?

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
File chrome/browser/translate/translate_prefs.cc (right):

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs.cc:381: const std::string& lang = *it;
On 2013/08/19 12:53:46, Takashi Toyoshima (chromium) wrote:
> It's not worth reassigning it.

Done.

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs.cc:426: std::string app_locale =
g_browser_process->GetApplicationLocale();
On 2013/08/19 12:53:46, Takashi Toyoshima (chromium) wrote:
> [optional] const std::string& is better?

Done.

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs.cc:435: // Regarding crbug/36182, even
though English exists in Accept language
On 2013/08/19 12:53:46, Takashi Toyoshima (chromium) wrote:
> To cite a crbug formatting it in http://crbug.com/number is encouraged.

Done.

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
File chrome/browser/translate/translate_prefs_unittest.cc (right):

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs_unittest.cc:21: virtual void SetUp()
override {
On 2013/08/19 12:53:46, Takashi Toyoshima (chromium) wrote:
> Don't use override directly. Chromium use OVERRIDE macro to avoid troubles in
> gcc.
> See base/compiler_specific.h.

Done.

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs_unittest.cc:34: std::string
original_locale_;
I didn't know g_browser_process is reset for each tests (maybe in
ChromeTestSuite?).

On 2013/08/19 12:53:46, Takashi Toyoshima (chromium) wrote:
> BTW, do you think to save and to restore the original locale are needed?

[Takashi Toyoshima, 2013-08-20 02:03:10]

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
File chrome/browser/translate/translate_prefs_unittest.cc (right):

https://codereview.chromium.org/23013007/diff/1/chrome/browser/translate/tran...
chrome/browser/translate/translate_prefs_unittest.cc:34: std::string
original_locale_;
Basic principle is that all tests run independently. In typical case, a process
isn't reused in multiple tests.

https://codereview.chromium.org/23013007/diff/7001/chrome/browser/translate/t...
File chrome/browser/translate/translate_prefs_unittest.cc (right):

https://codereview.chromium.org/23013007/diff/7001/chrome/browser/translate/t...
chrome/browser/translate/translate_prefs_unittest.cc:14: class
TranslatePrefsTest : public testing::Test {
By the way, do you really need this class?
You can use TEST() instead of using TEST_F with class declaration. Or typedef
testing::Test ... sounds enough.

[hajimehoshi, 2013-08-20 03:31:31]

Thank you! PTAL

https://codereview.chromium.org/23013007/diff/7001/chrome/browser/translate/t...
File chrome/browser/translate/translate_prefs_unittest.cc (right):

https://codereview.chromium.org/23013007/diff/7001/chrome/browser/translate/t...
chrome/browser/translate/translate_prefs_unittest.cc:14: class
TranslatePrefsTest : public testing::Test {
On 2013/08/20 02:03:10, Takashi Toyoshima (chromium) wrote:
> By the way, do you really need this class?
> You can use TEST() instead of using TEST_F with class declaration. Or typedef
> testing::Test ... sounds enough.

Done.

[Takashi Toyoshima, 2013-08-20 03:51:54]

lgtm

[commit-bot: I haz the power, 2013-08-20 06:22:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/23013007/14002

[commit-bot: I haz the power, 2013-08-20 14:06:02]

Change committed as 218477