Refactor this test to reduce code duplication.

chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc

 // Copyright (c) 2011 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 "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
 #include "base/scoped_temp_dir.h"
@@ skipping 32 matching lines @@
     key_loaded_event_.Signal();
   }
 
   void Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
     store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
                                 base::Unretained(this)));
     loaded_event_.Wait();
     *cookies = cookies_;
   }
 
+  void DestroyStore() {
+    store_ = NULL;
+    // Make sure we wait until the destructor has run.

[Randy Smith (Not in Mondays), 2012/03/01 22:12:10]

nit, suggestion: It's not obvious from the name what using ThreadTestHelper
without inheritance does, so you might want to add "by waiting for all pending
tasks on the DB thread to run."

[erikwright (departed), 2012/03/02 03:20:31]

Done.

+    scoped_refptr<base::ThreadTestHelper> helper(
+        new base::ThreadTestHelper(
+            BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
+    ASSERT_TRUE(helper->Run());
+  }
+
+  void CreateAndLoad(
+      bool restore_old_session_cookies,
+      std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
+    store_ = new SQLitePersistentCookieStore(
+        temp_dir_.path().Append(chrome::kCookieFilename),
+        restore_old_session_cookies);
+    Load(cookies);
+  }
+
+  void InitializeStore(bool restore_old_session_cookies) {
+    std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+    CreateAndLoad(restore_old_session_cookies, &cookies);
+    ASSERT_EQ(0u, cookies.size());
+  }
+
   // 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::CookieMonster::CanonicalCookie(GURL(), name, value, domain, path,
+                                            std::string(), std::string(),
+                                            creation, creation, creation,
+                                            false, false, true, true));
+  }
+
   virtual void SetUp() {
     ui_thread_.Start();
     db_thread_.Start();
     io_thread_.Start();
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    store_ = new SQLitePersistentCookieStore(
-        temp_dir_.path().Append(chrome::kCookieFilename), false);
-    std::vector<net::CookieMonster::CanonicalCookie*> cookies;
-    Load(&cookies);
-    ASSERT_EQ(0u, cookies.size());
-    // Make sure the store gets written at least once.
-    store_->AddCookie(
-        net::CookieMonster::CanonicalCookie(GURL(), "A", "B", "http://foo.bar",
-                                            "/", std::string(), std::string(),
-                                            base::Time::Now(),
-                                            base::Time::Now(),
-                                            base::Time::Now(),
-                                            false, false, true, true));
   }
 
  protected:
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread db_thread_;
   content::TestBrowserThread io_thread_;
   base::WaitableEvent loaded_event_;
   base::WaitableEvent key_loaded_event_;
   base::WaitableEvent db_thread_event_;
   std::vector<net::CookieMonster::CanonicalCookie*> cookies_;
   ScopedTempDir temp_dir_;
   scoped_refptr<SQLitePersistentCookieStore> store_;
 };
 
 TEST_F(SQLitePersistentCookieStoreTest, KeepOnDestruction) {
+  InitializeStore(false);
+  AddCookie("A", "B", "http://foo.bar", "/", base::Time::Now());

[Randy Smith (Not in Mondays), 2012/03/01 22:12:10]

Hmmm.  This is only a suggestion (up to you) but the purpose of this line isn't
obvious.  I presume it's to replace the "make sure the store gets written at
least once" AddCookie that used to be in SetUp(), but the code would be clearer
if that was in a helper function, maybe one named something like TouchStore() or
DirtyStore()?

[erikwright (departed), 2012/03/02 03:20:31]

Done.

   store_->SetClearLocalStateOnExit(false);
-  store_ = NULL;
-  // Make sure we wait until the destructor has run.
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  ASSERT_TRUE(helper->Run());
+  DestroyStore();
 
   ASSERT_TRUE(file_util::PathExists(
       temp_dir_.path().Append(chrome::kCookieFilename)));
   ASSERT_TRUE(file_util::Delete(
       temp_dir_.path().Append(chrome::kCookieFilename), false));
 }
 
 TEST_F(SQLitePersistentCookieStoreTest, RemoveOnDestruction) {
+  InitializeStore(false);
+  AddCookie("A", "B", "http://foo.bar", "/", base::Time::Now());
   store_->SetClearLocalStateOnExit(true);
-  // Replace the store effectively destroying the current one and forcing it
-  // to write it's data to disk. Then we can see if after loading it again it
-  // is still there.
-  store_ = NULL;
-  // Make sure we wait until the destructor has run.
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  ASSERT_TRUE(helper->Run());
+  DestroyStore();
 
   ASSERT_FALSE(file_util::PathExists(
       temp_dir_.path().Append(chrome::kCookieFilename)));
 }
 
 // Test if data is stored as expected in the SQLite database.
 TEST_F(SQLitePersistentCookieStoreTest, TestPersistance) {
-  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+  InitializeStore(false);
+  AddCookie("A", "B", "http://foo.bar", "/", base::Time::Now());
   // Replace the store effectively destroying the current one and forcing it
   // to write it's data to disk. Then we can see if after loading it again it
   // is still there.
-  store_ = NULL;
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  // Make sure we wait until the destructor has run.
-  ASSERT_TRUE(helper->Run());
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir_.path().Append(chrome::kCookieFilename), false);
-
+  DestroyStore();
   // Reload and test for persistence
