Update BrowsingDataRemover with the asynchronous CookieMonster API.

BUG=XXXX
TEST=XXXX

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

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

chrome/browser/browsing_data_remover.cc

@@ -7,6 +7,7 @@
 #include <map>
 #include <set>
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ -71,6 +72,7 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_history_(false),
       waiting_for_clear_quota_managed_data_(false),
       waiting_for_clear_networking_history_(false),
+      waiting_for_clear_cookies_(false),
       waiting_for_clear_cache_(false),
       waiting_for_clear_lso_data_(false) {
   DCHECK(profile);
@@ -93,6 +95,7 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_history_(false),
       waiting_for_clear_quota_managed_data_(false),
       waiting_for_clear_networking_history_(false),
+      waiting_for_clear_cookies_(false),
       waiting_for_clear_cache_(false),
       waiting_for_clear_lso_data_(false) {
   DCHECK(profile);
@@ -104,6 +107,7 @@ BrowsingDataRemover::~BrowsingDataRemover() {
 
 void BrowsingDataRemover::Remove(int remove_mask) {
   DCHECK(!removing_);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   removing_ = true;
 
   if (remove_mask & REMOVE_HISTORY) {
@@ -179,14 +183,14 @@ void BrowsingDataRemover::Remove(int remove_mask) {
   if (remove_mask & REMOVE_COOKIES) {
     UserMetrics::RecordAction(UserMetricsAction("ClearBrowsingData_Cookies"));
     // Since we are running on the UI thread don't call GetURLRequestContext().
-    net::CookieMonster* cookie_monster = NULL;
     net::URLRequestContextGetter* rq_context = profile_->GetRequestContext();
     if (rq_context) {
-      cookie_monster = rq_context->DONTUSEME_GetCookieStore()->
-          GetCookieMonster();
+      waiting_for_clear_cookies_ = true;
+      BrowserThread::PostTask(
+          BrowserThread::IO, FROM_HERE,
+          base::Bind(&BrowsingDataRemover::ClearCookiesOnIOThread,
+                     base::Unretained(this), base::Unretained(rq_context)));
     }
-    if (cookie_monster)
-      cookie_monster->DeleteAllCreatedBetween(delete_begin_, delete_end_, true);
 
     // REMOVE_COOKIES is actually "cookies and other site data" so we make sure
     // to remove other data such local databases, STS state, etc. These only can
@@ -532,3 +536,31 @@ void BrowsingDataRemover::OnWaitableEventSignaled(
   waiting_for_clear_lso_data_ = false;
   NotifyAndDeleteIfDone();
 }
+
+void BrowsingDataRemover::OnClearedCookies(int num_deleted) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&BrowsingDataRemover::OnClearedCookies,
+                   base::Unretained(this), num_deleted));
+    return;
+  }
+
+  waiting_for_clear_cookies_ = false;
+  NotifyAndDeleteIfDone();
+}
+
+void BrowsingDataRemover::ClearCookiesOnIOThread(
+    net::URLRequestContextGetter* rq_context) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  net::CookieMonster* cookie_monster = rq_context->
+      GetURLRequestContext()->cookie_store()->GetCookieMonster();
+  if (cookie_monster) {
+      cookie_monster->DeleteAllCreatedBetweenAsync(
+          delete_begin_, delete_end_, true,
+          base::Bind(&BrowsingDataRemover::OnClearedCookies,
+                     base::Unretained(this)));
+  } else {
+    OnClearedCookies(0);
+  }
+}

chrome/browser/browsing_data_remover.h

@@ -163,12 +163,19 @@ class BrowsingDataRemover : public NotificationObserver,
   // NotifyAndDeleteIfDone on the UI thread.
   void CheckQuotaManagedDataDeletionStatus();
 
+  // Callback when Cookies has been deleted. Invokes NotifyAndDeleteIfDone.
+  void OnClearedCookies(int num_deleted);
+
+  // Invoked on the IO thread to delete cookies.
+  void ClearCookiesOnIOThread(net::URLRequestContextGetter* rq_context);
+
   // Calculate the begin time for the deletion range specified by |time_period|.
   base::Time CalculateBeginDeleteTime(TimePeriod time_period);
 
   // Returns true if we're all done.
   bool all_done() {
     return registrar_.IsEmpty() && !waiting_for_clear_cache_ &&
+           !waiting_for_clear_cookies_&&
            !waiting_for_clear_history_ &&
            !waiting_for_clear_quota_managed_data_ &&
            !waiting_for_clear_networking_history_ &&
@@ -213,6 +220,7 @@ class BrowsingDataRemover : public NotificationObserver,
   bool waiting_for_clear_history_;
   bool waiting_for_clear_quota_managed_data_;
   bool waiting_for_clear_networking_history_;
+  bool waiting_for_clear_cookies_;
   bool waiting_for_clear_cache_;
   bool waiting_for_clear_lso_data_;
 

chrome/browser/browsing_data_remover_unittest.cc

@@ -6,12 +6,16 @@
 
 #include <set>
 
+#include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/platform_file.h"
 #include "chrome/browser/extensions/mock_extension_special_storage_policy.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/testing_browser_process_test.h"
+#include "net/base/cookie_monster.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.cc"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_file_util.h"
@@ -34,11 +38,20 @@ const GURL kOrigin3(kTestkOrigin3);
 
 class BrowsingDataRemoverTester : public BrowsingDataRemover::Observer {
  public:
-  BrowsingDataRemoverTester() {}
+  BrowsingDataRemoverTester()
+      : start_(false),
+        already_quit_(false) {}
   virtual ~BrowsingDataRemoverTester() {}
 
   void BlockUntilNotified() {
-    MessageLoop::current()->Run();
+    if (!already_quit_) {
+      DCHECK(!start_);
+      start_ = true;
+      MessageLoop::current()->Run();
+    } else {
+      DCHECK(!start_);
+      already_quit_ = false;
+    }
   }
 
  protected:
@@ -48,15 +61,78 @@ class BrowsingDataRemoverTester : public BrowsingDataRemover::Observer {
   }
 
   void Notify() {
-    MessageLoop::current()->Quit();
+    if (start_) {
+      DCHECK(!already_quit_);
+      MessageLoop::current()->Quit();
+      start_ = false;
+    } else {
+      DCHECK(!already_quit_);
+      already_quit_ = true;
+    }
   }
 
  private:
+  // Helps prevent from running message_loop, if the callback invoked
+  // immediately.
+  bool start_;
+  bool already_quit_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowsingDataRemoverTester);
 };
 
 // Testers -------------------------------------------------------------------
 
+class RemoveCookieTester : public BrowsingDataRemoverTester {
+ public:
+  explicit RemoveCookieTester(TestingProfile* profile)
+      : get_cookie_success_(false) {
+    profile->CreateRequestContext();
+    monster_ = profile->GetRequestContext()->GetURLRequestContext()->
+        cookie_store()->GetCookieMonster();
+  }
+
+  // Returns true, if the given cookie exists in the cookie store.
+  bool ContainsCookie() {
+    get_cookie_success_ = false;
+    monster_->GetCookiesWithOptionsAsync(
+        kOrigin1, net::CookieOptions(),
+        base::Bind(&RemoveCookieTester::GetCookieCallback,
+                   base::Unretained(this)));
+    BlockUntilNotified();
+    return get_cookie_success_;
+  }
+
+  void AddCookie() {
+    monster_->SetCookieWithOptionsAsync(
+        kOrigin1, "A=1", net::CookieOptions(),
+        base::Bind(&RemoveCookieTester::SetCookieCallback,
+                   base::Unretained(this)));
+    BlockUntilNotified();
+  }
+
+ private:
+  void GetCookieCallback(const std::string& cookies) {
+    if (cookies == "A=1") {
+      get_cookie_success_ = true;
+    } else {
+      EXPECT_EQ(cookies, "");
+      get_cookie_success_ = false;
+    }
+    Notify();
+  }
+
+  void SetCookieCallback(bool result) {
+    ASSERT_TRUE(result);
+    Notify();
+  }
+
+  bool get_cookie_success_;
+
+  net::CookieStore* monster_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoveCookieTester);
+};
+
 class RemoveHistoryTester : public BrowsingDataRemoverTester {
  public:
   explicit RemoveHistoryTester(TestingProfile* profile)
@@ -220,6 +296,19 @@ class BrowsingDataRemoverTest : public TestingBrowserProcessTest {
 
 // Tests ---------------------------------------------------------------------
 
+TEST_F(BrowsingDataRemoverTest, RemoveCookieForever) {
+  scoped_ptr<RemoveCookieTester> tester(
+      new RemoveCookieTester(GetProfile()));
+
+  tester->AddCookie();
+  ASSERT_TRUE(tester->ContainsCookie());
+
+  BlockUntilBrowsingDataRemoved(BrowsingDataRemover::EVERYTHING,
+      base::Time::Now(), BrowsingDataRemover::REMOVE_COOKIES, tester.get());
+
+  EXPECT_FALSE(tester->ContainsCookie());
+}
+
 TEST_F(BrowsingDataRemoverTest, RemoveHistoryForever) {
   scoped_ptr<RemoveHistoryTester> tester(
       new RemoveHistoryTester(GetProfile()));