[Offline Pages] Implement OfflinePageStorageManager.

components/offline_pages/offline_page_storage_manager.cc

Index: components/offline_pages/offline_page_storage_manager.cc
diff --git a/components/offline_pages/offline_page_storage_manager.cc b/components/offline_pages/offline_page_storage_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a860e27ead9a4e8148c3e0c37e8198156f0765da
--- /dev/null
+++ b/components/offline_pages/offline_page_storage_manager.cc
@@ -0,0 +1,86 @@
+// 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.
+
+#include "components/offline_pages/offline_page_storage_manager.h"
+
+#include "base/bind.h"
+#include "components/offline_pages/offline_page_client_policy.h"
+#include "components/offline_pages/offline_page_item.h"
+
+using DeletePageResult = offline_pages::OfflinePageModel::DeletePageResult;
+using MultipleOfflinePageItemResult =
+    offline_pages::OfflinePageModel::MultipleOfflinePageItemResult;
+
+namespace offline_pages {
+
+OfflinePageStorageManager::OfflinePageStorageManager(OfflinePageModel* model,
+                                                     int64_t totalSize)
+    : model_(model),
+      policy_controller_(model->GetPolicyController()),
+      in_progress_(false),
+      size_(totalSize),
+      weak_ptr_factory_(this) {
+}
+
+OfflinePageStorageManager::~OfflinePageStorageManager() {
+}
+
+void OfflinePageStorageManager::ClearPagesIfNeeded(
+    const ClearPageCallback& callback) {
+  if (in_progress_ || !ShouldClearPages())
+    return;
+  in_progress_ = true;
+  model_->GetAllPages(base::Bind(&OfflinePageStorageManager::PurgePages,
+                                 weak_ptr_factory_.GetWeakPtr(), callback));
+}
+
+void OfflinePageStorageManager::PurgePages(
+    const ClearPageCallback& callback,
+    const MultipleOfflinePageItemResult& pages) {
+  std::vector<int64_t> offline_ids;
+  int pages_to_clear = GetExpiredPageIds(pages, offline_ids);

[fgorski, 2016/05/05 04:53:08]

what is the problem you are trying to solve with an extra method?
Let's talk about that.

[romax, 2016/05/05 21:00:02]

Acknowledged.

+  model_->DeletePagesByOfflineId(
+      offline_ids,
+      base::Bind(&OfflinePageStorageManager::OnExpiredPagesDeleted,
+                 weak_ptr_factory_.GetWeakPtr(), callback, pages_to_clear));
+}
+
+int OfflinePageStorageManager::GetExpiredPageIds(
+    const MultipleOfflinePageItemResult& pages,
+    std::vector<int64_t>& offline_ids) {
+  base::Time now = base::Time::Now();

[fgorski, 2016/05/05 04:53:08]

this will be more flexible and testable using base::Clock

E.g. like here:
https://code.google.com/p/chromium/codesearch#chromium/src/components/gcm_dri...

You can do it in a separate patch.

[romax, 2016/05/05 21:00:03]

Acknowledged.

+  for (const auto& page : pages) {
+    if (now - page.last_access_time >
+        policy_controller_->GetPolicy(page.client_id.name_space)
+            .lifetime_policy.expiration_period) {
+      offline_ids.push_back(page.offline_id);
+    }
+  }
+  return offline_ids.size();

[fgorski, 2016/05/05 04:53:08]

this information is already available outside through offline_ids.

[romax, 2016/05/05 21:00:02]

Done.

+}
+
+void OfflinePageStorageManager::OnExpiredPagesDeleted(
+    const ClearPageCallback& callback,
+    int pages_to_clear,
+    DeletePageResult result) {
+  if (result != DeletePageResult::SUCCESS)

[fgorski, 2016/05/05 04:53:08]

this may not be exactly what you want here.

[romax, 2016/05/05 21:00:02]

let's talk about this...

+    pages_to_clear = 0;
+  callback.Run(pages_to_clear);
+}
+
+bool OfflinePageStorageManager::ShouldClearPages() {
+  return model_->is_loaded();

[fgorski, 2016/05/05 04:53:08]

We should pretty much always clear pages?
Where is the throttling mentioned in the documentation?

[romax, 2016/05/05 21:00:03]

I also mentioned it's not implemented yet in the CL comment...

+}
+
+bool OfflinePageStorageManager::isPageExpired(const OfflinePageItem& page) {

[fgorski, 2016/05/05 04:53:08]

I think you meant to use this method above?

[romax, 2016/05/05 21:00:02]

yeah.. red bull killed me...

+  DCHECK(model_);

[fgorski, 2016/05/05 04:53:08]

why?

[romax, 2016/05/05 21:00:02]

Done.

+  base::Time now = base::Time::Now();
+  LifetimePolicy policy =
+      policy_controller_->GetPolicy(page.client_id.name_space).lifetime_policy;
+  if (now - page.last_access_time > policy.expiration_period)
+    return true;
+  return false;

[fgorski, 2016/05/05 04:53:08]

three lines above is a one-liner (return is bool)

[romax, 2016/05/05 21:00:02]

Done.

+}
+
+}  // namespace offline_pages