Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(120)

Issue 1947323002: [Offline Pages] Implement OfflinePageStorageManager. (Closed)

Created:
4 years, 7 months ago by romax
Modified:
4 years, 7 months ago
Reviewers:
fgorski
CC:
chromium-reviews, fgorski+watch_chromium.org, romax+watch_chromium.org, dewittj+watch_chromium.org, petewil+watch_chromium.org, dimich+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Offline Pages] Implement OfflinePageStorageManager. OfflinePageStorageManager should be responsible for making offline page model to clear expired offline pages automatically. BUG=609357 Committed: https://crrev.com/ff3a7d889cd3326bc5f080cfd7d7663c2abdd604 Cr-Commit-Position: refs/heads/master@{#392455}

Patch Set 1 #

Total comments: 53

Patch Set 2 : Addressed some comments. #

Patch Set 3 : More comments. #

Total comments: 20

Patch Set 4 : Comments and test. #

Total comments: 1

Patch Set 5 : Fixing gyp builds. #

Total comments: 1

Patch Set 6 : Fix build and comments. #

Patch Set 7 : fix rebasing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+395 lines, -6 lines) Patch
M components/offline_pages.gypi View 1 2 3 4 5 6 1 chunk +7 lines, -2 lines 0 comments Download
M components/offline_pages/BUILD.gn View 1 2 3 4 5 6 2 chunks +3 lines, -0 lines 0 comments Download
M components/offline_pages/offline_page_model.h View 1 2 3 4 5 6 5 chunks +27 lines, -4 lines 0 comments Download
M components/offline_pages/offline_page_model.cc View 1 2 3 4 5 6 4 chunks +20 lines, -0 lines 0 comments Download
M components/offline_pages/offline_page_model_unittest.cc View 1 2 3 4 7 chunks +54 lines, -0 lines 0 comments Download
A components/offline_pages/offline_page_storage_manager.h View 1 2 3 4 5 1 chunk +88 lines, -0 lines 0 comments Download
A components/offline_pages/offline_page_storage_manager.cc View 1 2 3 1 chunk +77 lines, -0 lines 0 comments Download
A components/offline_pages/offline_page_storage_manager_unittest.cc View 1 2 3 4 5 1 chunk +119 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (4 generated)
romax
A basic draft without any fancy logic (size limit/time constraint throttling/ etc.) Please take a ...
4 years, 7 months ago (2016-05-05 01:21:53 UTC) #2
fgorski
Good start. https://codereview.chromium.org/1947323002/diff/1/components/offline_pages/BUILD.gn File components/offline_pages/BUILD.gn (right): https://codereview.chromium.org/1947323002/diff/1/components/offline_pages/BUILD.gn#newcode9 components/offline_pages/BUILD.gn:9: # GYP: //components/offline_pages.gypi:offline_pages don't forget to update ...
4 years, 7 months ago (2016-05-05 04:53:09 UTC) #3
romax
addressed some comments, will do more changes and improvments after offline talk. And also will ...
4 years, 7 months ago (2016-05-05 21:00:03 UTC) #4
romax
some comments. still struggling writing explicit tests for the new class. https://codereview.chromium.org/1947323002/diff/1/components/offline_pages/offline_page_storage_manager.h File components/offline_pages/offline_page_storage_manager.h (right): ...
4 years, 7 months ago (2016-05-06 01:02:32 UTC) #5
fgorski
https://codereview.chromium.org/1947323002/diff/1/components/offline_pages/offline_page_storage_manager.h File components/offline_pages/offline_page_storage_manager.h (right): https://codereview.chromium.org/1947323002/diff/1/components/offline_pages/offline_page_storage_manager.h#newcode28 components/offline_pages/offline_page_storage_manager.h:28: class OfflinePageStorageManager { On 2016/05/05 04:53:09, fgorski wrote: > ...
4 years, 7 months ago (2016-05-06 05:57:25 UTC) #6
romax
updated with comments and separate test file. However only using dummy inheritance in the storage_manager_unittest.cc ...
4 years, 7 months ago (2016-05-07 02:22:43 UTC) #7
fgorski
lgtm. Let's put this in and continue moving tests (and introduce the right interface for ...
4 years, 7 months ago (2016-05-09 20:20:38 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1947323002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1947323002/120001
4 years, 7 months ago (2016-05-09 22:44:56 UTC) #11
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 7 months ago (2016-05-09 22:50:11 UTC) #12
commit-bot: I haz the power
4 years, 7 months ago (2016-05-09 22:52:00 UTC) #14
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/ff3a7d889cd3326bc5f080cfd7d7663c2abdd604
Cr-Commit-Position: refs/heads/master@{#392455}

Powered by Google App Engine
This is Rietveld 408576698