[Translate] Factor out access to Accept-Languages

[Translate] Factor out access to Accept-Languages

Code to get accept-lang was repeated in several places.
Unified into TranslatePrefs, so all accessors share the
same code path.

BUG=569219
Committed: https://crrev.com/c46f7a2a47d62334679c764abde2d177785a883d
Cr-Commit-Position: refs/heads/master@{#365924}

[groby-ooo-7-16, 2015-12-16 02:48:26]

PTAL - this is more work in preparation for a translate experiment to change the
target language in some locales.

As a somewhat related question: I'm wondering if it would make sense to store
the TranslatePrefs as UserData on a BrowserContext - I'm not quite sure why we
keep creating them, and I assume this might be because Translate simply predates
UserData? Or is there a technical reason that I am missing?

https://codereview.chromium.org/1525203003/diff/1/components/translate/core/b...
File components/translate/core/browser/translate_prefs.cc (right):

https://codereview.chromium.org/1525203003/diff/1/components/translate/core/b...
components/translate/core/browser/translate_prefs.cc:375: void
TranslatePrefs::GetAcceptLanguageList(
Since GetAcceptLanguageList is only used by TranslateManager::GetTargetLanguage,
and GetTargetLanguage calls TranslateDownloadManager::GetLanguageCode, I believe
it is safe to just use GetLanguageList, even though it returns the
preferred_languages on ChromeOS. 

I.e. setting the languages to 'en-US', 'ja', 'en-CA' would result in
en-US,ja,en-CA for the preferred languages, 'en-US', 'en', 'ja', 'en-CA' for the
accept-languages. The comparison in GetTargetLanguage calls GetLanguageCode on
all of them, which strips all the en-* variants to en - so the lists are
fundamentally identical.

Please let me know if I misunderstood.

[groby-ooo-7-16, 2015-12-17 01:38:23]

Description was changed from

==========
[Translate] Factor out access to Accept-Languages

Code to get accept-lang was repeated in several places.
Unified into TranslatePrefs, so all accessors share the
same code path.

BUG=569219
==========

to

==========
[Translate] Factor out access to Accept-Languages

Code to get accept-lang was repeated in several places.
Unified into TranslatePrefs, so all accessors share the
same code path.

BUG=569219
==========

[groby-ooo-7-16, 2015-12-17 01:38:23]

groby@chromium.org changed reviewers:
+ hajimehoshi@chromium.org

[groby-ooo-7-16, 2015-12-17 01:38:56]

PTAL (also, please see previous comment - I hit send without adding you to
reviewers, sorry)

[hajimehoshi, 2015-12-17 06:18:46]

lgtm

On 2015/12/16 02:48:26, groby wrote:
> PTAL - this is more work in preparation for a translate experiment to change
the
> target language in some locales.
> 
> As a somewhat related question: I'm wondering if it would make sense to store
> the TranslatePrefs as UserData on a BrowserContext - I'm not quite sure why we
> keep creating them, and I assume this might be because Translate simply
predates
> UserData? Or is there a technical reason that I am missing?

You mean we might be able to use Profile inheriting BrowserContext instead of
PrefService? I think they are semantically different but am not sure.

> 
>
https://codereview.chromium.org/1525203003/diff/1/components/translate/core/b...
> File components/translate/core/browser/translate_prefs.cc (right):
> 
>
https://codereview.chromium.org/1525203003/diff/1/components/translate/core/b...
> components/translate/core/browser/translate_prefs.cc:375: void
> TranslatePrefs::GetAcceptLanguageList(
> Since GetAcceptLanguageList is only used by
TranslateManager::GetTargetLanguage,
> and GetTargetLanguage calls TranslateDownloadManager::GetLanguageCode, I
believe
> it is safe to just use GetLanguageList, even though it returns the
> preferred_languages on ChromeOS. 
> 
> I.e. setting the languages to 'en-US', 'ja', 'en-CA' would result in
> en-US,ja,en-CA for the preferred languages, 'en-US', 'en', 'ja', 'en-CA' for
the
> accept-languages. The comparison in GetTargetLanguage calls GetLanguageCode on
> all of them, which strips all the en-* variants to en - so the lists are
> fundamentally identical.
> 
> Please let me know if I misunderstood.

[groby-ooo-7-16, 2015-12-17 21:56:29]

The CQ bit was checked by groby@chromium.org

[groby-ooo-7-16, 2015-12-17 21:56:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
hajimehoshi@chromium.org
Link to the patchset: https://codereview.chromium.org/1525203003/#ps60001
(title: "Fix CrOS build")

[commit-bot: I haz the power, 2015-12-17 21:58:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1525203003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1525203003/60001

[commit-bot: I haz the power, 2015-12-17 22:39:52]

Description was changed from

==========
[Translate] Factor out access to Accept-Languages

Code to get accept-lang was repeated in several places.
Unified into TranslatePrefs, so all accessors share the
same code path.

BUG=569219
==========

to

==========
[Translate] Factor out access to Accept-Languages

Code to get accept-lang was repeated in several places.
Unified into TranslatePrefs, so all accessors share the
same code path.

BUG=569219
==========

[commit-bot: I haz the power, 2015-12-17 22:39:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-17 22:40:49]

Description was changed from

==========
[Translate] Factor out access to Accept-Languages

Code to get accept-lang was repeated in several places.
Unified into TranslatePrefs, so all accessors share the
same code path.

BUG=569219
==========

to

==========
[Translate] Factor out access to Accept-Languages

Code to get accept-lang was repeated in several places.
Unified into TranslatePrefs, so all accessors share the
same code path.

BUG=569219

Committed: https://crrev.com/c46f7a2a47d62334679c764abde2d177785a883d
Cr-Commit-Position: refs/heads/master@{#365924}
==========

[commit-bot: I haz the power, 2015-12-17 22:40:50]

Patchset 4 (id:??) landed as
https://crrev.com/c46f7a2a47d62334679c764abde2d177785a883d
Cr-Commit-Position: refs/heads/master@{#365924}