-  Load(&cookies);
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+  CreateAndLoad(false, &cookies);
   ASSERT_EQ(1U, cookies.size());
   ASSERT_STREQ("http://foo.bar", cookies[0]->Domain().c_str());
   ASSERT_STREQ("A", cookies[0]->Name().c_str());
   ASSERT_STREQ("B", cookies[0]->Value().c_str());
 
   // Now delete the cookie and check persistence again.
   store_->DeleteCookie(*cookies[0]);
-  store_ = NULL;
-  // Make sure we wait until the destructor has run.
-  ASSERT_TRUE(helper->Run());
+  DestroyStore();
   STLDeleteContainerPointers(cookies.begin(), cookies.end());
   cookies.clear();
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir_.path().Append(chrome::kCookieFilename), false);
 
   // Reload and check if the cookie has been removed.
-  Load(&cookies);
+  CreateAndLoad(false, &cookies);
   ASSERT_EQ(0U, cookies.size());
 }
 
 // Test that priority load of cookies for a specfic domain key could be
 // completed before the entire store is loaded
 TEST_F(SQLitePersistentCookieStoreTest, TestLoadCookiesForKey) {
-  base::Time t = base::Time::Now() + base::TimeDelta::FromInternalValue(10);
-  // A foo.bar cookie was already added in setup.
-  store_->AddCookie(
-    net::CookieMonster::CanonicalCookie(GURL(), "A", "B",
-                                        "www.aaa.com", "/",
-                                        std::string(), std::string(),
-                                        t, t, t, false, false, true, true));
+  InitializeStore(false);
+  base::Time t = base::Time::Now();
+  AddCookie("A", "B", "http://foo.bar", "/", t);
   t += base::TimeDelta::FromInternalValue(10);
-  store_->AddCookie(
-    net::CookieMonster::CanonicalCookie(GURL(), "A", "B",
-                                        "travel.aaa.com", "/",
-                                        std::string(), std::string(),
-                                        t, t, t, false, false, true, true));
+  AddCookie("A", "B", "www.aaa.com", "/", t);
   t += base::TimeDelta::FromInternalValue(10);
-  store_->AddCookie(
-    net::CookieMonster::CanonicalCookie(GURL(), "A", "B",
-                                        "www.bbb.com", "/",
-                                        std::string(), std::string(),
-                                        t, t, t, false, false, true, true));
-  store_ = NULL;
-
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  // Make sure we wait until the destructor has run.
-  ASSERT_TRUE(helper->Run());
+  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);
   // 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,
       base::Bind(&SQLitePersistentCookieStoreTest::WaitOnDBEvent,
                  base::Unretained(this)));
   store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
