Refactor cache counting into a separate helper class

chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc

+// Copyright (c) 2015 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 <algorithm>
+#include <memory>
+#include <set>
+#include <string>
+
+#include "base/run_loop.h"
+#include "chrome/browser/browsing_data/cache_test_util.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/browsing_data/content/conditional_cache_counting_helper.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
+
+using browsing_data::ConditionalCacheCountingHelper;
+using content::BrowserThread;
+
+class ConditionalCacheCountingHelperBrowserTest : public InProcessBrowserTest {
+ public:
+  // Initialization ------------------------------------------------------------
+
+  const int64_t kTimeoutMs = 10;
+
+  void SetUpOnMainThread() override {
+    count_callback_ =
+        base::Bind(&ConditionalCacheCountingHelperBrowserTest::CountCallback,
+                   base::Unretained(this));
+
+    cache_util_ = base::MakeUnique<CacheTestUtil>(
+        content::BrowserContext::GetDefaultStoragePartition(
+            browser()->profile()));
+    cache_util_->SetUpOnMainThread();
+  }
+
+  void TearDownOnMainThread() override { cache_util_->TearDownOnMainThread(); }
+
+  // Counting helpers. ---------------------------------------------------------

[msramek, 2016/12/20 01:03:00]

nit: The section delimiter is not needed now that there's only one small
section.

[dullweber, 2016/12/21 10:29:19]

Done.

+
+  void CountCallback(int64_t size) {
+    // Negative values represent an unexpected error.
+    DCHECK(size >= 0 || size == net::ERR_ABORTED);
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    last_size_ = size;
+
+    if (run_loop_)
+      run_loop_->Quit();
+  }
+
+  void WaitForTasksOnIOThread() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
+  }
+
+  void CountEntries(base::Time begin_time, base::Time end_time) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    last_size_ = -1;
+    auto helper = ConditionalCacheCountingHelper::CreateForRange(
+        cache_util_->partition(), begin_time, end_time);
+    helper->CountAndDestroySelfWhenFinished(count_callback_);
+  }
+
+  void CountEntriesAndCancel(base::Time begin_time, base::Time end_time) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    last_size_ = -1;
+    auto helper = ConditionalCacheCountingHelper::CreateForRange(
+        cache_util_->partition(), begin_time, end_time);
+    // Cancel first to make sure we don't cancel after the IO thread finishes.
+    helper->CancelCounting();
+    helper->CountAndDestroySelfWhenFinished(count_callback_);
+  }
+
+  int64_t GetResult() {
+    DCHECK_GT(last_size_, 0);
+    return last_size_;
+  }
+
+  int64_t GetResultOrError() { return last_size_; }
+
+ protected:
+  std::unique_ptr<CacheTestUtil> cache_util_;

[msramek, 2016/12/20 01:02:59]

It's unusual to inherit member variables. The naming is technically also wrong,
as _$ means private.

Could you make this private and instead expose

protected:
 CacheTestUtil* GetCacheTestUtil();

?

[dullweber, 2016/12/21 10:29:19]

Done.

+
+ private:
+  ConditionalCacheCountingHelper::CacheCountCallback count_callback_;
+  std::unique_ptr<base::RunLoop> run_loop_;
+
+  int64_t last_size_;
+};
+
+// Tests that ConditionalCacheCountingHelper only counts those cache entries
+// that match the condition.
+IN_PROC_BROWSER_TEST_F(ConditionalCacheCountingHelperBrowserTest, Count) {
+  // Create 5 entries.
+  std::set<std::string> keys1 = {"1", "2", "3", "4", "5"};
+
+  base::Time t1 = base::Time::Now();
+  cache_util_->CreateCacheEntries(keys1);
+
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs));
+  base::Time t2 = base::Time::Now();
+
+  std::set<std::string> keys2 = {"6", "7"};
+  cache_util_->CreateCacheEntries(keys2);
+
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs));
+  base::Time t3 = base::Time::Now();
+
+  // Count the size of the first set of entries.
+  CountEntries(t1, t2);
+  WaitForTasksOnIOThread();
+  int64_t size_1_2 = GetResult();
+
+  // Count the size of the second set of entries.
+  CountEntries(t2, t3);
+  WaitForTasksOnIOThread();
+  int64_t size_2_3 = GetResult();
+
+  // Count all entries.
+  CountEntries(t1, t3);
+  WaitForTasksOnIOThread();
+  int64_t size_1_3 = GetResult();
+  EXPECT_EQ(size_1_2 + size_2_3, size_1_3);
+
+  // Count everything
+  CountEntries(base::Time(), base::Time::Max());
+  WaitForTasksOnIOThread();
+  EXPECT_EQ(size_1_3, GetResult());
+}
+
+// Tests that ConditionalCacheCountingHelper returns net::ERR_ABORTED when
+// cancelled.
+IN_PROC_BROWSER_TEST_F(ConditionalCacheCountingHelperBrowserTest,
+                       CancelCounting) {
+  std::set<std::string> keys = {"1", "2", "3", "4", "5"};
+  base::Time t1 = base::Time::Now();
+  cache_util_->CreateCacheEntries(keys);
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs));
+
+  // Count and cancel
+  CountEntriesAndCancel(t1, base::Time::Now());
+  WaitForTasksOnIOThread();
+  EXPECT_EQ(net::ERR_ABORTED, GetResultOrError());
+}