Implement ping_freshness for update_client.

Implement ping_freshness for update_client.

BUG=603980
Committed: https://crrev.com/e7dff73db0785613c1bd68c9909f6ebe6a9c07e5
Cr-Commit-Position: refs/heads/master@{#387754}

[waffles, 2016-04-15 18:28:34]

waffles@chromium.org changed reviewers:
+ sorin@chromium.org

[waffles, 2016-04-15 18:28:34]

Sorin, PTAL at this small CL to add ping_freshness.

[Sorin Jianu, 2016-04-15 20:26:53]

lgtm

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
File components/update_client/persisted_data.cc (right):

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data.cc:49: std::string freshness =
kPingFreshnessUnknown;
Is there a need to define kPingFreshnessUnknown at all? We could just return
std::string().

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data.cc:56: return "{" + freshness + "}";
if we need to format I suggest stringprintf, since expressions like this may
result in temporary variables being created.

This pattern is uses in other places in this CL and previous CLs. It might be
worth reviewing its usage.

I have to do some science with C++11 and the current compilers, since I believe
in some cases, it may avoid creating some of the temporaries. I am not concerned
about time efficiency but it space efficiency due to additional code needed to
create and destroy the temporaries.

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
File components/update_client/persisted_data.h (right):

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data.h:51: int datenum) const;
The const modifier on a setter is debatable, as a general idea:
https://isocpp.org/wiki/faq/const-correctness#logical-vs-physical-state

It is a small issue for this CL and our project but might be worth a thought.

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
File components/update_client/persisted_data_unittest.cc (right):

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data_unittest.cc:25: std::string pf1 =
metadata->GetPingFreshness("someappid");
const?

[waffles, 2016-04-15 21:54:20]

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
File components/update_client/persisted_data.cc (right):

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data.cc:49: std::string freshness =
kPingFreshnessUnknown;
On 2016/04/15 20:26:53, Sorin Jianu wrote:
> Is there a need to define kPingFreshnessUnknown at all? We could just return
> std::string().

Done.

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data.cc:56: return "{" + freshness + "}";
On 2016/04/15 20:26:53, Sorin Jianu wrote:
> if we need to format I suggest stringprintf, since expressions like this may
> result in temporary variables being created.
> 
> This pattern is uses in other places in this CL and previous CLs. It might be
> worth reviewing its usage.
> 
> I have to do some science with C++11 and the current compilers, since I
believe
> in some cases, it may avoid creating some of the temporaries. I am not
concerned
> about time efficiency but it space efficiency due to additional code needed to
> create and destroy the temporaries.

Done.

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
File components/update_client/persisted_data.h (right):

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data.h:51: int datenum) const;
On 2016/04/15 20:26:53, Sorin Jianu wrote:
> The const modifier on a setter is debatable, as a general idea:
> https://isocpp.org/wiki/faq/const-correctness#logical-vs-physical-state
> 
> It is a small issue for this CL and our project but might be worth a thought.

Done.

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
File components/update_client/persisted_data_unittest.cc (right):

https://codereview.chromium.org/1889263002/diff/1/components/update_client/pe...
components/update_client/persisted_data_unittest.cc:25: std::string pf1 =
metadata->GetPingFreshness("someappid");
On 2016/04/15 20:26:53, Sorin Jianu wrote:
> const?

Done.

[waffles, 2016-04-15 21:54:59]

Patchset #2 (id:20001) has been deleted

[Sorin Jianu, 2016-04-15 22:21:05]

lgtm

thank you!

[waffles, 2016-04-15 22:21:48]

The CQ bit was checked by waffles@chromium.org

[commit-bot: I haz the power, 2016-04-15 22:22:19]

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

[commit-bot: I haz the power, 2016-04-15 23:52:01]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-04-15 23:53:36]

Description was changed from

==========
Implement ping_freshness for update_client.

BUG=603980
==========

to

==========
Implement ping_freshness for update_client.

BUG=603980

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

[commit-bot: I haz the power, 2016-04-15 23:53:37]

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