Sending LanguageModel info to suggestions server

Sending LanguageModel info to suggestions server

This CL makes use of recently introduced translate::LanguageModel.
It reveals top two languages from language model in requests to content
suggestions server.

As LanguageModel does not have an iOS factory yet, the iOS factory
passes nullptr to NTPSnippetsFetcher at the moment. Achieving feature
parity on iOS is left for follow-up CLs.

BUG=653058
Committed: https://crrev.com/c2dd3bbcfd8c37c233ba1663176de847b8a90023
Cr-Commit-Position: refs/heads/master@{#423906}

[jkrcal, 2016-10-07 09:41:18]

jkrcal@chromium.org changed reviewers:
+ bauerb@chromium.org, droger@chromium.org, sdefresne@chromium.org

[jkrcal, 2016-10-07 09:41:19]

Bernhard, could you PTAL?

Sylvain, could you PTAL at the iOS factory?
David, could you PTAL at translate/core/browser dependency?

[jkrcal, 2016-10-07 09:41:25]

The CQ bit was checked by jkrcal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 09:41:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sdefresne, 2016-10-07 09:47:17]

ios/ lgtm

[droger, 2016-10-07 09:55:38]

DEPS lgtm

[commit-bot: I haz the power, 2016-10-07 10:26:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 10:26:23]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2016-10-07 12:26:13]

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippets_fetcher.cc (right):

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.cc:172: char
language[ULOC_LANG_CAPACITY];
This is not going to be zero-initialized, right? So if for some reason we can't
convert the language, are we going to construct a string from uninitialized
data?

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.cc:174:
uloc_getLanguage(locale.c_str(), language, ULOC_LANG_CAPACITY, &error);
#include "third_party/icu/source/common/unicode/uloc.h"?

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.cc:175: DLOG_IF(WARNING,
U_ZERO_ERROR != error)
Write the condition the natural way around? A modern compiler would complain
anyway if you accidentally used = instead of ==.

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippets_fetcher.h (right):

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.h:169: struct
ContentLanguageInfo {
Why don't we just use LanguageModel::LanguageInfo for this?

[jkrcal, 2016-10-07 12:58:43]

Thanks! PTAL, again.

(note a new variation parameter guard appeared in the meantime)

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippets_fetcher.cc (right):

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.cc:172: char
language[ULOC_LANG_CAPACITY];
On 2016/10/07 12:26:13, Bernhard Bauer wrote:
> This is not going to be zero-initialized, right? So if for some reason we
can't
> convert the language, are we going to construct a string from uninitialized
> data?

Good point! This applies to the function above as well. I prefer to be more
explicit and use a regular if for error. Is it okay for you?

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.cc:174:
uloc_getLanguage(locale.c_str(), language, ULOC_LANG_CAPACITY, &error);
On 2016/10/07 12:26:13, Bernhard Bauer wrote:
> #include "third_party/icu/source/common/unicode/uloc.h"?

It is already there, isn't it? ;)

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.cc:175: DLOG_IF(WARNING,
U_ZERO_ERROR != error)
On 2016/10/07 12:26:13, Bernhard Bauer wrote:
> Write the condition the natural way around? A modern compiler would complain
> anyway if you accidentally used = instead of ==.

Done.

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippets_fetcher.h (right):

https://codereview.chromium.org/2400133002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippets_fetcher.h:169: struct
ContentLanguageInfo {
On 2016/10/07 12:26:13, Bernhard Bauer wrote:
> Why don't we just use LanguageModel::LanguageInfo for this?

Good point. Done.

[Bernhard Bauer, 2016-10-07 13:26:24]

lgtm

[jkrcal, 2016-10-07 13:49:27]

The CQ bit was checked by jkrcal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 13:49:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 14:16:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 14:16:24]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jkrcal, 2016-10-07 14:49:23]

The CQ bit was checked by jkrcal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 14:49:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 15:16:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 15:16:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jkrcal, 2016-10-07 16:22:54]

The CQ bit was checked by jkrcal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 16:23:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 17:14:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 17:14:16]

Dry run: This issue passed the CQ dry run.

[jkrcal, 2016-10-07 17:51:39]

The CQ bit was checked by jkrcal@chromium.org

[jkrcal, 2016-10-07 17:51:39]

The patchset sent to the CQ was uploaded after l-g-t-m from droger@chromium.org,
sdefresne@chromium.org, bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2400133002/#ps100001
(title: "Zero initialization #2")

[commit-bot: I haz the power, 2016-10-07 17:52:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 17:58:41]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-07 18:01:12]

Description was changed from

==========
Sending LanguageModel info to suggestions server

This CL makes use of recently introduced translate::LanguageModel.
It reveals top two languages from language model in requests to content
suggestions server.

As LanguageModel does not have an iOS factory yet, the iOS factory
passes nullptr to NTPSnippetsFetcher at the moment. Achieving feature
parity on iOS is left for follow-up CLs.

BUG=653058
==========

to

==========
Sending LanguageModel info to suggestions server

This CL makes use of recently introduced translate::LanguageModel.
It reveals top two languages from language model in requests to content
suggestions server.

As LanguageModel does not have an iOS factory yet, the iOS factory
passes nullptr to NTPSnippetsFetcher at the moment. Achieving feature
parity on iOS is left for follow-up CLs.

BUG=653058

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

[commit-bot: I haz the power, 2016-10-07 18:01:13]

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