Finalize a CL originally by departed intern ycxiao@ that detaches the loading of cookies from the IO thread.

They are now loaded on the DB thread. Cookie operations received in the meantime are queued and executed, on the IO thread, in the order they were received, when loading completes.

A few straggler clients are updated to use the asynchronous CookieStore/CookieMonster API as part of this CL, as the synchronous API is removed.

BUG=68657
TEST=net_unittests / DeferredCookieTaskTest.* and CookieMonsterTest.*

Review URL: http://codereview.chromium.org/7833042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@100188 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 148 insertions, 98 deletions

diff --git a/chrome/browser/automation/automation_util.cc b/chrome/browser/automation/automation_util.cc
index 189c90b..132ad14 100644
--- a/chrome/browser/automation/automation_util.cc
+++ b/chrome/browser/automation/automation_util.cc
@@ -42,7 +42,7 @@ void GetCookiesOnIOThread(
     base::WaitableEvent* event,
     std::string* cookies) {
   context_getter->GetURLRequestContext()->cookie_store()->
-      GetCookiesAsync(url,
+      GetCookiesWithOptionsAsync(url, net::CookieOptions(),
                       base::Bind(&GetCookiesCallback, event, cookies));
 }
 
diff --git a/chrome/browser/browsing_data_remover.cc b/chrome/browser/browsing_data_remover.cc
index b759d1d..99e4581 100644
--- a/chrome/browser/browsing_data_remover.cc
+++ b/chrome/browser/browsing_data_remover.cc
@@ -577,7 +577,7 @@ void BrowsingDataRemover::ClearCookiesOnIOThread(
       GetURLRequestContext()->cookie_store()->GetCookieMonster();
   if (cookie_monster) {
       cookie_monster->DeleteAllCreatedBetweenAsync(
-          delete_begin_, delete_end_, true,
+          delete_begin_, delete_end_,
           base::Bind(&BrowsingDataRemover::OnClearedCookies,
                      base::Unretained(this)));
   } else {
diff --git a/chrome/browser/fast_shutdown_uitest.cc b/chrome/browser/fast_shutdown_uitest.cc
index 2985d46..e4a4643 100644
--- a/chrome/browser/fast_shutdown_uitest.cc
+++ b/chrome/browser/fast_shutdown_uitest.cc
@@ -2,6 +2,7 @@
 // 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/file_path.h"
 #include "base/stl_util.h"
 #include "base/test/thread_test_helper.h"
@@ -18,6 +19,7 @@ class FastShutdown : public UITest {
  protected:
   FastShutdown()
       : db_thread_(BrowserThread::DB),
+        io_thread_(BrowserThread::IO),
         thread_helper_(new base::ThreadTestHelper(
             BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB))) {
     dom_automation_enabled_ = true;
@@ -25,7 +27,7 @@ class FastShutdown : public UITest {
 
   void Init() {
     ASSERT_TRUE(db_thread_.Start());
-
+    ASSERT_TRUE(io_thread_.Start());
     // Cache this, so that we can still access it after the browser exits.
     user_data_dir_ = user_data_dir();
   }
@@ -36,11 +38,17 @@ class FastShutdown : public UITest {
   void GetCookie(const net::CookieMonster::CanonicalCookie& cookie,
                  bool* has_cookie, std::string* cookie_value) {
     scoped_refptr<SQLitePersistentCookieStore> cookie_store(
-        new SQLitePersistentCookieStore(
-            user_data_dir_.AppendASCII(chrome::kInitialProfile)
-                          .Append(chrome::kCookieFilename)));
+       new SQLitePersistentCookieStore(
+           user_data_dir_.AppendASCII(chrome::kInitialProfile)
+                         .Append(chrome::kCookieFilename)));
     std::vector<net::CookieMonster::CanonicalCookie*> cookies;
-    ASSERT_TRUE(cookie_store->Load(&cookies));
+    ASSERT_TRUE(cookie_store->Load(
+        base::Bind(&FastShutdown::LoadCookiesCallback,
+                   base::Unretained(this),
+                   MessageLoop::current(),
+                   base::Unretained(&cookies))));
+    // Will receive a QuitTask when LoadCookiesCallback is invoked.
+    MessageLoop::current()->Run();
     *has_cookie = false;
     for (size_t i = 0; i < cookies.size(); ++i) {
       if (cookies[i]->IsEquivalent(cookie)) {
@@ -54,7 +62,16 @@ class FastShutdown : public UITest {
   }
 
  private:
-  BrowserThread db_thread_;  // Used by the cookie store during its clean up.
+  void LoadCookiesCallback(
+      MessageLoop* to_notify,
+      std::vector<net::CookieMonster::CanonicalCookie*>* cookies,
+      const std::vector<net::CookieMonster::CanonicalCookie*>& cookies_get) {
+    *cookies = cookies_get;
+    to_notify->PostTask(FROM_HERE, new MessageLoop::QuitTask());
+  }
+
+  BrowserThread db_thread_;
+  BrowserThread io_thread_;
   scoped_refptr<base::ThreadTestHelper> thread_helper_;
   FilePath user_data_dir_;
 
@@ -83,14 +100,6 @@ TEST_F(FastShutdown, SlowTermination) {
       false,         // httponly
       false);        // has_expires
 
-  bool has_cookie = false;
-  std::string cookie_value;
-
-  // Check that the cookie (to be set during unload) doesn't exist already.
-  GetCookie(cookie, &has_cookie, &cookie_value);
-  EXPECT_FALSE(has_cookie);
-  EXPECT_EQ("", cookie_value);
-
   // This page has an unload handler.
   const FilePath dir(FILE_PATH_LITERAL("fast_shutdown"));
   const FilePath file(FILE_PATH_LITERAL("on_unloader.html"));
@@ -111,6 +120,8 @@ TEST_F(FastShutdown, SlowTermination) {
   // cookie that's stored to disk.
   QuitBrowser();
 
+  bool has_cookie = false;
+  std::string cookie_value;
   // Read the cookie and check that it has the expected value.
   GetCookie(cookie, &has_cookie, &cookie_value);
   EXPECT_TRUE(has_cookie);
diff --git a/chrome/browser/net/cookie_policy_browsertest.cc b/chrome/browser/net/cookie_policy_browsertest.cc
index 251ae69..b242c5c 100644
--- a/chrome/browser/net/cookie_policy_browsertest.cc
+++ b/chrome/browser/net/cookie_policy_browsertest.cc
@@ -2,6 +2,7 @@
 // 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/task.h"
 #include "base/synchronization/waitable_event.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
@@ -29,10 +30,19 @@ class GetCookiesTask : public Task {
         cookies_(cookies) {}
 
   virtual void Run() {
-    *cookies_ =
-        context_getter_->GetURLRequestContext()->cookie_store()->
-        GetCookies(url_);
-    event_->Signal();
+    net::CookieOptions options;
+    context_getter_->GetURLRequestContext()->cookie_store()
+        ->GetCookiesWithOptionsAsync(
+            url_, options, base::Bind(&GetCookiesTask::GetCookiesCallback,
+                                      base::Unretained(cookies_),
+                                      base::Unretained(event_)));
+  }
+
+  static void GetCookiesCallback(std::string* cookies_out,
+                                 base::WaitableEvent* event,
+                                 const std::string& cookies) {
+    *cookies_out = cookies;
+    event->Signal();
   }
 
  private:
diff --git a/chrome/browser/net/sqlite_persistent_cookie_store.cc b/chrome/browser/net/sqlite_persistent_cookie_store.cc
index 7e8b238..b2adff1 100644
--- a/chrome/browser/net/sqlite_persistent_cookie_store.cc
+++ b/chrome/browser/net/sqlite_persistent_cookie_store.cc
@@ -7,6 +7,7 @@
 #include <list>
 
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ -27,6 +28,12 @@ using base::Time;
 
 // This class is designed to be shared between any calling threads and the
 // database thread.  It batches operations and commits them on a timer.
+// This class expects to be Load()'ed once on any thread. Loading occurs
+// asynchronously on the DB thread and the caller will be notified on the IO
+// thread. Subsequent to loading, mutations may be queued by any thread using
+// AddCookie, UpdateCookieAccessTime, and DeleteCookie. These are flushed to
+// disk on the DB thread every 30 seconds, 512 operations, or call to Flush(),
+// whichever occurs first.
 class SQLitePersistentCookieStore::Backend
     : public base::RefCountedThreadSafe<SQLitePersistentCookieStore::Backend> {
  public:
@@ -38,7 +45,7 @@ class SQLitePersistentCookieStore::Backend
   }
 
   // Creates or load the SQLite database.
-  bool Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies);
+  bool Load(const LoadedCallback& loaded_callback);
 
   // Batch a cookie addition.
   void AddCookie(const net::CookieMonster::CanonicalCookie& cc);
@@ -91,6 +98,18 @@ class SQLitePersistentCookieStore::Backend
   };
 
  private:
+  // Creates or load the SQLite database on DB thread.
+  void LoadAndNotifyOnDBThread(const LoadedCallback& loaded_callback);
+  // Notify the CookieMonster when loading complete.
+  void NotifyOnIOThread(
+      const LoadedCallback& loaded_callback,
+      bool load_success,
+      const std::vector<net::CookieMonster::CanonicalCookie*>& cookies);
+  // Initialize the data base.
+  bool InitializeDatabase();
+  // Load cookies to the data base, and read cookies.
+  bool LoadInternal(std::vector<net::CookieMonster::CanonicalCookie*>* cookies);
+
   // Batch a cookie operation (add or delete)
   void BatchOperation(PendingOperation::OperationType op,
                       const net::CookieMonster::CanonicalCookie& cc);
@@ -154,19 +173,40 @@ bool InitTable(sql::Connection* db) {
 }  // namespace
 
 bool SQLitePersistentCookieStore::Backend::Load(
-    std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
+    const LoadedCallback& loaded_callback) {
   // This function should be called only once per instance.
   DCHECK(!db_.get());
+  BrowserThread::PostTask(
+      BrowserThread::DB, FROM_HERE,
+      base::Bind(&Backend::LoadAndNotifyOnDBThread, base::Unretained(this),
+                 loaded_callback));
+  return true;
+}
 
-  // Ensure the parent directory for storing cookies is created before reading
-  // from it.  We make an exception to allow IO on the UI thread here because
-  // we are going to disk anyway in db_->Open.  (This code will be moved to the
-  // DB thread as part of http://crbug.com/52909.)
-  {
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    const FilePath dir = path_.DirName();
-    if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir))
-      return false;
+void SQLitePersistentCookieStore::Backend::LoadAndNotifyOnDBThread(
+    const LoadedCallback& loaded_callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+
+  bool load_success = LoadInternal(&cookies);
+
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
+      &SQLitePersistentCookieStore::Backend::NotifyOnIOThread,
+      base::Unretained(this), loaded_callback, load_success, cookies));
+}
+
+void SQLitePersistentCookieStore::Backend::NotifyOnIOThread(
+    const LoadedCallback& loaded_callback,
+    bool load_success,
+    const std::vector<net::CookieMonster::CanonicalCookie*>& cookies) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  loaded_callback.Run(cookies);
+}
+
+bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
+  const FilePath dir = path_.DirName();
+  if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir)) {
+    return false;
   }
 
   db_.reset(new sql::Connection);
@@ -185,6 +225,14 @@ bool SQLitePersistentCookieStore::Backend::Load(
   }
 
   db_->Preload();
+  return true;
+}
+
+bool SQLitePersistentCookieStore::Backend::LoadInternal(
+    std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
+  if (!InitializeDatabase()) {
+    return false;
+  }
 
   // Slurp all the cookies into the out-vector.
   sql::Statement smt(db_->GetUniqueStatement(
@@ -447,11 +495,14 @@ void SQLitePersistentCookieStore::Backend::Flush(Task* completion_task) {
 // pending commit timer that will be holding a reference on us, but if/when
 // this fires we will already have been cleaned up and it will be ignored.
 void SQLitePersistentCookieStore::Backend::Close() {
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::DB));
-  // Must close the backend on the background thread.
-  BrowserThread::PostTask(
-      BrowserThread::DB, FROM_HERE,
-      NewRunnableMethod(this, &Backend::InternalBackgroundClose));
+  if (BrowserThread::CurrentlyOn(BrowserThread::DB)) {
+    InternalBackgroundClose();
+  } else {
+    // Must close the backend on the background thread.
+    BrowserThread::PostTask(
+        BrowserThread::DB, FROM_HERE,
+        NewRunnableMethod(this, &Backend::InternalBackgroundClose));
+  }
 }
 
 void SQLitePersistentCookieStore::Backend::InternalBackgroundClose() {
@@ -483,9 +534,8 @@ SQLitePersistentCookieStore::~SQLitePersistentCookieStore() {
   }
 }
 
-bool SQLitePersistentCookieStore::Load(
-    std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
-  return backend_->Load(cookies);
+bool SQLitePersistentCookieStore::Load(const LoadedCallback& loaded_callback) {
+  return backend_->Load(loaded_callback);
 }
 
 void SQLitePersistentCookieStore::AddCookie(
diff --git a/chrome/browser/net/sqlite_persistent_cookie_store.h b/chrome/browser/net/sqlite_persistent_cookie_store.h
index f589f25..12a7de5 100644
--- a/chrome/browser/net/sqlite_persistent_cookie_store.h
+++ b/chrome/browser/net/sqlite_persistent_cookie_store.h
@@ -25,16 +25,20 @@ class SQLitePersistentCookieStore
   explicit SQLitePersistentCookieStore(const FilePath& path);
   virtual ~SQLitePersistentCookieStore();
 
-  virtual bool Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies);
+  virtual bool Load(const LoadedCallback& loaded_callback) OVERRIDE;
+
+  virtual void AddCookie(
+      const net::CookieMonster::CanonicalCookie& cc) OVERRIDE;
 
-  virtual void AddCookie(const net::CookieMonster::CanonicalCookie& cc);
   virtual void UpdateCookieAccessTime(
-      const net::CookieMonster::CanonicalCookie& cc);
-  virtual void DeleteCookie(const net::CookieMonster::CanonicalCookie& cc);
+      const net::CookieMonster::CanonicalCookie& cc) OVERRIDE;
+
+  virtual void DeleteCookie(
+      const net::CookieMonster::CanonicalCookie& cc) OVERRIDE;
 
-  virtual void SetClearLocalStateOnExit(bool clear_local_state);
+  virtual void SetClearLocalStateOnExit(bool clear_local_state) OVERRIDE;
 
-  virtual void Flush(Task* completion_task);
+  virtual void Flush(Task* completion_task) OVERRIDE;
 
  private:
   class Backend;
diff --git a/chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc b/chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc
index f3e52cc..942ca83 100644
--- a/chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc
+++ b/chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc
@@ -2,11 +2,13 @@
 // 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/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
 #include "base/scoped_temp_dir.h"
 #include "base/stl_util.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/test/thread_test_helper.h"
 #include "base/time.h"
 #include "chrome/browser/net/sqlite_persistent_cookie_store.h"
@@ -19,19 +21,37 @@ class SQLitePersistentCookieStoreTest : public testing::Test {
  public:
   SQLitePersistentCookieStoreTest()
       : ui_thread_(BrowserThread::UI),
-        db_thread_(BrowserThread::DB) {
+        db_thread_(BrowserThread::DB),
+        io_thread_(BrowserThread::IO),
+        event_(false, false) {
   }
 
  protected:
+  void OnLoaded(
+      const std::vector<net::CookieMonster::CanonicalCookie*>& cookies) {
+    cookies_ = cookies;
+    event_.Signal();
+  }
+
+  bool Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
+    bool result =
+        store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
+                                base::Unretained(this)));
+    event_.Wait();
+    *cookies = cookies_;
+    return result;
+  }
+
   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));
     std::vector<net::CookieMonster::CanonicalCookie*> cookies;
