Delete session cookies immediately after loading the cookie DB. (attempt #2)

The original CL had non-determinism in the unit test.

> Delete session cookies immediately after loading the cookie DB.
>
> Previously, the session cookies were deleted after all cookies were loaded into
> memory. This allows for a race condition where new session cookies are inserted
> into the database while the cookies are being loaded into memory, which causes
> the new session cookies to also be deleted.
>
> BUG=
>
> Committed: https://crrev.com/ca924dcfb4f3c6b89e0c40898e42aec349b6b843
> Cr-Commit-Position: refs/heads/master@{#326672}

BUG=

Review URL: https://codereview.chromium.org/1103003002

Cr-Commit-Position: refs/heads/master@{#327104}

content/browser/net/sqlite_persistent_cookie_store.cc

@@ -701,6 +701,9 @@ bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
       50);
 
   initialized_ = true;
+
+  if (!restore_old_session_cookies_)
+    DeleteSessionCookiesOnStartup();
   return true;
 }
 
@@ -1296,7 +1299,7 @@ void SQLitePersistentCookieStore::Backend::SetForceKeepSessionState() {
 
 void SQLitePersistentCookieStore::Backend::DeleteSessionCookiesOnStartup() {
   DCHECK(background_task_runner_->RunsTasksOnCurrentThread());
-  if (!db_->Execute("DELETE FROM cookies WHERE persistent == 0"))
+  if (!db_->Execute("DELETE FROM cookies WHERE persistent != 1"))
     LOG(WARNING) << "Unable to delete session cookies.";
 }
 
@@ -1321,8 +1324,6 @@ void SQLitePersistentCookieStore::Backend::FinishedLoadingCookies(
     bool success) {
   PostClientTask(FROM_HERE, base::Bind(&Backend::CompleteLoadInForeground, this,
                                        loaded_callback, success));
-  if (success && !restore_old_session_cookies_)
-    DeleteSessionCookiesOnStartup();
 }
 
 SQLitePersistentCookieStore::SQLitePersistentCookieStore(

content/browser/net/sqlite_persistent_cookie_store_unittest.cc

@@ -164,6 +164,17 @@ class SQLitePersistentCookieStoreTest : public testing::Test {
         false, false, net::COOKIE_PRIORITY_DEFAULT));
   }
 
+  void AddCookieWithExpiration(const std::string& name,
+                               const std::string& value,
+                               const std::string& domain,
+                               const std::string& path,
+                               const base::Time& creation,
+                               const base::Time& expiration) {
+    store_->AddCookie(net::CanonicalCookie(
+        GURL(), name, value, domain, path, creation, expiration, creation,
+        false, false, false, net::COOKIE_PRIORITY_DEFAULT));
+  }
+
   std::string ReadRawDBContents() {
     std::string contents;
     if (!base::ReadFileToString(temp_dir_.path().Append(kCookieFilename),
@@ -257,6 +268,82 @@ TEST_F(SQLitePersistentCookieStoreTest, TestPersistance) {
   ASSERT_EQ(0U, cookies.size());
 }
 
+TEST_F(SQLitePersistentCookieStoreTest, TestSessionCookiesDeletedOnStartup) {
+  // Initialize the cookie store with 3 persistent cookies, 5 transient
+  // cookies.
+  InitializeStore(false, false);
+
+  // Add persistent cookies.
+  base::Time t = base::Time::Now();
+  AddCookie("A", "B", "a1.com", "/", t);
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookie("A", "B", "a2.com", "/", t);
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookie("A", "B", "a3.com", "/", t);
+
+  // Add transient cookies.
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookieWithExpiration("A", "B", "b1.com", "/", t, base::Time());
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookieWithExpiration("A", "B", "b2.com", "/", t, base::Time());
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookieWithExpiration("A", "B", "b3.com", "/", t, base::Time());
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookieWithExpiration("A", "B", "b4.com", "/", t, base::Time());
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookieWithExpiration("A", "B", "b5.com", "/", t, base::Time());
+  DestroyStore();
+
+  // Load the store a second time. Before the store finishes loading, add a
+  // transient cookie and flush it to disk.
+  store_ = new SQLitePersistentCookieStore(
+      temp_dir_.path().Append(kCookieFilename),
+      client_task_runner(),
+      background_task_runner(),
+      false, NULL, NULL);
+
+  // Posting a blocking task to db_thread_ makes sure that the DB thread waits
+  // until both Load and Flush have been posted to its task queue.
+  background_task_runner()->PostTask(
+      FROM_HERE,
+      base::Bind(&SQLitePersistentCookieStoreTest::WaitOnDBEvent,
+                 base::Unretained(this)));
+  store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
+                          base::Unretained(this)));
+  t += base::TimeDelta::FromInternalValue(10);
+  AddCookieWithExpiration("A", "B", "c.com", "/", t, base::Time());
+  base::WaitableEvent event(false, false);
+  store_->Flush(base::Bind(&base::WaitableEvent::Signal,
+                           base::Unretained(&event)));
+
+  // Now the DB-thread queue contains:
+  // (active:)
+  // 1. Wait (on db_event)
+  // (pending:)
+  // 2. "Init And Chain-Load First Domain"
+  // 3. Add Cookie (c.com)
+  // 4. Flush Cookie (c.com)
+  db_thread_event_.Signal();
+  event.Wait();
+  loaded_event_.Wait();
+  STLDeleteElements(&cookies_);
+  DestroyStore();
+
+  // Load the store a third time, this time restoring session cookies. The
+  // store should contain exactly 4 cookies: the 3 persistent, and "c.com",
+  // which was added during the second cookie store load.
+  store_ = new SQLitePersistentCookieStore(
+      temp_dir_.path().Append(kCookieFilename),
+      client_task_runner(),
+      background_task_runner(),
+      true, NULL, NULL);
+  store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
+                          base::Unretained(this)));
+  loaded_event_.Wait();
+  ASSERT_EQ(4u, cookies_.size());
+  STLDeleteElements(&cookies_);
+}
+
 // Test that priority load of cookies for a specfic domain key could be
 // completed before the entire store is loaded
 TEST_F(SQLitePersistentCookieStoreTest, TestLoadCookiesForKey) {