It would also be great if we should update the offline pages policy code so that we could dynamically control -- aka not have to wait for releases -- expiration and maybe other configurations.

I think this is already scoped for other offline features but this is one case where it would have been useful to have this capability.

Expiration (or some other mechanism) can be useful to prevent accumulation of pages left behind for some reason. I'm not sure current mechanism of deleting pages on WebContentObserver::WasHidden is reliable enough to guarantee lack of accumulation of old pages. Unless we have some other sweeping mechanism, we need something to guarantee that eventually those are cleaned up. 

The progress to move temporary pages to cache directory is blocked. And seems this bug is blocked on that one. Once it's in, there's a 'guarantee' that the pages will be deleted eventually mentioned in comment #3, i think.

Re #2:
Currently, beyond expiration and the WasHidden event you mentioned there is another cleanup mechanisms triggered when a tab is closed that cleans any and all last_n offline pages linked to it [1].

But you make me realize we would still have issues should a tab closure not be reported. A while ago I used to sometimes lose all my open tabs so those closure would not have been reported. Should that happen again it could lead to last_n offline pages being left behind.

We need a solution for that anyway -- I don't think our current cleanup strategies cover that case? romax@ could you chime in on this? -- but in the meantime we could initially extend the OPC namespace expiration to something very long (a month or two?) that would already do wonders for this problem.


Re #3:
Is the guarantee you are referring to the user triggered cache cleanup? If that's so I don't think that covers the concern from #2 as it requires user to realize what's happening and manually clean the cache. I think we want a solution that works unattended.


[1] https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java?type=cs&q="bridge.deletePagesByClientId"&sq=package:chromium

To #4: agree that extending OPC expiration to 30 days or so could be interesting (lets add UMA as well, something like "age of OPC page that has just been used for tab restore". 
We also need to make sure the removal of cache pages driven by low storage pressure is also there (there is a mechanism for that, just need to verify it is used for OPC).

i guess i'm not on the point here, but i'll try to dump what I know:
1. the files left behind will be removed during consistency check, since there'll be no *page* pointing to those *files*.
2. extending expiration is fine, and feels like it will only affects a little pages, if there's a strong guarantee that the number of last_n pages is in the same order of the number of opened tabs.
3. for removing cache pages by low storage pressure, I guess StorageManager is still in charge of it. there should be some room for improvements.
4. Since p2p sharing is coming, and it will very likely introduce another strategy of consistency check (which will not remove pages immediately and instead of removing files, import them into DB), I'm wondering if last_n pages needs to be treated the same, or if consistency check should be split into two different strategy for cache/persistent pages.

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

commit bda7562c6417fe2315cb6423e906805cdc174ece
Author: Carlos Knippschild <carlosk@chromium.org>
Date: Fri Dec 01 00:29:53 2017

Increase Offline Page Cache expiration to 30 days.

OPC pages seem to be expiring too early based on time-to-first-open
metric. It seems there's room for increasing its usefulness by
increasing the expiration time. This CL increases it to 30 days as this
longer time has been already considered safe while experiencing with
Prefetching of Offline Pages.

Bug:  740706 
Change-Id: I954394c769e37ccb1afe3ed7dba2aa4cfa2c6864
Reviewed-on: https://chromium-review.googlesource.com/794750
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520770}
[modify] https://crrev.com/bda7562c6417fe2315cb6423e906805cdc174ece/components/offline_pages/core/client_policy_controller.cc
[modify] https://crrev.com/bda7562c6417fe2315cb6423e906805cdc174ece/components/offline_pages/core/model/clear_storage_task_unittest.cc

Just to provide some more context for the expiration increase:
* With Prefetching we realized it is actually safe for offline pages to have longer expiration dates.
* We don't expect such a significant increase in used storage space due to this increase given that OPC page count is bound to the number of open tabs.
* Since M63 there is a metric that tracks used storage space per namespace, the one specific to OPC being OfflinePages.ClearStoragePreRunUsage2.last_n. It will allow us to detect any significant changes.
* There's no expected increase in any other kind of resource usage by OPC with this change.

This change landed a while ago. Marking as fixed.