-    ASSERT_TRUE(store_->Load(&cookies));
-    ASSERT_TRUE(0 == cookies.size());
+    ASSERT_TRUE(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",
@@ -44,6 +64,9 @@ class SQLitePersistentCookieStoreTest : public testing::Test {
 
   BrowserThread ui_thread_;
   BrowserThread db_thread_;
+  BrowserThread io_thread_;
+  base::WaitableEvent event_;
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies_;
   ScopedTempDir temp_dir_;
   scoped_refptr<SQLitePersistentCookieStore> store_;
 };
@@ -95,7 +118,7 @@ TEST_F(SQLitePersistentCookieStoreTest, TestPersistance) {
       temp_dir_.path().Append(chrome::kCookieFilename));
 
   // Reload and test for persistence
-  ASSERT_TRUE(store_->Load(&cookies));
+  ASSERT_TRUE(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());
@@ -112,7 +135,7 @@ TEST_F(SQLitePersistentCookieStoreTest, TestPersistance) {
       temp_dir_.path().Append(chrome::kCookieFilename));
 
   // Reload and check if the cookie has been removed.
-  ASSERT_TRUE(store_->Load(&cookies));
+  ASSERT_TRUE(Load(&cookies));
   ASSERT_EQ(0U, cookies.size());
 }
 
