Adds Asynchronous APIs to CookieManager

android_webview/native/cookie_manager.cc

Index: android_webview/native/cookie_manager.cc
diff --git a/android_webview/native/cookie_manager.cc b/android_webview/native/cookie_manager.cc
index adb0ec87b5a804c53b906c3aec64e684552e8f55..47092ab8df041191df75abb0060f332d31c3a377 100644
--- a/android_webview/native/cookie_manager.cc
+++ b/android_webview/native/cookie_manager.cc
@@ -35,8 +35,10 @@
 #include "net/url_request/url_request_context.h"
 
 using base::FilePath;
+using base::WaitableEvent;
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertJavaStringToUTF16;
+using base::android::ScopedJavaGlobalRef;
 using content::BrowserThread;
 using net::CookieList;
 using net::CookieMonster;
@@ -53,6 +55,44 @@ using net::CookieMonster;
 
 namespace android_webview {
 
+class RemoveCookiesCallback {

[mkosiba (inactive), 2014/05/13 09:37:17]

same here. CookieCallbackInteger, CookieCallbackBoolean

[hjd_google, 2014/05/15 10:52:07]

Done.

+ public:
+  RemoveCookiesCallback(JNIEnv* env, jobject callback) {
+    callback_.Reset(env, callback);
+  }
+
+  void Invoke(int num_deleted) {
+    if (!callback_.is_null()) {
+      JNIEnv* env = base::android::AttachCurrentThread();
+      Java_AwCookieManager_invokeRemoveCookieCallback(
+          env, callback_.obj(), num_deleted);
+    }
+  }
+
+ private:
+  ScopedJavaGlobalRef<jobject> callback_;
+  DISALLOW_COPY_AND_ASSIGN(RemoveCookiesCallback);
+};
+
+class SetCookieCallback {
+ public:
+  SetCookieCallback(JNIEnv* env, jobject callback) {
+    callback_.Reset(env, callback);
+  }
+
+  void Invoke(bool success) {
+    if (!callback_.is_null()) {
+      JNIEnv* env = base::android::AttachCurrentThread();
+      Java_AwCookieManager_invokeSetCookieCallback(
+          env, callback_.obj(), success);
+    }
+  }
+
+ private:
+  ScopedJavaGlobalRef<jobject> callback_;
+  DISALLOW_COPY_AND_ASSIGN(SetCookieCallback);
+};
+
 namespace {
 
 // Are cookies allowed for file:// URLs by default?
@@ -98,10 +138,16 @@ class CookieManager {
   bool AcceptCookie();
   void SetAcceptThirdPartyCookie(bool accept);
   bool AcceptThirdPartyCookie();
-  void SetCookie(const GURL& host, const std::string& cookie_value);
+  void SetCookie(const GURL& host,
+                 const std::string& cookie_value,
+                 scoped_ptr<SetCookieCallback> callback);
+  void SetCookieSync(const GURL& host,
+                 const std::string& cookie_value);
   std::string GetCookie(const GURL& host);
-  void RemoveSessionCookie();
-  void RemoveAllCookie();
+  void RemoveSessionCookie(scoped_ptr<RemoveCookiesCallback> callback);
+  void RemoveAllCookie(scoped_ptr<RemoveCookiesCallback> callback);
+  void RemoveAllCookieSync();
+  void RemoveSessionCookieSync();
   void RemoveExpiredCookie();
   void FlushCookieStore();
   bool HasCookies();
@@ -114,32 +160,44 @@ class CookieManager {
   CookieManager();
   ~CookieManager();
 
-  typedef base::Callback<void(base::WaitableEvent*)> CookieTask;
+  typedef base::Callback<void(WaitableEvent*)> CookieSyncTask;
+  typedef base::Callback<void()> CookieTask;

[mkosiba (inactive), 2014/05/13 09:37:17]

FYI: you're redefining base::Closure

+  void ExecCookieTaskSync(const CookieSyncTask& task);
   void ExecCookieTask(const CookieTask& task);
 
-  void SetCookieAsyncHelper(
+  void SetCookieHelper(
+      const GURL& host,
+      const std::string& value,
+      scoped_ptr<SetCookieCallback> callback);
+  void SetCookieSyncHelper(
       const GURL& host,
       const std::string& value,
-      base::WaitableEvent* completion);
-  void SetCookieCompleted(base::WaitableEvent* completion, bool success);
+      WaitableEvent* completion);
+  void SetCookieCompleted(scoped_ptr<SetCookieCallback> callback, bool success);
+  void SetCookieSyncCompleted(WaitableEvent* completion, bool success);
 
   void GetCookieValueAsyncHelper(
       const GURL& host,
       std::string* result,
-      base::WaitableEvent* completion);
-  void GetCookieValueCompleted(base::WaitableEvent* completion,
+      WaitableEvent* completion);
+  void GetCookieValueCompleted(WaitableEvent* completion,
                                std::string* result,
                                const std::string& value);
 
-  void RemoveSessionCookieAsyncHelper(base::WaitableEvent* completion);
-  void RemoveAllCookieAsyncHelper(base::WaitableEvent* completion);
-  void RemoveCookiesCompleted(base::WaitableEvent* completion, int num_deleted);
+  void RemoveSessionCookieHelper(scoped_ptr<RemoveCookiesCallback> callback);
+  void RemoveAllCookieHelper(scoped_ptr<RemoveCookiesCallback> callback);
+  void RemoveCookiesCompleted(scoped_ptr<RemoveCookiesCallback> callback,
+                              int num_deleted);
+
+  void RemoveSessionCookieSyncHelper(WaitableEvent* completion);
+  void RemoveAllCookieSyncHelper(WaitableEvent* completion);
+  void RemoveCookiesSyncCompleted(WaitableEvent* completion, int num_deleted);
 
-  void FlushCookieStoreAsyncHelper(base::WaitableEvent* completion);
+  void FlushCookieStoreAsyncHelper(WaitableEvent* completion);
 
   void HasCookiesAsyncHelper(bool* result,
-                             base::WaitableEvent* completion);
-  void HasCookiesCompleted(base::WaitableEvent* completion,
+                             WaitableEvent* completion);
+  void HasCookiesCompleted(WaitableEvent* completion,
                            bool* result,
                            const CookieList& cookies);
 
@@ -163,6 +221,7 @@ class CookieManager {
   DISALLOW_COPY_AND_ASSIGN(CookieManager);
 };
 
+
 base::LazyInstance<CookieManager>::Leaky g_lazy_instance;
 
 // static
@@ -221,22 +280,27 @@ void CookieManager::EnsureCookieMonsterExistsLocked() {
                       cookie_monster_backend_thread_->message_loop_proxy());
 }
 
-// Executes the |task| on the |cookie_monster_proxy_| message loop.
-void CookieManager::ExecCookieTask(const CookieTask& task) {
-  base::WaitableEvent completion(false, false);
+// Executes the |task| on the |cookie_monster_proxy_| message loop and
+// waits for it to complete before returning.
+void CookieManager::ExecCookieTaskSync(const CookieSyncTask& task) {
+  WaitableEvent completion(false, false);
   base::AutoLock lock(cookie_monster_lock_);
 
   EnsureCookieMonsterExistsLocked();
 
   cookie_monster_proxy_->PostTask(FROM_HERE, base::Bind(task, &completion));
 
-  // We always wait for the posted task to complete, even when it doesn't return
-  // a value, because previous versions of the CookieManager API were
-  // synchronous in most/all cases and the caller may be relying on this.
   ScopedAllowWaitForLegacyWebViewApi wait;
   completion.Wait();
 }
 
+// Executes the |task| on the |cookie_monster_proxy_| message loop.
+void CookieManager::ExecCookieTask(const CookieTask& task) {
+  base::AutoLock lock(cookie_monster_lock_);
+  EnsureCookieMonsterExistsLocked();
+  cookie_monster_proxy_->PostTask(FROM_HERE, task);
+}
+
 scoped_refptr<net::CookieStore> CookieManager::CreateBrowserThreadCookieStore(
     AwBrowserContext* browser_context) {
   base::AutoLock lock(cookie_monster_lock_);
@@ -284,17 +348,27 @@ bool CookieManager::AcceptThirdPartyCookie() {
 }
 
 void CookieManager::SetCookie(const GURL& host,
+                              const std::string& cookie_value,
+                              scoped_ptr<SetCookieCallback> callback) {
+  ExecCookieTask(base::Bind(&CookieManager::SetCookieHelper,
+                            base::Unretained(this),
+                            host,
+                            cookie_value,
+                            Passed(&callback)));
+}
+
+void CookieManager::SetCookieSync(const GURL& host,
                               const std::string& cookie_value) {
-  ExecCookieTask(base::Bind(&CookieManager::SetCookieAsyncHelper,
+  ExecCookieTaskSync(base::Bind(&CookieManager::SetCookieSyncHelper,
                             base::Unretained(this),
                             host,
                             cookie_value));
 }
 
-void CookieManager::SetCookieAsyncHelper(
+void CookieManager::SetCookieHelper(
     const GURL& host,
     const std::string& value,
-    base::WaitableEvent* completion) {
+    scoped_ptr<SetCookieCallback> callback) {
   net::CookieOptions options;
   options.set_include_httponly();
 
@@ -302,19 +376,36 @@ void CookieManager::SetCookieAsyncHelper(
       host, value, options,
       base::Bind(&CookieManager::SetCookieCompleted,
                  base::Unretained(this),
+                 Passed(&callback)));
+}
+
+void CookieManager::SetCookieSyncHelper(
+    const GURL& host,
+    const std::string& value,
+    WaitableEvent* completion) {
+  net::CookieOptions options;
+  options.set_include_httponly();
+
+  cookie_monster_->SetCookieWithOptionsAsync(
+      host, value, options,
+      base::Bind(&CookieManager::SetCookieSyncCompleted,
+                 base::Unretained(this),
                  completion));
 }
 
-void CookieManager::SetCookieCompleted(base::WaitableEvent* completion,
+void CookieManager::SetCookieCompleted(scoped_ptr<SetCookieCallback> callback,
                                        bool success) {
-  // The CookieManager API does not return a value for SetCookie,
-  // so we don't need to propagate the |success| value back to the caller.
+  callback->Invoke(success);
+}
+
+void CookieManager::SetCookieSyncCompleted(WaitableEvent* completion,
+                                           bool success) {
   completion->Signal();
 }
 
 std::string CookieManager::GetCookie(const GURL& host) {
   std::string cookie_value;
-  ExecCookieTask(base::Bind(&CookieManager::GetCookieValueAsyncHelper,
+  ExecCookieTaskSync(base::Bind(&CookieManager::GetCookieValueAsyncHelper,
                             base::Unretained(this),
                             host,
                             &cookie_value));
@@ -325,7 +416,7 @@ std::string CookieManager::GetCookie(const GURL& host) {
 void CookieManager::GetCookieValueAsyncHelper(
     const GURL& host,
     std::string* result,
-    base::WaitableEvent* completion) {
+    WaitableEvent* completion) {
   net::CookieOptions options;
   options.set_include_httponly();
 
@@ -338,44 +429,77 @@ void CookieManager::GetCookieValueAsyncHelper(
                  result));
 }
 
-void CookieManager::GetCookieValueCompleted(base::WaitableEvent* completion,
+void CookieManager::GetCookieValueCompleted(WaitableEvent* completion,
                                             std::string* result,
                                             const std::string& value) {
   *result = value;
   completion->Signal();
 }
 
-void CookieManager::RemoveSessionCookie() {
-  ExecCookieTask(base::Bind(&CookieManager::RemoveSessionCookieAsyncHelper,
+void CookieManager::RemoveSessionCookie(
+    scoped_ptr<RemoveCookiesCallback> callback) {
+  ExecCookieTask(base::Bind(&CookieManager::RemoveSessionCookieHelper,
+                            base::Unretained(this),
+                            Passed(&callback)));
+}
+
+void CookieManager::RemoveSessionCookieSync() {
+  ExecCookieTaskSync(base::Bind(&CookieManager::RemoveSessionCookieSyncHelper,
                             base::Unretained(this)));
 }
 
-void CookieManager::RemoveSessionCookieAsyncHelper(
-    base::WaitableEvent* completion) {
+void CookieManager::RemoveSessionCookieHelper(
+    scoped_ptr<RemoveCookiesCallback> callback) {
   cookie_monster_->DeleteSessionCookiesAsync(
       base::Bind(&CookieManager::RemoveCookiesCompleted,
                  base::Unretained(this),
+                 Passed(&callback)));
+}
+
+void CookieManager::RemoveSessionCookieSyncHelper(WaitableEvent* completion) {
+  cookie_monster_->DeleteSessionCookiesAsync(
+      base::Bind(&CookieManager::RemoveCookiesSyncCompleted,
+                 base::Unretained(this),
                  completion));
 }
 
-void CookieManager::RemoveCookiesCompleted(base::WaitableEvent* completion,
-                                           int num_deleted) {
-  // The CookieManager API does not return a value for removeSessionCookie or
-  // removeAllCookie, so we don't need to propagate the |num_deleted| value back
-  // to the caller.
+void CookieManager::RemoveCookiesCompleted(

[mkosiba (inactive), 2014/05/13 09:37:17]

like we talked offline - let's try and collapse the sync and async paths. I
think it should be possible to get around some of the repetition.

[hjd_google, 2014/05/15 10:52:07]

Done!

+    scoped_ptr<RemoveCookiesCallback> callback,
+    int num_deleted) {
+  callback->Invoke(num_deleted);
+}
+
+void CookieManager::RemoveCookiesSyncCompleted(
+    WaitableEvent* completion,
+    int num_deleted) {
   completion->Signal();
 }
 
-void CookieManager::RemoveAllCookie() {
-  ExecCookieTask(base::Bind(&CookieManager::RemoveAllCookieAsyncHelper,
+void CookieManager::RemoveAllCookie(
+    scoped_ptr<RemoveCookiesCallback> callback) {
+  ExecCookieTask(base::Bind(&CookieManager::RemoveAllCookieHelper,
+                            base::Unretained(this),
+                            Passed(&callback)));
+}
+
+void CookieManager::RemoveAllCookieSync() {
+  ExecCookieTaskSync(base::Bind(&CookieManager::RemoveAllCookieSyncHelper,
                             base::Unretained(this)));
 }
 
-void CookieManager::RemoveAllCookieAsyncHelper(
-    base::WaitableEvent* completion) {
+void CookieManager::RemoveAllCookieHelper(
+    scoped_ptr<RemoveCookiesCallback> callback) {
   cookie_monster_->DeleteAllAsync(
       base::Bind(&CookieManager::RemoveCookiesCompleted,
                  base::Unretained(this),
+                 Passed(&callback)));
+}
+
+void CookieManager::RemoveAllCookieSyncHelper(
+    WaitableEvent* completion) {
+  cookie_monster_->DeleteAllAsync(
+      base::Bind(&CookieManager::RemoveCookiesSyncCompleted,
+                 base::Unretained(this),
                  completion));
 }
 
@@ -385,19 +509,19 @@ void CookieManager::RemoveExpiredCookie() {
 }
 
 void CookieManager::FlushCookieStoreAsyncHelper(
-    base::WaitableEvent* completion) {
-  cookie_monster_->FlushStore(base::Bind(&base::WaitableEvent::Signal,
+    WaitableEvent* completion) {
+  cookie_monster_->FlushStore(base::Bind(&WaitableEvent::Signal,
                                          base::Unretained(completion)));
 }
 
 void CookieManager::FlushCookieStore() {
-  ExecCookieTask(base::Bind(&CookieManager::FlushCookieStoreAsyncHelper,
+  ExecCookieTaskSync(base::Bind(&CookieManager::FlushCookieStoreAsyncHelper,
                             base::Unretained(this)));
 }
 
 bool CookieManager::HasCookies() {
   bool has_cookies;
-  ExecCookieTask(base::Bind(&CookieManager::HasCookiesAsyncHelper,
+  ExecCookieTaskSync(base::Bind(&CookieManager::HasCookiesAsyncHelper,
                             base::Unretained(this),
                             &has_cookies));
   return has_cookies;
@@ -406,7 +530,7 @@ bool CookieManager::HasCookies() {
 // TODO(kristianm): Simplify this, copying the entire list around
 // should not be needed.
 void CookieManager::HasCookiesAsyncHelper(bool* result,
-                                  base::WaitableEvent* completion) {
+                                  WaitableEvent* completion) {
   cookie_monster_->GetAllCookiesAsync(
       base::Bind(&CookieManager::HasCookiesCompleted,
                  base::Unretained(this),
@@ -414,7 +538,7 @@ void CookieManager::HasCookiesAsyncHelper(bool* result,
                  result));
 }
 
-void CookieManager::HasCookiesCompleted(base::WaitableEvent* completion,
+void CookieManager::HasCookiesCompleted(WaitableEvent* completion,
                                         bool* result,
                                         const CookieList& cookies) {
   *result = cookies.size() != 0;
@@ -467,11 +591,25 @@ static jboolean AcceptThirdPartyCookie(JNIEnv* env, jobject obj) {
   return CookieManager::GetInstance()->AcceptThirdPartyCookie();
 }
 
-static void SetCookie(JNIEnv* env, jobject obj, jstring url, jstring value) {
+static void SetCookie(JNIEnv* env,
+                      jobject obj,
+                      jstring url,
+                      jstring value,
+                      jobject java_callback) {
   GURL host(ConvertJavaStringToUTF16(env, url));
   std::string cookie_value(ConvertJavaStringToUTF8(env, value));
+  scoped_ptr<SetCookieCallback> callback(
+      new SetCookieCallback(env, java_callback));
+  CookieManager::GetInstance()->SetCookie(host, cookie_value, callback.Pass());
+}
 
-  CookieManager::GetInstance()->SetCookie(host, cookie_value);
+static void SetCookieSync(JNIEnv* env,
+                      jobject obj,
+                      jstring url,
+                      jstring value) {
+  GURL host(ConvertJavaStringToUTF16(env, url));
+  std::string cookie_value(ConvertJavaStringToUTF8(env, value));
+  CookieManager::GetInstance()->SetCookieSync(host, cookie_value);
 }
 
 static jstring GetCookie(JNIEnv* env, jobject obj, jstring url) {
@@ -482,12 +620,26 @@ static jstring GetCookie(JNIEnv* env, jobject obj, jstring url) {
       CookieManager::GetInstance()->GetCookie(host)).Release();
 }
 
-static void RemoveSessionCookie(JNIEnv* env, jobject obj) {
-  CookieManager::GetInstance()->RemoveSessionCookie();
+static void RemoveSessionCookie(JNIEnv* env,
+                                jobject obj,
+                                jobject java_callback) {
+  scoped_ptr<RemoveCookiesCallback> callback(
+      new RemoveCookiesCallback(env, java_callback));
+  CookieManager::GetInstance()->RemoveSessionCookie(callback.Pass());
+}
+
+static void RemoveSessionCookieSync(JNIEnv* env, jobject obj) {
+    CookieManager::GetInstance()->RemoveSessionCookieSync();
+}
+
+static void RemoveAllCookie(JNIEnv* env, jobject obj, jobject java_callback) {
+  scoped_ptr<RemoveCookiesCallback> callback(
+      new RemoveCookiesCallback(env, java_callback));
+  CookieManager::GetInstance()->RemoveAllCookie(callback.Pass());
 }
 
-static void RemoveAllCookie(JNIEnv* env, jobject obj) {
-  CookieManager::GetInstance()->RemoveAllCookie();
+static void RemoveAllCookieSync(JNIEnv* env, jobject obj) {
+    CookieManager::GetInstance()->RemoveAllCookieSync();
 }
 
 static void RemoveExpiredCookie(JNIEnv* env, jobject obj) {