Make SQLitePersistentCookieStore run on injected SequencedTaskRunners vs named BrowserThreads.

chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc

 // Copyright (c) 2012 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 <map>
 #include <set>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
+#include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "base/synchronization/waitable_event.h"
-#include "base/test/thread_test_helper.h"
+#include "base/test/sequenced_worker_pool_owner.h"
+#include "base/threading/sequenced_worker_pool.h"
 #include "base/time.h"
 #include "chrome/browser/net/clear_on_exit_policy.h"
 #include "chrome/browser/net/sqlite_persistent_cookie_store.h"
 #include "chrome/common/chrome_constants.h"
-#include "content/public/test/test_browser_thread.h"
 #include "googleurl/src/gurl.h"
 #include "net/cookies/canonical_cookie.h"
 #include "sql/connection.h"
 #include "sql/meta_table.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/quota/mock_special_storage_policy.h"
 
-using content::BrowserThread;
-
 typedef std::vector<net::CanonicalCookie*> CanonicalCookieVector;
 
 class SQLitePersistentCookieStoreTest : public testing::Test {
  public:
   SQLitePersistentCookieStoreTest()
-      : ui_thread_(BrowserThread::UI),
-        db_thread_(BrowserThread::DB),
-        io_thread_(BrowserThread::IO),
+      : pool_owner_(new base::SequencedWorkerPoolOwner(1, "Background Pool")),
         loaded_event_(false, false),
         key_loaded_event_(false, false),
         db_thread_event_(false, false) {
   }
 
   void OnLoaded(const CanonicalCookieVector& cookies) {
     cookies_ = cookies;
     loaded_event_.Signal();
   }
 
   void OnKeyLoaded(const CanonicalCookieVector& cookies) {
     cookies_ = cookies;
     key_loaded_event_.Signal();
   }
 
   void Load(CanonicalCookieVector* cookies) {
+    EXPECT_FALSE(loaded_event_.IsSignaled());
     store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
                             base::Unretained(this)));
     loaded_event_.Wait();
     *cookies = cookies_;
   }
 
+  void Flush() {
+    base::WaitableEvent event(false, false);
+    store_->Flush(base::Bind(&base::WaitableEvent::Signal,
+                             base::Unretained(&event)));
+    event.Wait();
+  }
+
+  scoped_refptr<base::SequencedTaskRunner> background_task_runner() {
+    return pool_owner_->pool()->GetSequencedTaskRunner(
+        pool_owner_->pool()->GetNamedSequenceToken("background"));
+  }
+
+  scoped_refptr<base::SequencedTaskRunner> client_task_runner() {
+    return pool_owner_->pool()->GetSequencedTaskRunner(
+        pool_owner_->pool()->GetNamedSequenceToken("client"));
+  }
+
   void DestroyStore() {
     store_ = NULL;
-    // Make sure we wait until the destructor has run by waiting for all pending
-    // tasks on the DB thread to run.
-    scoped_refptr<base::ThreadTestHelper> helper(
-        new base::ThreadTestHelper(
-            BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-    ASSERT_TRUE(helper->Run());
+    // Make sure we wait until the destructor has run by shutting down the pool
+    // resetting the owner (whose destructor blocks on the pool completion).
+    pool_owner_->pool()->Shutdown();
+    // Create a new pool for the few tests that create multiple stores. In other
+    // cases this is wasted but harmless.
+    pool_owner_.reset(new base::SequencedWorkerPoolOwner(1, "Background Pool"));
   }
 
   void CreateAndLoad(bool restore_old_session_cookies,
                      CanonicalCookieVector* cookies) {
     store_ = new SQLitePersistentCookieStore(
         temp_dir_.path().Append(chrome::kCookieFilename),
+        client_task_runner(),
+        background_task_runner(),
         restore_old_session_cookies,
         NULL);
     Load(cookies);
   }
 
   void InitializeStore(bool restore_old_session_cookies) {
     CanonicalCookieVector cookies;
     CreateAndLoad(restore_old_session_cookies, &cookies);
-    ASSERT_EQ(0U, cookies.size());
+    EXPECT_EQ(0U, cookies.size());

[Randy Smith (Not in Mondays), 2013/02/22 18:20:16]

I know this is probably my cluelessness, but why the change?

[erikwright (departed), 2013/02/25 16:25:47]

ASSERT is designed to terminate a test. It does so by returning from the test
body (which is defined as void). Thus you cannot use ASSERT in helper methods
with return types.

Most developers (this is from direct observation) assume that ASSERT is
terminating the test, without understanding why. So using it in a void helper
method is misleading. It is unexpected (or, at best, an intended side-effect
that requires detailed knowledge of the implementation of GTest) that it causes
the helper to immediately terminate while allowing the test to continue.

It is more correct to use EXPECT and ADD_FAILURE (vs FAIL) anywhere outside main
test bodies and SetUp/TearDown.

   }
 
   // We have to create this method to wrap WaitableEvent::Wait, since we cannot
   // bind a non-void returning method as a Closure.
   void WaitOnDBEvent() {
     db_thread_event_.Wait();
   }
 
   // Adds a persistent cookie to store_.
   void AddCookie(const std::string& name,
                  const std::string& value,
                  const std::string& domain,
                  const std::string& path,
                  const base::Time& creation) {
     store_->AddCookie(
         net::CanonicalCookie(GURL(), name, value, domain, path, std::string(),
                              std::string(), creation, creation, creation, false,
                              false));
   }
 
-  virtual void SetUp() {
-    ui_thread_.Start();
-    db_thread_.Start();
-    io_thread_.Start();
+  virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   }
 
+  virtual void TearDown() OVERRIDE {
+    DestroyStore();
+    pool_owner_->pool()->Shutdown();
+  }
+
  protected:
-  content::TestBrowserThread ui_thread_;
-  content::TestBrowserThread db_thread_;
-  content::TestBrowserThread io_thread_;
+  MessageLoop main_loop_;
+  scoped_ptr<base::SequencedWorkerPoolOwner> pool_owner_;
   base::WaitableEvent loaded_event_;
   base::WaitableEvent key_loaded_event_;
   base::WaitableEvent db_thread_event_;
   CanonicalCookieVector cookies_;
   base::ScopedTempDir temp_dir_;
   scoped_refptr<SQLitePersistentCookieStore> store_;
 };
 
 TEST_F(SQLitePersistentCookieStoreTest, TestInvalidMetaTableRecovery) {
   InitializeStore(false);
@@ skipping 69 matching lines @@
   AddCookie("A", "B", "foo.bar", "/", t);
   t += base::TimeDelta::FromInternalValue(10);
   AddCookie("A", "B", "www.aaa.com", "/", t);
   t += base::TimeDelta::FromInternalValue(10);
   AddCookie("A", "B", "travel.aaa.com", "/", t);
   t += base::TimeDelta::FromInternalValue(10);
   AddCookie("A", "B", "www.bbb.com", "/", t);
   DestroyStore();
 
   store_ = new SQLitePersistentCookieStore(
-      temp_dir_.path().Append(chrome::kCookieFilename), false, NULL);
+      temp_dir_.path().Append(chrome::kCookieFilename),
+      client_task_runner(),
+      background_task_runner(),
+      false, NULL);
   // Posting a blocking task to db_thread_ makes sure that the DB thread waits
   // until both Load and LoadCookiesForKey have been posted to its task queue.
-  BrowserThread::PostTask(
-      BrowserThread::DB, FROM_HERE,
+  background_task_runner()->PostTask(
+      FROM_HERE,
       base::Bind(&SQLitePersistentCookieStoreTest::WaitOnDBEvent,
                  base::Unretained(this)));
   store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
                           base::Unretained(this)));
   store_->LoadCookiesForKey("aaa.com",
     base::Bind(&SQLitePersistentCookieStoreTest::OnKeyLoaded,
                base::Unretained(this)));