@@ skipping 31 matching lines @@
        it = cookies_.begin(); it != cookies_.end(); ++it)
     cookies_loaded.insert((*it)->Domain().c_str());
   ASSERT_EQ(4U, cookies_loaded.size());
   ASSERT_EQ(cookies_loaded.find("http://foo.bar") != cookies_loaded.end(),
             true);
   ASSERT_EQ(cookies_loaded.find("www.bbb.com") != cookies_loaded.end(), true);
 }
 
 // Test that we can force the database to be written by calling Flush().
 TEST_F(SQLitePersistentCookieStoreTest, TestFlush) {
+  InitializeStore(false);
   // File timestamps don't work well on all platforms, so we'll determine
   // whether the DB file has been modified by checking its size.
   FilePath path = temp_dir_.path().Append(chrome::kCookieFilename);
   base::PlatformFileInfo info;
   ASSERT_TRUE(file_util::GetFileInfo(path, &info));
   int64 base_size = info.size;
 
   // 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);
-    store_->AddCookie(
-        net::CookieMonster::CanonicalCookie(GURL(), name, value,
-                                            "http://foo.bar", "/",
-                                            std::string(), std::string(),
-                                            t, t, t,
-                                            false, false, true, true));
+    AddCookie(name, value, "http://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());
 
   // We forced a write, so now the file will be bigger.
@@ skipping 14 matching lines @@
     return callback_count_;
   }
 
  private:
   friend class base::RefCountedThreadSafe<CallbackCounter>;
   volatile int callback_count_;
 };
 
 // Test that we can get a completion callback after a Flush().
 TEST_F(SQLitePersistentCookieStoreTest, TestFlushCompletionCallback) {
+  InitializeStore(false);
+  AddCookie("A", "B", "http://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) {
-  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
-
-  // Use a separate cookie store for this test.
-  ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir.path().Append(chrome::kCookieFilename), true);
-  // Make sure the database is initialized.
-  Load(&cookies);
-  ASSERT_EQ(0u, cookies.size());
+  InitializeStore(true);
 
   // Add a session cookie.
   store_->AddCookie(
       net::CookieMonster::CanonicalCookie(
           GURL(), "C", "D", "http://sessioncookie.com", "/", std::string(),
           std::string(), base::Time::Now(), base::Time::Now(),
           base::Time::Now(), false, false, true, false /*is_persistent*/));
 
   // Force the store to write its data to the disk.
-  store_ = NULL;
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  // Make sure we wait until the destructor has run.
-  ASSERT_TRUE(helper->Run());
+  DestroyStore();
 
-  // Create a store that loads session cookies.
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir.path().Append(chrome::kCookieFilename), true);
+  // Create a store that loads session cookies and test that the session cookie
+  // was loaded.
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+  CreateAndLoad(true, &cookies);
 
-  // Reload and test that the session cookie was loaded.
-  Load(&cookies);
   ASSERT_EQ(1U, cookies.size());
   ASSERT_STREQ("http://sessioncookie.com", cookies[0]->Domain().c_str());
   ASSERT_STREQ("C", cookies[0]->Name().c_str());
   ASSERT_STREQ("D", cookies[0]->Value().c_str());
+
+  STLDeleteContainerPointers(cookies.begin(), cookies.end());
+  cookies.clear();
 }
 
 // Test loading old session cookies from the disk.
 TEST_F(SQLitePersistentCookieStoreTest, TestDontLoadOldSessionCookies) {
-  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
-  // Use a separate cookie store for this test.
-  ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir.path().Append(chrome::kCookieFilename), true);
-  // Make sure the database is initialized.
-  Load(&cookies);
-  ASSERT_EQ(0u, cookies.size());
+  InitializeStore(true);
 
   // Add a session cookie.
   store_->AddCookie(
       net::CookieMonster::CanonicalCookie(
           GURL(), "C", "D", "http://sessioncookie.com", "/", std::string(),
           std::string(), base::Time::Now(), base::Time::Now(),
           base::Time::Now(), false, false, true, false /*is_persistent*/));
 
   // Force the store to write its data to the disk.
-  store_ = NULL;
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  // Make sure we wait until the destructor has run.
-  ASSERT_TRUE(helper->Run());
+  DestroyStore();
 