diff --git a/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.cc b/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.cc
index 33eea81..0b8f48d 100644
--- a/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.cc
+++ b/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.cc
@@ -26,52 +26,6 @@
 // configurable.
 static const char kSyncDefaultViewOnlineUrl[] = "http://docs.google.com";
 
-// TODO(idana): the following code was originally copied from
-// toolbar_importer.h/cc and it needs to be moved to a common Google Accounts
-// utility.
-
-// A simple pair of fields that identify a set of Google cookies, used to
-// filter from a larger set.
-struct GoogleCookieFilter {
-  // The generalized, fully qualified URL of pages where
-  // cookies with id |cookie_id| are obtained / accessed.
-  const char* url;
-  // The id of the cookie this filter is selecting,
-  // with name/value delimiter (i.e '=').
-  const char* cookie_id;
-};
-
-// Filters to select Google GAIA cookies.
-static const GoogleCookieFilter kGAIACookieFilters[] = {
-  { "http://.google.com/",       "SID=" },     // Gmail.
-  // Add filters here for other interesting cookies that should result in
-  // showing the promotions (e.g ASIDAS for dasher accounts).
-};
-
-bool IsGoogleGAIACookieInstalled() {
-  for (size_t i = 0; i < arraysize(kGAIACookieFilters); ++i) {
-    // Since we are running on the UI thread don't call GetURLRequestContext().
-    net::CookieStore* store =
-        Profile::Deprecated::GetDefaultRequestContext()->
-        DONTUSEME_GetCookieStore();
-    GURL url(kGAIACookieFilters[i].url);
-    net::CookieOptions options;
-    options.set_include_httponly();  // The SID cookie might be httponly.
-    std::string cookies = store->GetCookiesWithOptions(url, options);
-    std::vector<std::string> cookie_list;
-    base::SplitString(cookies, ';', &cookie_list);
-    for (std::vector<std::string>::iterator current = cookie_list.begin();
-         current != cookie_list.end();
-         ++current) {
-      size_t position =
-          current->find(kGAIACookieFilters[i].cookie_id);
-      if (0 == position)
-        return true;
-    }
-  }
-  return false;
-}
-
 NewTabPageSyncHandler::NewTabPageSyncHandler() : sync_service_(NULL),
   waiting_for_initial_page_load_(true) {
 }
diff --git a/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.h b/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.h
index 9ebb02f..fcd9f7e 100644
--- a/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.h
+++ b/chrome/browser/ui/webui/ntp/new_tab_page_sync_handler.h
@@ -68,6 +68,4 @@ class NewTabPageSyncHandler : public WebUIMessageHandler,
   DISALLOW_COPY_AND_ASSIGN(NewTabPageSyncHandler);
 };
 
-bool IsGoogleGAIACookieInstalled();
-
 #endif  // CHROME_BROWSER_UI_WEBUI_NTP_NEW_TAB_PAGE_SYNC_HANDLER_H_