[Offline Pages] Adding metadatas for namespace async_loading.

[Offline Pages] Adding metadatas for namespace async_loading.

Added namespace, histogram suffix and client policy for async_loading.
Also fixed a crash when adding suffixes for expiring histograms.

BUG=623736
Committed: https://crrev.com/14c0911f78eaf6c704b65ed46b300ed52f0f1626
Cr-Commit-Position: refs/heads/master@{#405401}

[romax, 2016-06-29 21:20:35]

romax@chromium.org changed reviewers:
+ fgorski@chromium.org

[romax, 2016-06-29 21:20:35]

PTAL, I need suggestion for policy numbers for async_loading...

[fgorski, 2016-07-01 19:26:09]

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
File components/offline_pages/client_policy_controller.cc (right):

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
components/offline_pages/client_policy_controller.cc:28: kAsyncNamespace,
MakePolicy(kAsyncNamespace, LifetimeType::TEMPORARY,
They should be LifetimeType::PERSISTENT.

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
components/offline_pages/client_policy_controller.cc:29:
base::TimeDelta::FromDays(2), kUnlimitedPages,
We need to make sure we have an infinite expiration_period.
Unless we ignore it already for persistent lifetime.

[romax, 2016-07-06 22:03:28]

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
File components/offline_pages/client_policy_controller.cc (right):

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
components/offline_pages/client_policy_controller.cc:28: kAsyncNamespace,
MakePolicy(kAsyncNamespace, LifetimeType::TEMPORARY,
On 2016/07/01 19:26:09, fgorski wrote:
> They should be LifetimeType::PERSISTENT.
> 

I'm not against to change it to persistent. However I don't see us having a
mechanism to delete persistent pages now. It means if we mark it as persistent
and make it have an infinite expiration_period (which is a good thing to do),
there's no way to delete them unless wipe all pages. So I'm not sure if I should
do this and add a check when expiring pages *for now*. Also please let me know
if we have something for user to delete persistent pages, if there is one, I'm
more than happy to finish the changes.

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
components/offline_pages/client_policy_controller.cc:29:
base::TimeDelta::FromDays(2), kUnlimitedPages,
On 2016/07/01 19:26:09, fgorski wrote:
> We need to make sure we have an infinite expiration_period.
> Unless we ignore it already for persistent lifetime.

please see previous comment.

[fgorski, 2016-07-07 17:01:45]

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
File components/offline_pages/client_policy_controller.cc (right):

https://codereview.chromium.org/2111653002/diff/1/components/offline_pages/cl...
components/offline_pages/client_policy_controller.cc:28: kAsyncNamespace,
MakePolicy(kAsyncNamespace, LifetimeType::TEMPORARY,
On 2016/07/06 22:03:28, romax wrote:
> On 2016/07/01 19:26:09, fgorski wrote:
> > They should be LifetimeType::PERSISTENT.
> > 
> 
> I'm not against to change it to persistent. However I don't see us having a
> mechanism to delete persistent pages now. It means if we mark it as persistent
> and make it have an infinite expiration_period (which is a good thing to do),
> there's no way to delete them unless wipe all pages. So I'm not sure if I
should
> do this and add a check when expiring pages *for now*. Also please let me know
> if we have something for user to delete persistent pages, if there is one, I'm
> more than happy to finish the changes.

We are not done with the feature. Downloads home is where we will be able to
delete the pages from. I believe it should already be doable from then clear
browsing data (or similar settings page).
Also, for development you can do it from chrome://offline-internals.

[romax, 2016-07-09 00:52:10]

Patchset #2 (id:20001) has been deleted

[romax, 2016-07-09 01:04:32]

some changes in *impl.cc was not correct...please ignore.

[fgorski, 2016-07-12 22:19:49]

lgtm, but please run and fix the test.

https://codereview.chromium.org/2111653002/diff/40001/components/offline_page...
File components/offline_pages/client_policy_controller_unittest.cc (right):

https://codereview.chromium.org/2111653002/diff/40001/components/offline_page...
components/offline_pages/client_policy_controller_unittest.cc:69:
EXPECT_TRUE(isTemporary(policy));
does it work?

[romax, 2016-07-13 18:06:09]

romax@chromium.org changed reviewers:
+ holte@chromium.org

[romax, 2016-07-13 18:06:10]

Addressed comments.

holte@ PTAL on histograms.xml (one line change)

Thanks!

[Steven Holte, 2016-07-13 20:14:47]

lgtm

[romax, 2016-07-13 20:58:06]

The CQ bit was checked by romax@chromium.org

[romax, 2016-07-13 20:58:06]

The patchset sent to the CQ was uploaded after l-g-t-m from fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/2111653002/#ps60001
(title: "Fixing tests.")

[commit-bot: I haz the power, 2016-07-13 20:59:05]

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

[romax, 2016-07-13 23:46:46]

The CQ bit was unchecked by romax@chromium.org

[romax, 2016-07-13 23:53:26]

The CQ bit was checked by romax@chromium.org

[romax, 2016-07-13 23:53:27]

The patchset sent to the CQ was uploaded after l-g-t-m from holte@chromium.org,
fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/2111653002/#ps80001
(title: "Fixing tests.")

[commit-bot: I haz the power, 2016-07-13 23:54:41]

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-07-14 02:01:20]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-07-14 02:04:12]

Description was changed from

==========
[Offline Pages] Adding metadatas for namespace async_loading.

Added namespace, histogram suffix and client policy for async_loading.
Also fixed a crash when adding suffixes for expiring histograms.

BUG=623736
==========

to

==========
[Offline Pages] Adding metadatas for namespace async_loading.

Added namespace, histogram suffix and client policy for async_loading.
Also fixed a crash when adding suffixes for expiring histograms.

BUG=623736

Committed: https://crrev.com/14c0911f78eaf6c704b65ed46b300ed52f0f1626
Cr-Commit-Position: refs/heads/master@{#405401}
==========

[commit-bot: I haz the power, 2016-07-14 02:04:13]

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