[Offline Pages] Add cached offline page utils and show usage in settings.

components/offline_pages/core/cached_offline_page_utils_unittest.cc

Index: components/offline_pages/core/cached_offline_page_utils_unittest.cc
diff --git a/components/offline_pages/core/cached_offline_page_utils_unittest.cc b/components/offline_pages/core/cached_offline_page_utils_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..bbb786e5f7fe21e98a7243ea786b6defa57c3b34
--- /dev/null
+++ b/components/offline_pages/core/cached_offline_page_utils_unittest.cc
@@ -0,0 +1,214 @@
+// 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/core/cached_offline_page_utils.h"
+
+#include <stdint.h>
+#include <map>
+
+#include "base/bind.h"
+#include "base/memory/weak_ptr.h"
+#include "base/test/simple_test_clock.h"
+#include "base/time/time.h"
+#include "components/offline_pages/core/client_policy_controller.h"
+#include "components/offline_pages/core/offline_page_item.h"
+#include "components/offline_pages/core/offline_page_model_query.h"
+#include "components/offline_pages/core/offline_page_types.h"
+#include "components/offline_pages/core/stub_offline_page_model.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace offline_pages {
+
+namespace {
+// Constants for test OfflinePageItems.
+const char kTemporaryNamespace[] = "temporary_namespace";
+const char kPersistentNamespace[] = "persistent_namespace";
+const GURL kTestUrl("http://foo.com/bar.html");
+const GURL kTestUrl2("http://bar.com/foo");
+const GURL kTestUrl3("http://42.com");
+const int64_t kOfflineId1 = 1234LL;
+const int64_t kOfflineId2 = 5678LL;
+const int64_t kOfflineId3 = 42LL;
+const int64_t kOfflineId4 = 123LL;
+const ClientId kTempClientId1(kTemporaryNamespace, "1234");
+const ClientId kTempClientId2(kTemporaryNamespace, "5678");
+const ClientId kTempClientId3(kTemporaryNamespace, "123");
+const ClientId kPersistentClientId(kPersistentNamespace, "42");
+const int64_t kTestFileSize1 = 131415LL;
+const int64_t kTestFileSize2 = 666666LL;
+const int64_t kTestFileSize3 = 500000LL;
+const base::FilePath kTestFilePath =
+    base::FilePath(FILE_PATH_LITERAL("file_path"));
+}  // namespace
+
+class CachedOfflinePageTestModel : public StubOfflinePageModel {

[dewittj, 2017/05/03 21:40:51]

no action required: I think we are overdue for a nicer fake model.

[romax, 2017/05/04 04:20:37]

Acknowledged.

+ public:
+  CachedOfflinePageTestModel();
+  ~CachedOfflinePageTestModel() override;
+
+  void GetPagesMatchingQuery(
+      std::unique_ptr<OfflinePageModelQuery> query,
+      const MultipleOfflinePageItemCallback& callback) override;
+  void GetAllPages(const MultipleOfflinePageItemCallback& callback) override;
+
+  ClientPolicyController* GetPolicyController() override;
+
+  void AddOfflinePage(const OfflinePageItem& page);
+
+ private:
+  std::map<int64_t, OfflinePageItem> offline_pages_;
+  std::unique_ptr<ClientPolicyController> policy_controller_;
+};
+
+CachedOfflinePageTestModel::CachedOfflinePageTestModel()
+    : policy_controller_(new ClientPolicyController()) {
+  // Adds the policies for test namespaces.
+  policy_controller_->AddPolicyForTest(
+      kTemporaryNamespace,
+      OfflinePageClientPolicyBuilder(kTemporaryNamespace,
+                                     LifetimePolicy::LifetimeType::TEMPORARY,
+                                     kUnlimitedPages, kUnlimitedPages));
+  policy_controller_->AddPolicyForTest(
+      kPersistentNamespace,
+      OfflinePageClientPolicyBuilder(kPersistentNamespace,
+                                     LifetimePolicy::LifetimeType::PERSISTENT,
+                                     kUnlimitedPages, kUnlimitedPages)
+          .SetIsRemovedOnCacheReset(false));
+}
+
+CachedOfflinePageTestModel::~CachedOfflinePageTestModel() {}
+
+void CachedOfflinePageTestModel::GetPagesMatchingQuery(
+    std::unique_ptr<OfflinePageModelQuery> query,
+    const MultipleOfflinePageItemCallback& callback) {
+  MultipleOfflinePageItemResult result;
+  for (const auto& page : offline_pages_) {
+    if (query->Matches(page.second))
+      result.push_back(page.second);
+  }
+  callback.Run(result);
+}
+
+void CachedOfflinePageTestModel::GetAllPages(
+    const MultipleOfflinePageItemCallback& callback) {
+  MultipleOfflinePageItemResult result;
+  for (const auto& page : offline_pages_)
+    result.push_back(page.second);
+  callback.Run(result);
+}
+
+ClientPolicyController* CachedOfflinePageTestModel::GetPolicyController() {
+  return policy_controller_.get();
+}
+
+void CachedOfflinePageTestModel::AddOfflinePage(const OfflinePageItem& page) {
+  offline_pages_[page.offline_id] = page;
+}
+
+class CachedOfflinePageUtilsTest
+    : public testing::Test,
+      public base::SupportsWeakPtr<CachedOfflinePageUtilsTest> {
+ public:
+  CachedOfflinePageUtilsTest();
+
+  // testing::Test
+  void SetUp() override;
+  void TearDown() override;
+
+  void AddPage(const GURL& url,
+               const int64_t offline_id,
+               const ClientId& client_id,
+               const base::FilePath& path,
+               const int64_t file_size,
+               const base::Time& creation_time);
+  MultipleOfflinePageItemResult GetAllPages();
+
+  void OnSizeCalculated(int64_t size);
+  void OnGetMultipleOfflinePageItemsResult(
+      MultipleOfflinePageItemResult* storage,
+      const MultipleOfflinePageItemResult& result);
+
+  OfflinePageModel* model() { return model_.get(); }
+  base::SimpleTestClock* clock() { return clock_.get(); }
+
+  int64_t last_cache_size() { return last_cache_size_; }
+
+ private:
+  std::unique_ptr<CachedOfflinePageTestModel> model_;

[dewittj, 2017/05/03 21:40:53]

I think this doesn't need to be a unique_ptr, just a CachedOfflinePageTestModel
object.

[romax, 2017/05/04 04:20:37]

Done.

+  std::unique_ptr<base::SimpleTestClock> clock_;

[dewittj, 2017/05/03 21:40:53]

same, don't think unique_ptr is necessary.

[romax, 2017/05/04 04:20:37]

Done.

+  int64_t last_cache_size_;
+};
+
+CachedOfflinePageUtilsTest::CachedOfflinePageUtilsTest()
+    : model_(new CachedOfflinePageTestModel),
+      clock_(new base::SimpleTestClock()),
+      last_cache_size_(0) {}
+
+void CachedOfflinePageUtilsTest::SetUp() {
+  // Add 3 pages to the model used for test cases.
+  clock_->SetNow(base::Time::Now());
+  // Time 00:00:00.
+  AddPage(kTestUrl, kOfflineId1, kTempClientId1, kTestFilePath, kTestFileSize1,
+          clock()->Now());
+  // time 02:00:00.
+  clock()->Advance(base::TimeDelta::FromHours(2));
+  AddPage(kTestUrl, kOfflineId2, kPersistentClientId, kTestFilePath,
+          kTestFileSize1, clock()->Now());
+  // time 03:00:00.
+  clock()->Advance(base::TimeDelta::FromHours(1));
+  AddPage(kTestUrl2, kOfflineId3, kTempClientId2, kTestFilePath, kTestFileSize2,
+          clock()->Now());
+  // Add a temporary page to test boundary at base::Time::Max().
+  AddPage(kTestUrl3, kOfflineId4, kTempClientId3, kTestFilePath, kTestFileSize3,
+          base::Time::Max());

[carlosk, 2017/05/03 21:54:35]

Time::Max is not a good time for an entry; it should be a "normal"/valid time
instead. Especially now that the upper limit is open this time would never be a
match.

[romax, 2017/05/04 04:20:37]

Done.

+  ASSERT_EQ(4UL, GetAllPages().size());

[dewittj, 2017/05/03 21:40:51]

ASSERT in SetUp seems strange to me.

[carlosk, 2017/05/03 21:54:35]

I disagree. This is testing an assumption that all tests using this data set
depend on to work properly.

[romax, 2017/05/04 04:20:37]

THere's no easy way (and no one did) to get all pages in a sync way (in this
file). To keep with tradition, i'm removing this assert.

