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

components/offline_pages/offline_page_storage_manager.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_STORAGE_MANAGER_H_
 #define COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_STORAGE_MANAGER_H_
 
 #include <stdint.h>
 
 #include <vector>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "components/offline_pages/archive_manager.h"
 #include "components/offline_pages/offline_page_types.h"
 
 namespace base {
 class Clock;
 }  // namespace base
 
 namespace offline_pages {
 
-// Limit of the total storage space occupied by offline pages should be 30% of
-// available storage. And we clear storage when it is over the threshold,
-// reducing the usage below threshold.
+// Maximum % of total available storage that will be occupied by offline pages
+// before a storage clearing.

[jianli, 2016/05/26 21:59:31]

nit: storage clearing => storage cleanup

[romax, 2016/05/27 00:41:38]

Done.

 const double kOfflinePageStorageLimit = 0.3;

[jianli, 2016/05/26 21:59:32]

If these constants are only needed in .cc file, please move them over.

[romax, 2016/05/27 00:41:38]

per discussion I'm leaving them here.

+// The target % of storage usage we try to reach below when expiring pages.
 const double kOfflinePageStorageClearThreshold = 0.1;
+// Storage manager will clear storage if it has been longer than this interval

[jianli, 2016/05/26 21:59:31]

nit: The time that the storage cleanup will be triggered again since the last
cleanup.

[romax, 2016/05/27 00:41:38]

Done.

+// since last clearing.
 const base::TimeDelta kClearStorageInterval = base::TimeDelta::FromMinutes(10);
+// Storage manager will remove a page from metadata store once the page has been

[jianli, 2016/05/26 21:59:32]

nit: The time that the page record will be removed from the store since the page
has been expired.

[romax, 2016/05/27 00:41:38]

Done.

+// expired longer than this interval.
+const base::TimeDelta kRemovePageItemInterval = base::TimeDelta::FromDays(21);
 
 class ArchiveManager;
 class ClientPolicyController;
 
 // This class is used for storage management of offline pages. It provides
 // a ClearPagesIfNeeded method which is used to clear expired offline pages
 // based on last_access_time and lifetime policy of its namespace.
 // It has its own throttle mechanism so calling the method would not be
 // guaranteed to clear the pages immediately.
 //
 // OfflinePageModel should own and control the lifecycle of this manager.
 // And this manager would use OfflinePageModel to get/remove pages.
 class OfflinePageStorageManager {
  public:
   // This interface should have no knowledge of offline page model.
   // This interface should be implemented by clients managed by storage manager.
   class Client {
    public:
     virtual ~Client() {}
 
+    // Asks the client to delete pages based on |ofline_ids| and invoke

[jianli, 2016/05/26 21:59:32]

nit: invoke => invokes
also consider adding "upon completion" after |callback|.

[romax, 2016/05/27 00:41:38]

Done.

+    // |callback|.
+    virtual void DeletePagesByOfflineId(const std::vector<int64_t>& offline_ids,
+                                        const DeletePageCallback& callback) = 0;
+
     // Asks the client to get all offline pages and invoke |callback|.
     virtual void GetAllPages(
         const MultipleOfflinePageItemCallback& callback) = 0;
 
-    // Asks the client to delete pages based on |offline_ids| and invoke
-    // |callback|.
-    virtual void DeletePagesByOfflineId(const std::vector<int64_t>& offline_ids,
-                                        const DeletePageCallback& callback) = 0;
+    // Asks the client to mark pages with |offline_ids| as expired and the

[jianli, 2016/05/26 21:59:32]

nit: add "deletes" after "and".

[romax, 2016/05/27 00:41:38]

Done.

+    // associated archive files.
+    virtual void ExpirePages(const std::vector<int64_t>& offline_ids,
+                             const base::Time& expiration_time,
+                             const base::Callback<void(bool)>& callback) = 0;
   };
 
   enum class ClearStorageResult {
     SUCCESS,         // Cleared successfully.
     UNNECESSARY,     // No expired pages.
+    EXPIRE_FAILURE,  // Expiration failed.
     DELETE_FAILURE,  // Deletion failed.
   };
 
   // Callback used when calling ClearPagesIfNeeded.
-  // int: the number of expired pages.
+  // size_t: the number of expired pages.
   // ClearStorageResult: result of expiring pages in storage.
-  typedef base::Callback<void(int, ClearStorageResult)> ClearPagesCallback;
+  typedef base::Callback<void(size_t, ClearStorageResult)> ClearPagesCallback;
 
   explicit OfflinePageStorageManager(Client* client,
                                      ClientPolicyController* policy_controller,
                                      ArchiveManager* archive_manager);
 
   ~OfflinePageStorageManager();
 
   // The manager would *try* to clear pages when called. It may not delete any
   // pages (if clearing condition wasn't satisfied).
   // It clears the storage (expire pages) when it's using more disk space than a
@@ skipping 15 matching lines @@
     NOT_NEEDED,
   };
 
   // Callback called after getting storage stats from archive manager.
   void OnGetStorageStatsDone(const ClearPagesCallback& callback,
                              const ArchiveManager::StorageStats& pages);
 
   // Callback called after getting all pages from client done.
   void OnGetAllPagesDone(const ClearPagesCallback& callback,
                          const ArchiveManager::StorageStats& storage_stats,
+                         const base::Time& now,
                          const MultipleOfflinePageItemResult& pages);
 
   // Callback called after expired pages have been deleted.
-  void OnExpiredPagesDeleted(const ClearPagesCallback& callback,
-                             int pages_to_clear,
-                             DeletePageResult result);
+  void OnPagesExpired(const ClearPagesCallback& callback,
+                      size_t pages_to_clear,
+                      std::vector<int64_t>* page_ids_to_remove,
+                      bool success);
 
-  // Gets offline IDs of all expired pages and return in |offline_ids|.
-  void GetExpiredPageIds(const MultipleOfflinePageItemResult& pages,
+  // Callback called after clearing outdated pages from client.
+  void OnDeadPagesCleared(const ClearPagesCallback& callback,

[jianli, 2016/05/26 21:59:32]

replace Dead with Outdated

[romax, 2016/05/27 00:41:38]

Done.

+                          size_t pages_cleared,
+                          bool success,
+                          DeletePageResult result);
+
+  // Gets offline IDs of both pages that should be expired and the ones that
+  // need to be removed from metadata store. |page_ids_to_expire| will have
+  // the pages to be expired, |page_ids_to_remove| will have the pages to be
+  // removed. |now| is used to keep the time consistecy with other operations.
+  void GetPageIdsToClear(const MultipleOfflinePageItemResult& pages,
                          const ArchiveManager::StorageStats& stats,
-                         std::vector<int64_t>& offline_ids);
+                         const base::Time& now,
+                         std::vector<int64_t>* page_ids_to_expire,
+                         std::vector<int64_t>* page_ids_to_remove);
 
   // Determine if manager should clear pages.
-  ClearMode ShouldClearPages(const ArchiveManager::StorageStats& storage_stats);
+  ClearMode ShouldClearPages(const ArchiveManager::StorageStats& storage_stats,
+                             const base::Time& now);
 
   // Return true if |page| is expired comparing to |now|.
   bool ShouldBeExpired(const base::Time& now, const OfflinePageItem& page);
 
   // Not owned.
   Client* client_;
 
   // Not owned.
   ClientPolicyController* policy_controller_;
 
   // Not owned.
   ArchiveManager* archive_manager_;
 
   bool in_progress_;
 
   base::Time last_clear_time_;
 
   // Clock for getting time.
   std::unique_ptr<base::Clock> clock_;
 
   base::WeakPtrFactory<OfflinePageStorageManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(OfflinePageStorageManager);
 };
 
 }  // namespace offline_pages
 
 #endif  // COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_STORAGE_MANAGER_H_