Refactored cookies persistent store clean-up on shutdown.

chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc

Index: chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc
diff --git a/chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc b/chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d8de902d9f87e11e875787692955a830d20d329c
--- /dev/null
+++ b/chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc
@@ -0,0 +1,128 @@
+// Copyright (c) 2010 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/file_util.h"
+#include "base/message_loop.h"
+#include "base/ref_counted.h"
+#include "base/scoped_temp_dir.h"
+#include "base/stl_util-inl.h"
+#include "base/time.h"
+#include "chrome/browser/browser_thread.h"
+#include "chrome/browser/net/sqlite_persistent_cookie_store.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/test/thread_test_helper.h"
+#include "googleurl/src/gurl.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class SQLitePersistentCookieStoreTest : public testing::Test {
+ public:
+  SQLitePersistentCookieStoreTest()
+      : message_loop_(MessageLoop::TYPE_UI),
+        ui_thread_(BrowserThread::UI, &message_loop_),
+        db_thread_(BrowserThread::DB) {}

[jochen (gone - plz use gerrit), 2010/12/02 16:48:44]

nit. closing bracket on new line

[pastarmovj, 2010/12/03 16:43:02]

Done.

+
+ protected:
+  virtual void SetUp() {
+    db_thread_.Start();

[jochen (gone - plz use gerrit), 2010/12/02 16:48:44]

I meant that you can start the db_thread just like the ui_thread by explicitly
passing it the message_loop_. it's fine to use the same loop for several threads
in unit tests

[pastarmovj, 2010/12/03 16:43:02]

Done.

+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    ASSERT_TRUE(store_ = new SQLitePersistentCookieStore(

[jochen (gone - plz use gerrit), 2010/12/02 16:48:44]

either make it ASSERT(store_) in the next line and check everywhere where you
create a store, or (better) just leave the ASSERT away

[pastarmovj, 2010/12/03 16:43:02]

Done.

+        temp_dir_.path().Append(chrome::kCookieFilename)));
+    ASSERT_TRUE(store_->Load(&cookies_));
+    ASSERT_TRUE(0 == cookies_.size());

[jochen (gone - plz use gerrit), 2010/12/02 16:48:44]

ASSERT_EQ

I think it's good ok to have the store in the SetUp method, but I still think
you should get rid of cookies_ and ClearCookies. It's only used in a single
test, and you only need to call ClearCookies once

[pastarmovj, 2010/12/03 16:43:02]

However I can't use the store if don't call the Load function which actually
creates the tables if they don't exist. And this call needs the cookies array. I
either have to duplicate it once for the start up function and once for the test
or just reuse it as a member variable as it is now.

[pastarmovj, 2010/12/03 16:46:40]

Sorry I posted an obsoleted comment here ignore it.

+  }
+
+  void ClearCookies() {
+    STLDeleteContainerPointers(cookies_.begin(), cookies_.end());
+    cookies_.clear();
+  }
+
+  MessageLoop message_loop_;
+

[jochen (gone - plz use gerrit), 2010/12/02 16:48:44]

no newline

[pastarmovj, 2010/12/03 16:43:02]

Done.

+  BrowserThread ui_thread_;
+  BrowserThread db_thread_;
+  scoped_refptr<SQLitePersistentCookieStore> store_;
+  ScopedTempDir temp_dir_;
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies_;
+};
+
+TEST_F(SQLitePersistentCookieStoreTest, KeepOnDestruction) {
+  // Make sure the store gets written at least once.
+  store_->AddCookie(
+      net::CookieMonster::CanonicalCookie("A", "B", "http://foo.bar", "/",
+                                          false, false,
+                                          base::Time(), base::Time(),
+                                          true, base::Time()));
+  store_->SetClearLocalStateOnExit(false);
+  // Replace the store effectively destroying the current one and forcing it
+  // to call it's destructor. We can't just NULL it so that our TearDown code
+  // won't crash.
+  store_ = new SQLitePersistentCookieStore(

[jochen (gone - plz use gerrit), 2010/12/02 16:48:44]

store_ = NULL (so it's symmetrical to the test below)

[pastarmovj, 2010/12/03 16:43:02]

Done.

+      temp_dir_.path().Append(chrome::kCookieFilename));
+  // Make sure we wait until the destructor has run.
+  scoped_refptr<ThreadTestHelper> helper(
+      new ThreadTestHelper(BrowserThread::DB));
+  ASSERT_TRUE(helper->Run());
+
+  ASSERT_TRUE(file_util::PathExists(
+      temp_dir_.path().Append(chrome::kCookieFilename)));
+}
+
+TEST_F(SQLitePersistentCookieStoreTest, RemoveOnDestruction) {
+  // Make sure the store gets written at least once.
+  store_->AddCookie(
+      net::CookieMonster::CanonicalCookie("A", "B", "http://foo.bar", "/",
+                                          false, false,
+                                          base::Time(), base::Time(),
+                                          true, base::Time()));
+  store_->SetClearLocalStateOnExit(true);
+  store_ = NULL;
+  // Make sure we wait until the destructor has run.
+  scoped_refptr<ThreadTestHelper> helper(
+      new ThreadTestHelper(BrowserThread::DB));
+  ASSERT_TRUE(helper->Run());
+
+  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) {
+  // Make sure the store gets written at least once.
+  store_->AddCookie(
+      net::CookieMonster::CanonicalCookie("A", "B", "http://foo.bar", "/",
+                                          false, false,
+                                          base::Time::Now(), base::Time::Now(),
+                                          true, 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<ThreadTestHelper> helper(
+      new ThreadTestHelper(BrowserThread::DB));
+  // Make sure we wait until the destructor has run.
+  ASSERT_TRUE(helper->Run());
+  ClearCookies();

[jochen (gone - plz use gerrit), 2010/12/02 16:48:44]

cookies_ should be empty here. no need to clear

[pastarmovj, 2010/12/03 16:43:02]

Done.

+  store_ = new SQLitePersistentCookieStore(
+      temp_dir_.path().Append(chrome::kCookieFilename));
+
+  // Reload and test for persistence
+  ASSERT_TRUE(store_->Load(&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());
+  ClearCookies();
+  store_ = new SQLitePersistentCookieStore(
+      temp_dir_.path().Append(chrome::kCookieFilename));
+
+  // Reload and check if the cookie has been removed.
+  ASSERT_TRUE(store_->Load(&cookies_));
+  ASSERT_EQ(0U, cookies_.size());
+}