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

chrome/browser/extensions/extension_cookies_api.cc

-// 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.
 
 // Implements the Chrome Extensions Cookies API.
 
 #include "chrome/browser/extensions/extension_cookies_api.h"
 
 #include "base/json/json_writer.h"
 #include "base/task.h"
 #include "base/values.h"
@@ skipping 131 matching lines @@
   return true;
 }
 
 GetCookieFunction::GetCookieFunction() {}
 
 GetCookieFunction::~GetCookieFunction() {}
 
 bool GetCookieFunction::RunImpl() {
   // Return false if the arguments are malformed.
   DictionaryValue* details;
   EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details));

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

so if this fails, you return with undefined result instead of null result. is
this intended?

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

Based on what Aaron said in his comments, it seems to be the expected result. 
It's consistent with the "error" behavior of the other API functions, though I
think it's arguable that that behavior isn't desirable.  :)

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

Agree the current mechanism is bad, but I'd rather be consistent in this case.

   DCHECK(details);
 
   // Read/validate input parameters.
   if (!ParseUrl(details, &url_, true))
     return false;
 
   // Get the cookie name string or return false.
   EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kNameKey, &name_));
 
   URLRequestContextGetter* store_context = NULL;
   if (!ParseStoreContext(details, &store_context, &store_id_))
     return false;
 
   DCHECK(store_context && !store_id_.empty());
   store_context_ = store_context;
 
   bool rv = BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(this, &GetCookieFunction::GetCookieOnIOThread));
   DCHECK(rv);
 
   // Will finish asynchronously.
   return true;
 }
 
 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_ =
+
+  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_);
-
-  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) {
-    // 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_));
       break;
     }
   }
 
-  // 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);
 }
 
 GetAllCookiesFunction::GetAllCookiesFunction() : details_(NULL) {}
 
 GetAllCookiesFunction::~GetAllCookiesFunction() {}
 
 bool GetAllCookiesFunction::RunImpl() {
   // Return false if the arguments are malformed.
   EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details_));
@@ skipping 14 matching lines @@
       NewRunnableMethod(this, &GetAllCookiesFunction::GetAllCookiesOnIOThread));
   DCHECK(rv);
 
   // Will finish asynchronously.
   return true;
 }
 
 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();
     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));
+  DCHECK(rv);
+}
+
+void GetAllCookiesFunction::RespondOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   SendResponse(true);
 }
 
 SetCookieFunction::SetCookieFunction()
     : secure_(false),
       http_only_(false),
       success_(false) {
 }
 
 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));
   DCHECK(details);
 
   // Read/validate input parameters.
   if (!ParseUrl(details, &url_, true))
       return false;
   // The macros below return false if argument types are not as expected.
   if (details->HasKey(keys::kNameKey))
@@ skipping 43 matching lines @@
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(this, &SetCookieFunction::SetCookieOnIOThread));
   DCHECK(rv);
 
   // Will finish asynchronously.
   return true;
 }
 
 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));
   DCHECK(rv);
 }
 
 void SetCookieFunction::RespondOnUIThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!success_) {
     error_ = ExtensionErrorUtils::FormatErrorMessage(
         keys::kCookieSetFailedError, name_);
   }
+
   SendResponse(success_);
 }
 
 namespace {
 
 class RemoveCookieTask : public Task {
  public:
   RemoveCookieTask(const GURL& url,
                    const std::string& name,
                    const scoped_refptr<URLRequestContextGetter>& context_getter)
@@ skipping 10 matching lines @@
   const GURL url_;
   const std::string name_;
   const scoped_refptr<URLRequestContextGetter> context_getter_;
 
   DISALLOW_COPY_AND_ASSIGN(RemoveCookieTask);
 };
 
 }  // 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));
   DCHECK(details);
 
   // Read/validate input parameters.
   GURL url;
   if (!ParseUrl(details, &url, true))
     return false;
 
   std::string name;
   // Get the cookie name string or return false.
   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);
 
   // We don't bother to synchronously wait for the result here, because
   // CookieMonster is only ever accessed on the IO thread, so any other accesses
   // should happen after this.
   bool rv = BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       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;
 }
 
 void RemoveCookieFunction::Run() {
   SendResponse(RunImpl());
 }
 
 bool GetAllCookieStoresFunction::RunImpl() {
   Profile* original_profile = profile();
   DCHECK(original_profile);
@@ skipping 34 matching lines @@
         extension_cookies_helpers::CreateCookieStoreValue(
             incognito_profile, incognito_tab_ids.release()));
   }
   result_.reset(cookie_store_list);
   return true;
 }
 
 void GetAllCookieStoresFunction::Run() {
   SendResponse(RunImpl());
 }