Reading list: add first read PB entries

Reading list: add first read PB entries

This is needed for metrics to know the time between read and deletion.

BUG=651041
Committed: https://crrev.com/012b96dca047694e05df7e3c00fcc1719ddc5a31
Cr-Commit-Position: refs/heads/master@{#434779}

[Olivier, 2016-11-28 17:21:04]

olivierrobin@chromium.org changed reviewers:
+ gambard@chromium.org

[gambard, 2016-11-28 17:33:51]

lgtm with 1 comment

https://codereview.chromium.org/2534903003/diff/1/components/reading_list/ios...
File components/reading_list/ios/proto/reading_list.proto (right):

https://codereview.chromium.org/2534903003/diff/1/components/reading_list/ios...
components/reading_list/ios/proto/reading_list.proto:22: optional int64
first_read_time_us = 11;
It makes sense to regroup the fields by usage (i.e. "timestamp"), but I would
have made them ordered by field number so it would be easier to add a new field.

[Olivier, 2016-11-28 17:59:14]

olivierrobin@chromium.org changed reviewers:
+ pavely@chromium.org

[Olivier, 2016-11-28 17:59:15]

Hi pavely

PTAL

https://codereview.chromium.org/2534903003/diff/1/components/reading_list/ios...
File components/reading_list/ios/proto/reading_list.proto (right):

https://codereview.chromium.org/2534903003/diff/1/components/reading_list/ios...
components/reading_list/ios/proto/reading_list.proto:22: optional int64
first_read_time_us = 11;
On 2016/11/28 17:33:51, gambard wrote:
> It makes sense to regroup the fields by usage (i.e. "timestamp"), but I would
> have made them ordered by field number so it would be easier to add a new
field.

It is  like that in other specifics, so I guess it is fine.

[pavely, 2016-11-28 19:11:55]

lgtm. You'll need corresponding code in ReadingListEntry, right?

[Olivier, 2016-11-28 21:33:02]

Thanks

Yes, gambard (in charge of metrics) will add the code in ReadingListEntry. I
just wanted to add the code here first as there may be some time before it is
pushed to the server (not sure it is needed...)

[Olivier, 2016-11-28 21:33:06]

The CQ bit was checked by olivierrobin@chromium.org

[commit-bot: I haz the power, 2016-11-28 21:34:29]

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-11-28 23:43:08]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1480368786108420, "parent_rev":
"24fba93386ab68b8ae7eb56d5cf27a8c7b64cac3", "commit_rev":
"ba33fa69dd99271c09ac268fd9833f299bd8930c"}

[commit-bot: I haz the power, 2016-11-28 23:43:29]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-28 23:46:34]

Description was changed from

==========
Reading list: add first read PB entries

This is needed for metrics to know the time between read and deletion.

BUG=651041
==========

to

==========
Reading list: add first read PB entries

This is needed for metrics to know the time between read and deletion.

BUG=651041

Committed: https://crrev.com/012b96dca047694e05df7e3c00fcc1719ddc5a31
Cr-Commit-Position: refs/heads/master@{#434779}
==========

[commit-bot: I haz the power, 2016-11-28 23:46:34]

Patchset 1 (id:??) landed as
https://crrev.com/012b96dca047694e05df7e3c00fcc1719ddc5a31
Cr-Commit-Position: refs/heads/master@{#434779}