New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 605615 link

Starred by 0 users
Status: Fixed
Owner:
fgor...@chromium.org
Closed: May 2016
Cc:
█ romax@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Make sure that expiring an offline page is a 2 step process

Project Member Reported by fgor...@chromium.org, Apr 21 2016 
For offline page expiry we should follow a two step process:

1. Expiring should make sure that the file is removed (therefore space on disk is freed.)

1.a. If the offline copy was never accessed, the item can be removed together with the archive.

1.b. Otherwise we mark the item as removed.

2. After some time passes (e.g. 2-3 weeks) and we have don't expect there to be a tab that still shows the item, we can remove it.

Things to pay attention to:
* Ensure redirecting works properly for an offline item without an archive.
* Ensure pages are properly marked when accessed.
* Ensure deleted items are filtered from queries for available archives.
* Ensure omnibox behaves properly.

Comment 1 by fgor...@chromium.org, May 13 2016

Status: Started (was: Assigned)
Extracting ArchiveManager to begin with.
Project Member

Comment 2 by bugdroid1@chromium.org, May 17 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c

commit bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c
Author: fgorski <fgorski@chromium.org>
Date: Tue May 17 04:57:52 2016

[Offline pages] Adding ArchiveManager to abstract offline archives management

* This patch only creates the archiver (it will be hooked up to the model
  in the next one.)
* Creates set of tests for the archiver.
* Adding missing destructor for OfflinePageStorageManager::Client (found by ASAN tests enabled in this patch)

BUG= 605615 

Review-Url: https://codereview.chromium.org/1977853004
Cr-Commit-Position: refs/heads/master@{#394054}

[modify] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/components_tests.gyp
[modify] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages.gypi
[modify] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages/BUILD.gn
[add] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages/archive_manager.cc
[add] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages/archive_manager.h
[add] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages/archive_manager_unittest.cc
[modify] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages/offline_page_storage_manager.h
[modify] https://crrev.com/bc8e3e7ce63c8fcb53de75ec378daec78a9ea60c/components/offline_pages/offline_page_storage_manager_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, May 24 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48904d1cdbc4f70fd961efdc69199897be8cad84

commit 48904d1cdbc4f70fd961efdc69199897be8cad84
Author: fgorski <fgorski@chromium.org>
Date: Tue May 24 17:56:34 2016

[Offline pages] Adding expiration capability to OfflinePageModel

* Adds expiration_time to offline page item
* Adds capability to expire pages to the model
* Updates both stores and tests with the expiration_time

BUG= 605615 

Review-Url: https://codereview.chromium.org/1993953002
Cr-Commit-Position: refs/heads/master@{#395638}

[modify] https://crrev.com/48904d1cdbc4f70fd961efdc69199897be8cad84/components/offline_pages/offline_page_item.cc
[modify] https://crrev.com/48904d1cdbc4f70fd961efdc69199897be8cad84/components/offline_pages/offline_page_item.h
[modify] https://crrev.com/48904d1cdbc4f70fd961efdc69199897be8cad84/components/offline_pages/offline_page_metadata_store_impl_unittest.cc
[modify] https://crrev.com/48904d1cdbc4f70fd961efdc69199897be8cad84/components/offline_pages/offline_page_metadata_store_sql.cc
[modify] https://crrev.com/48904d1cdbc4f70fd961efdc69199897be8cad84/components/offline_pages/offline_page_model.cc
[modify] https://crrev.com/48904d1cdbc4f70fd961efdc69199897be8cad84/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/48904d1cdbc4f70fd961efdc69199897be8cad84/components/offline_pages/offline_page_model_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, May 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b43a3e24f906a24f3a85c94892e3824cce521e7

commit 2b43a3e24f906a24f3a85c94892e3824cce521e7
Author: romax <romax@chromium.org>
Date: Fri May 27 18:13:38 2016

[Offline Pages] Two-step expiration in storage manager.

This patch expanded the two-step expiration from offline page model to
storage manager. Now the storage manager would expire a page and delete
the archived file when it wasn't used long enough and after a certain
time interval remove the page entry from metadata store.
Also added tests for corresponding cases.

BUG= 605615 

Review-Url: https://codereview.chromium.org/2006923005
Cr-Commit-Position: refs/heads/master@{#396513}

[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/client_policy_controller.cc
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/client_policy_controller.h
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/offline_page_client_policy.h
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/offline_page_model.cc
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/offline_page_model_unittest.cc
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/offline_page_storage_manager.cc
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/offline_page_storage_manager.h
[modify] https://crrev.com/2b43a3e24f906a24f3a85c94892e3824cce521e7/components/offline_pages/offline_page_storage_manager_unittest.cc

Comment 6 by romax@chromium.org, May 27 2016

Status: Fixed (was: Started)
I'm marking as fixed since the patch has landed...
►

Sign in to add a comment