Refactor cache counting into a separate helper class

chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc

Index: chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc
diff --git a/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc b/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a4b583cbd262b481d5cd1fc8ab78407016235b2d
--- /dev/null
+++ b/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc
@@ -0,0 +1,238 @@
+// 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 <vector>
+
+#include "base/run_loop.h"
+#include "base/threading/platform_thread.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"
+#include "content/public/browser/storage_partition.h"
+#include "net/disk_cache/disk_cache.h"
+#include "net/http/http_cache.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
+
+using browsing_data::ConditionalCacheCountingHelper;
+using content::BrowserThread;
+
+class ConditionalCacheCountingHelperBrowserTest : public InProcessBrowserTest {

[msramek, 2016/12/15 15:58:50]

Optional: I wonder if we could extract the common parts (mainly all the helper
functions) from this and CCDH browsertest to something like a
cache_iteration_test_utils.h file.

[dullweber, 2016/12/16 16:41:10]

I extracted creating and retrieving cache entries into a separate class

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

Thanks!

+ public:
+  // Initialization ------------------------------------------------------------
+
+  const int64_t kTimeoutMs = 1;

[msramek, 2016/12/15 15:58:50]

Let's try 10ms. The CCDH browsertest got a lot of attention from sheriffs for
being flaky due to a short timeout.

[dullweber, 2016/12/16 16:41:10]

Done.

+
+  void SetUpOnMainThread() override {
+    // Prepare the commonly used callbacks.
+    done_callback_ =
+        base::Bind(&ConditionalCacheCountingHelperBrowserTest::DoneCallback,
+                   base::Unretained(this));
+
+    count_callback_ =
+        base::Bind(&ConditionalCacheCountingHelperBrowserTest::CountCallback,
+                   base::Unretained(this));
+
+    // Get the storage partition.
+    partition_ = content::BrowserContext::GetDefaultStoragePartition(
+        browser()->profile());
+
+    // Get the cache backends.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&ConditionalCacheCountingHelperBrowserTest::SetUpOnIOThread,
+                   base::Unretained(this)));
+    WaitForTasksOnIOThread();
+  }
+
+  void SetUpOnIOThread() {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    net::URLRequestContextGetter* context = partition_->GetURLRequestContext();
+
+    net::HttpCache* cache =
+        context->GetURLRequestContext()->http_transaction_factory()->GetCache();
+
+    SetNumberOfWaitedTasks(1);
+    WaitForCompletion(cache->GetBackend(&backend_, done_callback_));
+  }
+
+  // Waiting for tasks to be done on IO thread. --------------------------------
+
+  void WaitForTasksOnIOThread() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
+  }
+
+  void SetNumberOfWaitedTasks(int count) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    remaining_tasks_ = count;
+  }
+
+  void WaitForCompletion(int value) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    if (value >= 0) {
+      // We got the result immediately.
+      DoneCallback(value);
+    } else if (value == net::ERR_IO_PENDING) {
+      // We need to wait for the callback.
+    } else {
+      // An error has occurred.
+      NOTREACHED();
+    }
+  }
+
+  void DoneCallback(int value) {
+    DCHECK_GE(value, 0);  // Negative values represent an error.
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    if (--remaining_tasks_ > 0)
+      return;
+
+    if (run_loop_) {
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(&base::RunLoop::Quit, base::Unretained(run_loop_.get())));
+    }
+  }
+
+  // Cache operation shorthands. -----------------------------------------------
+
+  void CreateCacheEntries(const std::set<std::string>& keys) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+    entries_.resize(keys.size());
+    SetNumberOfWaitedTasks(keys.size());
+
+    int pos = 0;
+    for (const std::string& key : keys) {
+      WaitForCompletion(
+          backend_->CreateEntry(key, &entries_[pos++], done_callback_));
+    }
+  }
+
+  // Counting helpers. ---------------------------------------------------------
+
+  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 CountEntries(base::Time begin_time, base::Time end_time) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    last_size_ = -1;
+    auto helper = ConditionalCacheCountingHelper::CreateForRange(
+        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(
+        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_; }
+
+  // Miscellaneous. ------------------------------------------------------------

[msramek, 2016/12/15 15:58:50]

This section looks empty :)

[dullweber, 2016/12/16 16:41:10]

Done.

+ private:
+  content::StoragePartition* partition_;
+  disk_cache::Backend* backend_ = nullptr;
+  std::vector<disk_cache::Entry*> entries_;
+
+  base::Callback<void(int)> done_callback_;
+  ConditionalCacheCountingHelper::CacheCountCallback count_callback_;
+
+  std::unique_ptr<base::RunLoop> run_loop_;
+  int remaining_tasks_;
+  int64_t last_size_;
+};
+
+// Tests that ConditionalCacheCountingHelper only counts those cache entries
+// that match the condition.
+IN_PROC_BROWSER_TEST_F(ConditionalCacheCountingHelperBrowserTest,
+                       CountingWithTimeout) {

[msramek, 2016/12/15 15:58:50]

What does "Timeout" refer to?

[dullweber, 2016/12/16 16:41:10]

It doesn't refer to anything anymore, I forgot to remove it.

+  // Create 5 entries.
+  std::set<std::string> keys1 = {"1", "2", "3", "4", "5"};
+
+  base::Time t1 = base::Time::Now();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&ConditionalCacheCountingHelperBrowserTest::CreateCacheEntries,
+                 base::Unretained(this), base::ConstRef(keys1)));
+  WaitForTasksOnIOThread();
+
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs));
+  base::Time t2 = base::Time::Now();
+
+  std::set<std::string> keys2 = {"6", "7"};
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&ConditionalCacheCountingHelperBrowserTest::CreateCacheEntries,
+                 base::Unretained(this), base::ConstRef(keys2)));
+  WaitForTasksOnIOThread();
+
+  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();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&ConditionalCacheCountingHelperBrowserTest::CreateCacheEntries,
+                 base::Unretained(this), base::ConstRef(keys)));
+  WaitForTasksOnIOThread();
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(kTimeoutMs));
+
+  // Count and cancel
+  CountEntriesAndCancel(t1, base::Time::Now());
+  WaitForTasksOnIOThread();
+  EXPECT_EQ(net::ERR_ABORTED, GetResultOrError());
+}