[translate] Add ranker model proto and loader.

[translate] Add ranker model proto and loader.

This CL adds RankerModel and RankerModel loader classes to wrap,
load and cache a generic RankerModelProto.

BUG=646711
Review-Url: https://codereview.chromium.org/2761363002
Cr-Commit-Position: refs/heads/master@{#458886}
Committed: https://chromium.googlesource.com/chromium/src/+/56a66e411fe157c498b9a4a77be20a24bf697a31

[Roger McFarlane (Chromium), 2017-03-21 17:28:29]

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

[commit-bot: I haz the power, 2017-03-21 17:29:23]

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, 2017-03-21 18:46:36]

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

[commit-bot: I haz the power, 2017-03-21 18:46:40]

Dry run: This issue passed the CQ dry run.

[Roger McFarlane (Chromium), 2017-03-21 19:14:12]

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

[commit-bot: I haz the power, 2017-03-21 19:14:42]

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

[Roger McFarlane (Chromium), 2017-03-21 19:18:02]

Patchset #1 (id:1) has been deleted

[Roger McFarlane (Chromium), 2017-03-21 19:38:04]

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

[Roger McFarlane (Chromium), 2017-03-21 19:38:05]

PTAL

This is a peeled out extract of https://codereview.chromium.org/2565873002/ and
it's retake https://codereview.chromium.org/2726203002/ which you have reviewed
before.

This CL extracts only the RankerModelProto, Rankerodel and 
RankerModelLoader classes and tests, without the code changes for integrating
them into the rest of Chrome, which will come in a separate CL.

This code is identical to the code previously reviewed except for the following
details:

  * added a ranker_model_unittest.cc
  * updated copyright date to 2017 instead of 2016 for
    newly added files.
  * added TaskPriority::BACKGROUND to the backend's
    task runner. This seems to have gotten lost at
    some point during the previous reviews.

Out of band, I'll send a diff file capturing the above described delta between
the files in this CL vs the one(s) previously reviewed.

[commit-bot: I haz the power, 2017-03-21 20:04:36]

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

[commit-bot: I haz the power, 2017-03-21 20:04:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Roger McFarlane (Chromium), 2017-03-22 14:10:18]

rogerm@chromium.org changed reviewers:
+ andrewhayden@chromium.org, droger@chromium.org, hajimehoshi@chromium.org
- groby@chromium.org

[Roger McFarlane (Chromium), 2017-03-22 14:10:19]

Alas, it turns out groby@ is OOO on vacation.

hajimehoshi@, droger@ and/or andrewhayden@ are the other top suggested reviewers
by git cl owners.

Can one (or more) of you PTAL?

The code has actually already been reviewed by groby@ in the context of
https://codereview.chromium.org/2565873002/

There are minor diffs from the original (updated copyright date, two additional
unittests, one addition background TaskTrait setting), which I can forward as a
diff.

The corresponding design documentation is at http://go/chrome-ranker in the
"Model Load and Update" section.

[droger, 2017-03-22 14:30:07]

I only reviewed ranker_model_unittest.cc.
LGTM with comments below, and rubberstamping the other files.

https://codereview.chromium.org/2761363002/diff/20001/components/translate/co...
File components/translate/core/browser/ranker_model_unittest.cc (right):

https://codereview.chromium.org/2761363002/diff/20001/components/translate/co...
components/translate/core/browser/ranker_model_unittest.cc:20: int64_t
InSeconds(const base::Time& t) {
Time should be passed by value (here and in other places in this file).
See comment in time.h
https://cs.chromium.org/chromium/src/base/time/time.h?rcl=ec080c375b6006502f1...

https://codereview.chromium.org/2761363002/diff/20001/components/translate/co...
components/translate/core/browser/ranker_model_unittest.cc:55: auto
original_model_as_string = original_model->SerializeAsString();
Nit, optional: I think the code would be clearer with fewer uses of auto.
In particular std::string is not much longer to type than auto.

[hamelphi, 2017-03-22 15:09:29]

hamelphi@chromium.org changed reviewers:
+ hamelphi@chromium.org

[hamelphi, 2017-03-22 15:09:29]

lgtm

[Roger McFarlane (Chromium), 2017-03-22 20:31:01]

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

[Roger McFarlane (Chromium), 2017-03-22 20:31:43]

Thanks!

https://codereview.chromium.org/2761363002/diff/20001/components/translate/co...
File components/translate/core/browser/ranker_model_unittest.cc (right):

https://codereview.chromium.org/2761363002/diff/20001/components/translate/co...
components/translate/core/browser/ranker_model_unittest.cc:20: int64_t
InSeconds(const base::Time& t) {
On 2017/03/22 14:30:07, droger wrote:
> Time should be passed by value (here and in other places in this file).
> See comment in time.h
>
https://cs.chromium.org/chromium/src/base/time/time.h?rcl=ec080c375b6006502f1...

Done.

https://codereview.chromium.org/2761363002/diff/20001/components/translate/co...
components/translate/core/browser/ranker_model_unittest.cc:55: auto
original_model_as_string = original_model->SerializeAsString();
On 2017/03/22 14:30:07, droger wrote:
> Nit, optional: I think the code would be clearer with fewer uses of auto.
> In particular std::string is not much longer to type than auto.

Done.

[commit-bot: I haz the power, 2017-03-22 20:31:45]

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, 2017-03-22 21:35:01]

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

[commit-bot: I haz the power, 2017-03-22 21:35:02]

Dry run: This issue passed the CQ dry run.

[Roger McFarlane (Chromium), 2017-03-22 21:46:47]

The CQ bit was checked by rogerm@chromium.org

[Roger McFarlane (Chromium), 2017-03-22 21:46:47]

The patchset sent to the CQ was uploaded after l-g-t-m from droger@chromium.org,
hamelphi@chromium.org
Link to the patchset: https://codereview.chromium.org/2761363002/#ps40001
(title: "comments from droger@")

[commit-bot: I haz the power, 2017-03-22 21:47:12]

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, 2017-03-22 21:52:49]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490219207295910,
"parent_rev": "d0d5714663b6a19befac1ec77ebdb1b4585ae953", "commit_rev":
"56a66e411fe157c498b9a4a77be20a24bf697a31"}

[commit-bot: I haz the power, 2017-03-22 21:53:31]

Description was changed from

==========
[translate] Add ranker model proto and loader.

This CL adds RankerModel and RankerModel loader classes to wrap,
load and cache a generic RankerModelProto.

BUG=646711
==========

to

==========
[translate] Add ranker model proto and loader.

This CL adds RankerModel and RankerModel loader classes to wrap,
load and cache a generic RankerModelProto.

BUG=646711

Review-Url: https://codereview.chromium.org/2761363002
Cr-Commit-Position: refs/heads/master@{#458886}
Committed:
https://chromium.googlesource.com/chromium/src/+/56a66e411fe157c498b9a4a77be2...
==========

[commit-bot: I haz the power, 2017-03-22 21:53:32]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/56a66e411fe157c498b9a4a77be2...