Adding callbacks to `chrome.cookies.{set,remove}`.

Currently, neither `.set` nor `.remove` give the user the
ability to take action after the asynchronous cookie
monster does its thing in the background.  One can listen
for events, but can't distinguish between events generated
from one's own code, and events generated through
normal interaction with the web.  This makes infinite
loops much easier to write than they should be.  Callbacks
make that class of problem simpler to deal with.

BUG=70102
TEST=Check that `.set` and `.remove` callbacks are triggered.

Review URL: http://codereview.chromium.org/6525016
Patch from Mike West <mkwst@chromium.org>.

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

2 files changed, 46 insertions, 28 deletions

diff --git a/chrome/browser/extensions/extension_cookies_api.cc b/chrome/browser/extensions/extension_cookies_api.cc
index 04d5600..22b29da 100644
--- a/chrome/browser/extensions/extension_cookies_api.cc
+++ b/chrome/browser/extensions/extension_cookies_api.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -179,20 +179,10 @@ bool GetCookieFunction::RunImpl() {
 void GetCookieFunction::GetCookieOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   net::CookieStore* cookie_store = store_context_->GetCookieStore();
-  cookie_list_ =
+  net::CookieList cookie_list =
       extension_cookies_helpers::GetCookieListFromStore(cookie_store, url_);
-
-  bool rv = BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &GetCookieFunction::RespondOnUIThread));
-  DCHECK(rv);
-}
-
-void GetCookieFunction::RespondOnUIThread() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
   net::CookieList::iterator it;
-  for (it = cookie_list_.begin(); it != cookie_list_.end(); ++it) {
+  for (it = cookie_list.begin(); it != cookie_list.end(); ++it) {
     // Return the first matching cookie. Relies on the fact that the
     // CookieMonster returns them in canonical order (longest path, then
     // earliest creation time).
@@ -204,9 +194,17 @@ void GetCookieFunction::RespondOnUIThread() {
   }
 
   // The cookie doesn't exist; return null.
-  if (it == cookie_list_.end())
+  if (it == cookie_list.end())
     result_.reset(Value::CreateNullValue());
 
+  bool rv = BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      NewRunnableMethod(this, &GetCookieFunction::RespondOnUIThread));
+  DCHECK(rv);
+}
+
+void GetCookieFunction::RespondOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   SendResponse(true);
 }
 
@@ -241,9 +239,17 @@ bool GetAllCookiesFunction::RunImpl() {
 void GetAllCookiesFunction::GetAllCookiesOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   net::CookieStore* cookie_store = store_context_->GetCookieStore();
-  cookie_list_ =
+  net::CookieList cookie_list =
       extension_cookies_helpers::GetCookieListFromStore(cookie_store, url_);
 
+  const Extension* extension = GetExtension();
+  if (extension) {
+    ListValue* matching_list = new ListValue();
+    extension_cookies_helpers::AppendMatchingCookiesToList(
+        cookie_list, store_id_, url_, details_,
+        GetExtension(), matching_list);
+    result_.reset(matching_list);
+  }
   bool rv = BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(this, &GetAllCookiesFunction::RespondOnUIThread));
@@ -252,15 +258,6 @@ void GetAllCookiesFunction::GetAllCookiesOnIOThread() {
 
 void GetAllCookiesFunction::RespondOnUIThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  const Extension* extension = GetExtension();
-  if (extension) {
-    ListValue* matching_list = new ListValue();
-    extension_cookies_helpers::AppendMatchingCookiesToList(
-        cookie_list_, store_id_, url_, details_,
-        GetExtension(), matching_list);
-    result_.reset(matching_list);
-  }
   SendResponse(true);
 }
 
@@ -343,6 +340,21 @@ void SetCookieFunction::SetCookieOnIOThread() {
       url_, name_, value_, domain_, path_, expiration_time_,
       secure_, http_only_);
 
+  // Pull the newly set cookie.
+  net::CookieList cookie_list =
+      extension_cookies_helpers::GetCookieListFromStore(cookie_monster, url_);
+  net::CookieList::iterator it;
+  for (it = cookie_list.begin(); it != cookie_list.end(); ++it) {
+    // Return the first matching cookie. Relies on the fact that the
+    // CookieMonster returns them in canonical order (longest path, then
+    // earliest creation time).
+    if (it->Name() == name_) {
+      result_.reset(
+          extension_cookies_helpers::CreateCookieValue(*it, store_id_));
+      break;
+    }
+  }
+
   bool rv = BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(this, &SetCookieFunction::RespondOnUIThread));
@@ -400,7 +412,8 @@ bool RemoveCookieFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kNameKey, &name));
 
   URLRequestContextGetter* store_context = NULL;
-  if (!ParseStoreContext(details, &store_context, NULL))
+  std::string store_id;
+  if (!ParseStoreContext(details, &store_context, &store_id))
     return false;
   DCHECK(store_context);
 
@@ -412,6 +425,12 @@ bool RemoveCookieFunction::RunImpl() {
       new RemoveCookieTask(url, name, make_scoped_refptr(store_context)));
   DCHECK(rv);
 
+  DictionaryValue* resultDictionary = new DictionaryValue();
+  resultDictionary->SetString(keys::kNameKey, name);
+  resultDictionary->SetString(keys::kUrlKey, url.spec());
+  resultDictionary->SetString(keys::kStoreIdKey, store_id);
+  result_.reset(resultDictionary);
+
   return true;
 }
 
diff --git a/chrome/browser/extensions/extension_cookies_api.h b/chrome/browser/extensions/extension_cookies_api.h
index dc81731..4f2c97b 100644
--- a/chrome/browser/extensions/extension_cookies_api.h
+++ b/chrome/browser/extensions/extension_cookies_api.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -106,7 +106,6 @@ class GetCookieFunction : public CookiesFunction {
   GURL url_;
   std::string store_id_;
   scoped_refptr<URLRequestContextGetter> store_context_;
-  net::CookieList cookie_list_;
 };
 
 // Implements the cookies.getAll() extension function.
@@ -125,7 +124,6 @@ class GetAllCookiesFunction : public CookiesFunction {
   GURL url_;
   std::string store_id_;
   scoped_refptr<URLRequestContextGetter> store_context_;
-  net::CookieList cookie_list_;
 };
 
 // Implements the cookies.set() extension function.
@@ -149,6 +147,7 @@ class SetCookieFunction : public CookiesFunction {
   bool http_only_;
   base::Time expiration_time_;
   bool success_;
+  std::string store_id_;
   scoped_refptr<URLRequestContextGetter> store_context_;
 };