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

chrome/browser/extensions/extension_cookies_api.cc

Index: chrome/browser/extensions/extension_cookies_api.cc
diff --git a/chrome/browser/extensions/extension_cookies_api.cc b/chrome/browser/extensions/extension_cookies_api.cc
index 04d5600815c345b951a31555c29150db7b769e77..3d663ece3dc2d5cf13678d39571e44bfd361f7c4 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.
 
@@ -177,25 +177,16 @@ bool GetCookieFunction::RunImpl() {
 }
 
 void GetCookieFunction::GetCookieOnIOThread() {
+  // Start with "null" as return value
+  result_.reset(Value::CreateNullValue());
+
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   net::CookieStore* cookie_store = store_context_->GetCookieStore();
-  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 cookie_list_ =

[jochen (gone - plz use gerrit), 2011/02/16 10:02:09]

no trailing underscore for local variables

[Mike West, 2011/02/16 11:07:28]

Done.

+      extension_cookies_helpers::GetCookieListFromStore(cookie_store, 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

[jochen (gone - plz use gerrit), 2011/02/16 10:02:09]

why did you remove the comment?

[Mike West, 2011/02/16 11:07:28]

Copy/paste error.  Fixed now.

-    // 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_));
@@ -203,10 +194,14 @@ void GetCookieFunction::RespondOnUIThread() {
     }
   }
 
-  // The cookie doesn't exist; return null.
-  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,18 +236,9 @@ bool GetAllCookiesFunction::RunImpl() {
 void GetAllCookiesFunction::GetAllCookiesOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   net::CookieStore* cookie_store = store_context_->GetCookieStore();
-  cookie_list_ =
+  net::CookieList cookie_list_ =

[jochen (gone - plz use gerrit), 2011/02/16 10:02:09]

no underscore

[Mike West, 2011/02/16 11:07:28]

Done.

       extension_cookies_helpers::GetCookieListFromStore(cookie_store, url_);
 
-  bool rv = BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &GetAllCookiesFunction::RespondOnUIThread));
-  DCHECK(rv);
-}
-
-void GetAllCookiesFunction::RespondOnUIThread() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
   const Extension* extension = GetExtension();

[jochen (gone - plz use gerrit), 2011/02/16 10:02:09]

not sure whether Extension is thread safe?

[Mike West, 2011/02/16 11:07:28]

No idea.  I was copy/pasting here for consistency with the cookie_list-less
`GetCookieFunction`.  :)

[Aaron Boodman, 2011/02/16 19:45:10]

It is. The intention is that the const Extension* instances in the browser
process are immutable and can be used from any thread.

   if (extension) {
     ListValue* matching_list = new ListValue();
@@ -261,6 +247,14 @@ void GetAllCookiesFunction::RespondOnUIThread() {
         GetExtension(), matching_list);
     result_.reset(matching_list);
   }
+  bool rv = BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      NewRunnableMethod(this, &GetAllCookiesFunction::RespondOnUIThread));
+  DCHECK(rv);
+}
+
+void GetAllCookiesFunction::RespondOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   SendResponse(true);
 }
 
@@ -274,6 +268,10 @@ SetCookieFunction::~SetCookieFunction() {
 }
 
 bool SetCookieFunction::RunImpl() {
+  // Start with "null" being returned to callback, in case things fail in
+  // this synchronous bit of the workflow.
+  result_.reset(Value::CreateNullValue());
+
   // Return false if the arguments are malformed.
   DictionaryValue* details;
   EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details));
@@ -337,12 +335,24 @@ bool SetCookieFunction::RunImpl() {
 
 void SetCookieFunction::SetCookieOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  net::CookieMonster* cookie_monster =
-      store_context_->GetCookieStore()->GetCookieMonster();
+  net::CookieStore* cookie_store = store_context_->GetCookieStore();
+  net::CookieMonster* cookie_monster = cookie_store->GetCookieMonster();
   success_ = cookie_monster->SetCookieWithDetails(
       url_, name_, value_, domain_, path_, expiration_time_,
       secure_, http_only_);
 
+  // Pull the newly set cookie.
+  net::CookieList cookie_list_ =

[jochen (gone - plz use gerrit), 2011/02/16 10:02:09]

no underscore

[Mike West, 2011/02/16 11:07:28]

Done.

+      extension_cookies_helpers::GetCookieListFromStore(cookie_store, url_);
+  net::CookieList::iterator it;
+  for (it = cookie_list_.begin(); it != cookie_list_.end(); ++it) {
+    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));
@@ -355,6 +365,7 @@ void SetCookieFunction::RespondOnUIThread() {
     error_ = ExtensionErrorUtils::FormatErrorMessage(
         keys::kCookieSetFailedError, name_);
   }
+
   SendResponse(success_);
 }
 
@@ -385,6 +396,10 @@ class RemoveCookieTask : public Task {
 }  // namespace
 
 bool RemoveCookieFunction::RunImpl() {
+  // Start with "null" being returned to callback, in case things fail in
+  // this syncronous bit of the workflow.
+  result_.reset(Value::CreateNullValue());
+
   // Return false if the arguments are malformed.
   DictionaryValue* details;
   EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details));
@@ -400,7 +415,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_string;

[jochen (gone - plz use gerrit), 2011/02/16 10:02:09]

just store_id

[Mike West, 2011/02/16 11:07:28]

Done.

+  if (!ParseStoreContext(details, &store_context, &store_id_string))
     return false;
   DCHECK(store_context);
 
@@ -412,6 +428,14 @@ bool RemoveCookieFunction::RunImpl() {
       new RemoveCookieTask(url, name, make_scoped_refptr(store_context)));
   DCHECK(rv);
 
+  DictionaryValue* resultDictionary = new DictionaryValue();
+  std::string url_string;

[jochen (gone - plz use gerrit), 2011/02/16 10:02:09]

just url

[Mike West, 2011/02/16 11:07:28]

`url` is already a variable in this scope (line 409).  That's why I appended
`_string` to it, and `store_id`.

+  details->GetString(keys::kUrlKey, &url_string);
+  resultDictionary->SetString(keys::kNameKey, name);
+  resultDictionary->SetString(keys::kUrlKey, url_string);
+  resultDictionary->SetString(keys::kStoreIdKey, store_id_string);
+  result_.reset(resultDictionary);
+
   return true;
 }