+}
+
+void CachedOfflinePageUtilsTest::TearDown() {
+  model_.reset();

[dewittj, 2017/05/03 21:40:52]

Don't think this is necessary.

[romax, 2017/05/04 04:20:37]

Done.

+  clock_.reset();
+}
+
+void CachedOfflinePageUtilsTest::AddPage(const GURL& url,
+                                         const int64_t offline_id,
+                                         const ClientId& client_id,
+                                         const base::FilePath& path,
+                                         const int64_t file_size,
+                                         const base::Time& creation_time) {
+  OfflinePageItem page(url, offline_id, client_id, path, file_size,
+                       creation_time);
+  model_->AddOfflinePage(page);
+}
+
+MultipleOfflinePageItemResult CachedOfflinePageUtilsTest::GetAllPages() {
+  MultipleOfflinePageItemResult result;
+  model()->GetAllPages(base::Bind(
+      &CachedOfflinePageUtilsTest::OnGetMultipleOfflinePageItemsResult,
+      AsWeakPtr(), base::Unretained(&result)));
+  return result;
+}
+
+void CachedOfflinePageUtilsTest::OnSizeCalculated(int64_t size) {

[dewittj, 2017/05/03 21:40:52]

why do you have multiple paradigms for the callbacks (one uses |storage| and the
other uses member variables)

[romax, 2017/05/04 04:20:37]

Done.

+  last_cache_size_ = size;
+}
+
+void CachedOfflinePageUtilsTest::OnGetMultipleOfflinePageItemsResult(
+    MultipleOfflinePageItemResult* storage,
+    const MultipleOfflinePageItemResult& result) {
+  *storage = result;
+}
+
+TEST_F(CachedOfflinePageUtilsTest, TestGetCachedOfflinePageSizeBetween) {

[dewittj, 2017/05/03 21:40:51]

nit: moar tests plz, eg:
* no pages in model
* no pages in range
* all pages in range
* invalid range

[romax, 2017/05/04 04:20:37]

Done.

+  // Advance the clock so that we don't hit the time check boundary.
+  clock()->Advance(base::TimeDelta::FromMinutes(5));
+
+  // Get the size of cached offline pages between 01:05:00 and 03:05:00.
+  GetCachedOfflinePageSizeBetween(
+      model(),
+      base::Bind(&CachedOfflinePageUtilsTest::OnSizeCalculated, AsWeakPtr()),
+      clock()->Now() - base::TimeDelta::FromHours(2), clock()->Now());
+  EXPECT_EQ(last_cache_size(), kTestFileSize2);
+}
+
+}  // namespace offline_pages