-  // Create a store that doesn't load old session cookies.
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir.path().Append(chrome::kCookieFilename), false);
-
-  // Reload and test that the session cookie was not loaded.
-  Load(&cookies);
+  // Create a store that doesn't load old session cookies and test that the
+  // session cookie was not loaded.
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+  CreateAndLoad(false, &cookies);
   ASSERT_EQ(0U, cookies.size());
 
   // The store should also delete the session cookie. Wait until that has been
   // done.
-  store_ = NULL;
-  helper = new base::ThreadTestHelper(
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB));
-  ASSERT_TRUE(helper->Run());
+  DestroyStore();
 
-  // Create a store that loads old session cookies.
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir.path().Append(chrome::kCookieFilename), true);
-
-  // Reload and test that the session cookie is gone.
-  Load(&cookies);
+  // Create a store that loads old session cookies and test that the session
+  // cookie is gone.
+  CreateAndLoad(true, &cookies);
   ASSERT_EQ(0U, cookies.size());
 }
 
 TEST_F(SQLitePersistentCookieStoreTest, PersistHasExpiresAndIsPersistent) {
-  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
-
-  // Use a separate cookie store for this test.
-  ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir.path().Append(chrome::kCookieFilename), true);
-  // Make sure the database is initialized.
-  Load(&cookies);
-  ASSERT_EQ(0u, cookies.size());
+  InitializeStore(true);
 
   // Add a session cookie with has_expires = false, and another session cookie
   // with has_expires = true.
   store_->AddCookie(
       net::CookieMonster::CanonicalCookie(
           GURL(), "session-hasexpires", "val", "http://sessioncookie.com", "/",
           std::string(), std::string(),
           base::Time::Now() - base::TimeDelta::FromDays(3), base::Time::Now(),
           base::Time::Now(), false, false, true /* has_expires */,
           false /* is_persistent */));
   store_->AddCookie(
       net::CookieMonster::CanonicalCookie(
           GURL(), "session-noexpires", "val", "http://sessioncookie.com", "/",
           std::string(), std::string(),
           base::Time::Now() - base::TimeDelta::FromDays(2), base::Time::Now(),
           base::Time::Now(), false, false, false /* has_expires */,
           false /* is_persistent */));
   // Add a persistent cookie.
   store_->AddCookie(
       net::CookieMonster::CanonicalCookie(
           GURL(), "persistent", "val", "http://sessioncookie.com", "/",
           std::string(), std::string(),
           base::Time::Now() - base::TimeDelta::FromDays(1), base::Time::Now(),
           base::Time::Now(), false, false, true /* has_expires */,
           true /* is_persistent */));
 
   // Force the store to write its data to the disk.
-  store_ = NULL;
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  // Make sure we wait until the destructor has run.
-  ASSERT_TRUE(helper->Run());
+  DestroyStore();
 
-  // Create a store that loads session cookies.
-  store_ = new SQLitePersistentCookieStore(
-      temp_dir.path().Append(chrome::kCookieFilename), true);
-
-  // Reload and test that the the DoesExpire and IsPersistent attributes are
-  // restored.
-  Load(&cookies);
+  // Create a store that loads session cookies and test that the the DoesExpire
+  // and IsPersistent attributes are restored.
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+  CreateAndLoad(true, &cookies);
   ASSERT_EQ(3U, cookies.size());
 
   std::map<std::string, net::CookieMonster::CanonicalCookie*> cookie_map;
   std::vector<net::CookieMonster::CanonicalCookie*>::const_iterator it;
   for (it = cookies.begin(); it != cookies.end(); ++it)
     cookie_map[(*it)->Name()] = *it;
 
   EXPECT_TRUE(cookie_map["session-hasexpires"]->DoesExpire());
   EXPECT_FALSE(cookie_map["session-hasexpires"]->IsPersistent());
 
   EXPECT_FALSE(cookie_map["session-noexpires"]->DoesExpire());
   EXPECT_FALSE(cookie_map["session-noexpires"]->IsPersistent());
 
   EXPECT_TRUE(cookie_map["persistent"]->DoesExpire());
   EXPECT_TRUE(cookie_map["persistent"]->IsPersistent());
+
+  STLDeleteContainerPointers(cookies.begin(), cookies.end());
+  cookies.clear();
 }