-  BrowserThread::PostTask(
-      BrowserThread::DB, FROM_HERE,
+  background_task_runner()->PostTask(
+      FROM_HERE,
       base::Bind(&SQLitePersistentCookieStoreTest::WaitOnDBEvent,
                  base::Unretained(this)));
 
   // Now the DB-thread queue contains:
   // (active:)
   // 1. Wait (on db_event)
   // (pending:)
   // 2. "Init And Chain-Load First Domain"
   // 3. Priority Load (aaa.com)
   // 4. Wait (on db_event)
@@ skipping 38 matching lines @@
 
   // Write some large cookies, so the DB will have to expand by several KB.
   for (char c = 'a'; c < 'z'; ++c) {
     // Each cookie needs a unique timestamp for creation_utc (see DB schema).
     base::Time t = base::Time::Now() + base::TimeDelta::FromMicroseconds(c);
     std::string name(1, c);
     std::string value(1000, c);
     AddCookie(name, value, "foo.bar", "/", t);
   }
 
-  // Call Flush() and wait until the DB thread is idle.
-  store_->Flush(base::Closure());
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  ASSERT_TRUE(helper->Run());
+  Flush();
 
   // We forced a write, so now the file will be bigger.
   ASSERT_TRUE(file_util::GetFileInfo(path, &info));
   ASSERT_GT(info.size, base_size);
 }
 
-// Counts the number of times Callback() has been run.
-class CallbackCounter : public base::RefCountedThreadSafe<CallbackCounter> {
- public:
-  CallbackCounter() : callback_count_(0) {}
-
-  void Callback() {
-    ++callback_count_;
-  }
-
-  int callback_count() {
-    return callback_count_;
-  }
-
- private:
-  friend class base::RefCountedThreadSafe<CallbackCounter>;
-  ~CallbackCounter() {}
-
-  volatile int callback_count_;
-};
-
-// Test that we can get a completion callback after a Flush().
-TEST_F(SQLitePersistentCookieStoreTest, TestFlushCompletionCallback) {
-  InitializeStore(false);
-  // Put some data - any data - on disk, so that Flush is not a no-op.
-  AddCookie("A", "B", "foo.bar", "/", base::Time::Now());
-
-  scoped_refptr<CallbackCounter> counter(new CallbackCounter());
-
-  // Callback shouldn't be invoked until we call Flush().
-  ASSERT_EQ(0, counter->callback_count());
-
-  store_->Flush(base::Bind(&CallbackCounter::Callback, counter.get()));
-
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  ASSERT_TRUE(helper->Run());
-
-  ASSERT_EQ(1, counter->callback_count());
-}
-
 // Test loading old session cookies from the disk.
 TEST_F(SQLitePersistentCookieStoreTest, TestLoadOldSessionCookies) {
   InitializeStore(true);
 
   // Add a session cookie.
   store_->AddCookie(
       net::CanonicalCookie(
           GURL(), "C", "D", "sessioncookie.com", "/", std::string(),
           std::string(), base::Time::Now(), base::Time(),
           base::Time::Now(), false, false));
@@ skipping 81 matching lines @@
       cookie_map.find(kSessionName);
   ASSERT_TRUE(it != cookie_map.end());
   EXPECT_FALSE(cookie_map[kSessionName]->IsPersistent());
 
   it = cookie_map.find(kPersistentName);
   ASSERT_TRUE(it != cookie_map.end());
   EXPECT_TRUE(cookie_map[kPersistentName]->IsPersistent());
 
   STLDeleteElements(&cookies);
 }