[Offline Pages] Removing client in storage manager.

[Offline Pages] Removing client in storage manager.

Removing OfflinePageStorageManager::Client and merging into
OfflinePageModel. Also fixed some related minor issues and tests.

BUG=615165
Committed: https://crrev.com/44b9e6bd68d769be11b0520e73d0b2b37d53765a
Cr-Commit-Position: refs/heads/master@{#397806}

[romax, 2016-05-31 22:06:54]

Description was changed from

==========
[Offline Pages] Removing client in storage manager.

Removing OfflinePageStorageManager::Client and merging into
OfflinePageModel. Also fixed some related minor issues and tests.

BUG=615165
==========

to

==========
[Offline Pages] Removing client in storage manager.

Removing OfflinePageStorageManager::Client and merging into
OfflinePageModel. Also fixed some related minor issues and tests.

BUG=615165
==========

[romax, 2016-05-31 22:06:54]

romax@chromium.org changed reviewers:
+ dougarnett@chromium.org, jianli@chromium.org

[romax, 2016-06-01 01:23:07]

PTAL

[dougarnett, 2016-06-01 01:41:11]

lgtm

Great.

Be sure Jian Li is good with his 52 merge before you land this (in case we do
want to revert my change after all to merge his change - something he raised
this afternoon).

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_model.h (right):

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:28: static const char* const
kLastNNamespace = "last_n";
Btw, i was hoping we could get these constants out of this file into some
unknown-to-me better home. But good to keep them together for now.

[chili, 2016-06-01 03:54:36]

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

[chili, 2016-06-01 03:54:37]

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_model.h (right):

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:28: static const char* const
kLastNNamespace = "last_n";
On 2016/06/01 01:41:11, dougarnett wrote:
> Btw, i was hoping we could get these constants out of this file into some
> unknown-to-me better home. But good to keep them together for now.

Jian's CL (https://codereview.chromium.org/2022283003/) moved these out to a
different constants file I think...

[dougarnett, 2016-06-01 15:06:54]

On 2016/06/01 03:54:37, chili wrote:
>
https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
> File components/offline_pages/offline_page_model.h (right):
> 
>
https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
> components/offline_pages/offline_page_model.h:28: static const char* const
> kLastNNamespace = "last_n";
> On 2016/06/01 01:41:11, dougarnett wrote:
> > Btw, i was hoping we could get these constants out of this file into some
> > unknown-to-me better home. But good to keep them together for now.
> 
> Jian's CL (https://codereview.chromium.org/2022283003/) moved these out to a
> different constants file I think...

Perfect - thanks for the pointer.

[fgorski, 2016-06-01 16:18:06]

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

[fgorski, 2016-06-01 16:18:07]

lgtm

[jianli, 2016-06-01 20:41:21]

lgtm

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_model.h (right):

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:28: static const char* const
kLastNNamespace = "last_n";
On 2016/06/01 03:54:37, chili wrote:
> On 2016/06/01 01:41:11, dougarnett wrote:
> > Btw, i was hoping we could get these constants out of this file into some
> > unknown-to-me better home. But good to keep them together for now.
> 
> Jian's CL (https://codereview.chromium.org/2022283003/) moved these out to a
> different constants file I think...

Yes, please wait and sync after my patch (need to merge to M52) gets landed.

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:128: virtual void
GetAllPages(const MultipleOfflinePageItemCallback& callback) = 0;
Please add const modifier.

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:193: // Marks pages with
|offline_ids| as expired and delete the associated archive
nit: deletes

[romax, 2016-06-01 20:58:44]

I suppose the one Jian mentioned has landed, and I'm not sure about the const
comment...

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_model.h (right):

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:28: static const char* const
kLastNNamespace = "last_n";
On 2016/06/01 20:41:21, jianli wrote:
> On 2016/06/01 03:54:37, chili wrote:
> > On 2016/06/01 01:41:11, dougarnett wrote:
> > > Btw, i was hoping we could get these constants out of this file into some
> > > unknown-to-me better home. But good to keep them together for now.
> > 
> > Jian's CL (https://codereview.chromium.org/2022283003/) moved these out to a
> > different constants file I think...
> 
> Yes, please wait and sync after my patch (need to merge to M52) gets landed.

Is this the one to wait for? Or what's the CL number of the one I should wait?
https://codereview.chromium.org/2022283003/

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:128: virtual void
GetAllPages(const MultipleOfflinePageItemCallback& callback) = 0;
On 2016/06/01 20:41:21, jianli wrote:
> Please add const modifier.

i'm not sure where to add a const?

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:193: // Marks pages with
|offline_ids| as expired and delete the associated archive
On 2016/06/01 20:41:21, jianli wrote:
> nit: deletes

Done.

[jianli, 2016-06-01 20:59:37]

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_model.h (right):

https://codereview.chromium.org/2026843003/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model.h:128: virtual void
GetAllPages(const MultipleOfflinePageItemCallback& callback) = 0;
On 2016/06/01 20:58:44, romax wrote:
> On 2016/06/01 20:41:21, jianli wrote:
> > Please add const modifier.
> 
> i'm not sure where to add a const?

virtual void GetAllPages(const MultipleOfflinePageItemCallback& callback) const
= 0;

[romax, 2016-06-03 19:37:02]

The CQ bit was checked by romax@chromium.org

[romax, 2016-06-03 19:37:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
dougarnett@chromium.org, jianli@chromium.org, fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/2026843003/#ps40001
(title: "Styles.")

[commit-bot: I haz the power, 2016-06-03 19:37:23]

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

[commit-bot: I haz the power, 2016-06-03 21:26:58]

Description was changed from

==========
[Offline Pages] Removing client in storage manager.

Removing OfflinePageStorageManager::Client and merging into
OfflinePageModel. Also fixed some related minor issues and tests.

BUG=615165
==========

to

==========
[Offline Pages] Removing client in storage manager.

Removing OfflinePageStorageManager::Client and merging into
OfflinePageModel. Also fixed some related minor issues and tests.

BUG=615165
==========

[commit-bot: I haz the power, 2016-06-03 21:27:00]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-03 21:28:24]

Description was changed from

==========
[Offline Pages] Removing client in storage manager.

Removing OfflinePageStorageManager::Client and merging into
OfflinePageModel. Also fixed some related minor issues and tests.

BUG=615165
==========

to

==========
[Offline Pages] Removing client in storage manager.

Removing OfflinePageStorageManager::Client and merging into
OfflinePageModel. Also fixed some related minor issues and tests.

BUG=615165

Committed: https://crrev.com/44b9e6bd68d769be11b0520e73d0b2b37d53765a
Cr-Commit-Position: refs/heads/master@{#397806}
==========

[commit-bot: I haz the power, 2016-06-03 21:28:26]

Patchset 3 (id:??) landed as
https://crrev.com/44b9e6bd68d769be11b0520e73d0b2b37d53765a
Cr-Commit-Position: refs/heads/master@{#397806}