Back out trunk r37998.

Modify CookiePolicy to work asynchronously

This change will enable us to prompt the user before setting a cookie.  While we only need to prompt before setting, we actually need to make both CanSetCookie and CanGetCookies asynchronous.  This is necessary in order to preserve FIFO ordering since the value returned by GetCookies depends on the changes made to the cookie DB by SetCookie.

This change also includes some simplification of CookieStore.  Instead of N virtual functions, I distilled it down to only 4.  The remaining functions are instead expressed in terms of those.

While studying all the places where we currently use CookiePolicy, I found that some of them were not appropriate.  After discussing with Amit, I decided to remove the policy checks in URLRequestAutomationJob.  See the comments in the code regarding this.

I changed the signature of CookieMonster::GetRawCookies to GetAllCookiesForURL to better match GetAllCookies.  I also filed a bug about making it even closer in functionality.  Related to this change webkit/glue/webcookie.h grows a constructor that takes a CanonicalCookie to help clean up some code.

On the Chrome side, ChromeURLRequestContext now has a ChromeCookiePolicy object.  That object is threadsafe ref counted because it is passed between the UI and IO threads.  It is responsible for implementing the queuing logic described above.  It will also in the future trigger the Chrome UI code to actually show the setcookie prompt.

Please review the state machinery changes in URLRequestHttpJob carefully.

R=eroman
BUG=34331
TEST=no tests yet for prompting.
Review URL: http://codereview.chromium.org/567015

TBR=darin@chromium.org
Review URL: http://codereview.chromium.org/562037

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

4 files changed, 46 insertions, 256 deletions

diff --git a/chrome/browser/net/chrome_cookie_policy.cc b/chrome/browser/net/chrome_cookie_policy.cc
deleted file mode 100644
index 3e924d1..0000000
--- a/chrome/browser/net/chrome_cookie_policy.cc
+++ /dev/null
@@ -1,162 +0,0 @@
-// Copyright (c) 2010 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.
-
-#include "chrome/browser/net/chrome_cookie_policy.h"
-
-#include "base/string_util.h"
-#include "chrome/browser/chrome_thread.h"
-#include "chrome/browser/host_content_settings_map.h"
-#include "googleurl/src/gurl.h"
-#include "net/base/net_errors.h"
-#include "net/base/static_cookie_policy.h"
-
-// If we queue up more than this number of completions, then switch from ASK to
-// BLOCK.  More than this number of requests at once seems like it could be a
-// sign of trouble anyways.
-static const size_t kMaxCompletionsPerHost = 10000;
-
-ChromeCookiePolicy::ChromeCookiePolicy(HostContentSettingsMap* map)
-    : host_content_settings_map_(map) {
-}
-
-ChromeCookiePolicy::~ChromeCookiePolicy() {
-  DCHECK(host_completions_map_.empty());
-}
-
-int ChromeCookiePolicy::CanGetCookies(const GURL& url,
-                                      const GURL& first_party,
-                                      net::CompletionCallback* callback) {
-  if (host_content_settings_map_->BlockThirdPartyCookies()) {
-    net::StaticCookiePolicy policy(
-        net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES);
-    int rv = policy.CanGetCookies(url, first_party, NULL);
-    if (rv != net::OK)
-      return rv;
-  }
-
-  const std::string& host = url.host();
-
-  ContentSetting setting = host_content_settings_map_->GetContentSetting(
-      host, CONTENT_SETTINGS_TYPE_COOKIES);
-  if (setting == CONTENT_SETTING_BLOCK)
-    return net::ERR_ACCESS_DENIED;
-  if (setting == CONTENT_SETTING_ALLOW)
-    return net::OK;
-
-  DCHECK(callback);
-
-  // If we are currently prompting the user for a 'set-cookie' matching this
-  // host, then we need to defer reading cookies.
-
-  HostCompletionsMap::iterator it = host_completions_map_.find(host);
-  if (it == host_completions_map_.end())
-    return net::OK;
-
-  if (it->second.size() >= kMaxCompletionsPerHost) {
-    LOG(ERROR) << "Would exceed kMaxCompletionsPerHost";
-    return net::ERR_ACCESS_DENIED;
-  }
-
-  it->second.push_back(Completion::ForGetCookies(callback));
-
-  return net::ERR_IO_PENDING;
-}
-
-int ChromeCookiePolicy::CanSetCookie(const GURL& url,
-                                     const GURL& first_party,
-                                     const std::string& cookie_line,
-                                     net::CompletionCallback* callback) {
-  if (host_content_settings_map_->BlockThirdPartyCookies()) {
-    net::StaticCookiePolicy policy(
-        net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES);
-    int rv = policy.CanSetCookie(url, first_party, cookie_line, NULL);
-    if (rv != net::OK)
-      return rv;
-  }
-
-  const std::string& host = url.host();
-
-  ContentSetting setting = host_content_settings_map_->GetContentSetting(
-      host, CONTENT_SETTINGS_TYPE_COOKIES);
-  if (setting == CONTENT_SETTING_BLOCK)
-    return net::ERR_ACCESS_DENIED;
-  if (setting == CONTENT_SETTING_ALLOW)
-    return net::OK;
-
-  DCHECK(callback);
-
-  // Else, ask the user...
-
-  Completions& completions = host_completions_map_[host];
-
-  if (completions.size() >= kMaxCompletionsPerHost) {
-    LOG(ERROR) << "Would exceed kMaxCompletionsPerHost";
-    return net::ERR_ACCESS_DENIED;
-  }
-
-  completions.push_back(Completion::ForSetCookie(callback));
-
-  PromptForSetCookie(host, cookie_line);
-  return net::ERR_IO_PENDING;
-}
-
-void ChromeCookiePolicy::PromptForSetCookie(const std::string &host,
-                                            const std::string& cookie_line) {
-  if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) {
-    ChromeThread::PostTask(
-        ChromeThread::UI, FROM_HERE,
-        NewRunnableMethod(this, &ChromeCookiePolicy::PromptForSetCookie, host,
-                          cookie_line));
-    return;
-  }
-
-  // TODO(darin): Prompt user!
-#if 0
-  MessageBox(NULL,
-             UTF8ToWide(cookie_line).c_str(),
-             UTF8ToWide(host).c_str(),
-             MB_OK);
-#endif
-
-  DidPromptForSetCookie(host, net::OK);
-}
-
-void ChromeCookiePolicy::DidPromptForSetCookie(const std::string &host,
-                                               int result) {
-  if (!ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    ChromeThread::PostTask(
-        ChromeThread::IO, FROM_HERE,
-        NewRunnableMethod(this, &ChromeCookiePolicy::DidPromptForSetCookie,
-                          host, result));
-    return;
-  }
-
-  // Notify all callbacks, starting with the first until we hit another that
-  // is for a 'set-cookie'.
-  HostCompletionsMap::iterator it = host_completions_map_.find(host);
-  CHECK(it != host_completions_map_.end());
-
-  Completions& completions = it->second;
-  CHECK(!completions.empty() && completions[0].is_set_cookie_request());
-
-  // Gather the list of callbacks to notify, and remove them from the
-  // completions list before handing control to the callbacks (in case
-  // they should call back into us to modify host_completions_map_).
-
-  std::vector<net::CompletionCallback*> callbacks;
-  callbacks.push_back(completions[0].callback());
-  size_t i = 1;
-  for (; i < completions.size(); ++i) {
-    if (completions[i].is_set_cookie_request())
-      break;
-    callbacks.push_back(completions[i].callback());
-  }
-  completions.erase(completions.begin(), completions.begin() + i);
-
-  if (completions.empty())
-    host_completions_map_.erase(it);
-
-  for (size_t j = 0; j < callbacks.size(); ++j)
-    callbacks[j]->Run(result);
-}
diff --git a/chrome/browser/net/chrome_cookie_policy.h b/chrome/browser/net/chrome_cookie_policy.h
deleted file mode 100644
index 0d536f8..0000000
--- a/chrome/browser/net/chrome_cookie_policy.h
+++ /dev/null
@@ -1,74 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef CHROME_BROWSER_NET_CHROME_COOKIE_POLICY_H_
-#define CHROME_BROWSER_NET_CHROME_COOKIE_POLICY_H_
-
-#include <map>
-#include <vector>
-
-#include "base/ref_counted.h"
-#include "net/base/cookie_policy.h"
-
-class HostContentSettingsMap;
-
-// Implements CookiePolicy that may delay queries to ask the user to decide.
-//
-// We will only prompt the user before setting a cookie.  We do not prompt the
-// user before getting a cookie.  However, if we are in the process of
-// prompting the user, then any requests to get cookies will be deferred.
-// This is done so that cookie requests remain FIFO.
-//
-class ChromeCookiePolicy
-    : public base::RefCountedThreadSafe<ChromeCookiePolicy>,
-      public net::CookiePolicy {
- public:
-  explicit ChromeCookiePolicy(HostContentSettingsMap* map);
-  ~ChromeCookiePolicy();
-
-  // CookiePolicy methods:
-  virtual int CanGetCookies(const GURL& url, const GURL& first_party,
-                            net::CompletionCallback* callback);
-  virtual int CanSetCookie(const GURL& url, const GURL& first_party,
-                           const std::string& cookie_line,
-                           net::CompletionCallback* callback);
-
- private:
-  class Completion {
-   public:
-    static Completion ForSetCookie(net::CompletionCallback* callback) {
-      return Completion(true, callback);
-    }
-
-    static Completion ForGetCookies(net::CompletionCallback* callback) {
-      return Completion(false, callback);
-    }
-
-    bool is_set_cookie_request() const { return is_set_cookie_request_; }
-    net::CompletionCallback* callback() const { return callback_; }
-
-   private:
-    Completion(bool is_set_cookie_request, net::CompletionCallback* callback)
-        : is_set_cookie_request_(is_set_cookie_request),
-          callback_(callback) {
-    }
-
-    bool is_set_cookie_request_;
-    net::CompletionCallback* callback_;
-  };
-
-  typedef std::vector<Completion> Completions;
-  typedef std::map<std::string, Completions> HostCompletionsMap;
-
-  void PromptForSetCookie(const std::string& host,
-                          const std::string& cookie_line);
-  void DidPromptForSetCookie(const std::string& host, int result);
-
-  // A map from hostname to callbacks awaiting a cookie policy response.
-  HostCompletionsMap host_completions_map_;
-
-  scoped_refptr<HostContentSettingsMap> host_content_settings_map_;
-};
-
-#endif  // CHROME_BROWSER_NET_CHROME_COOKIE_POLICY_H_
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 8013800..6b97030 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -184,9 +184,6 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
     context->set_cookie_store(new net::CookieMonster(cookie_db.get()));
   }
 
-  context->set_cookie_policy(
-      new ChromeCookiePolicy(host_content_settings_map_));
-
   // Create a new AppCacheService (issues fetches through the
   // main URLRequestContext that we just created).
   context->set_appcache_service(
@@ -265,8 +262,6 @@ ChromeURLRequestContext* FactoryForOffTheRecord::Create() {
       new net::HttpCache(context->host_resolver(), context->proxy_service(),
                          context->ssl_config_service(), 0);
   context->set_cookie_store(new net::CookieMonster);
-  context->set_cookie_policy(
-      new ChromeCookiePolicy(host_content_settings_map_));
   context->set_http_transaction_factory(cache);
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(
@@ -305,7 +300,6 @@ ChromeURLRequestContext* FactoryForOffTheRecordExtensions::Create() {
   const char* schemes[] = {chrome::kExtensionScheme};
   cookie_monster->SetCookieableSchemes(schemes, 1);
   context->set_cookie_store(cookie_monster);
-  // No dynamic cookie policy for extensions.
 
   return context;
 }
@@ -347,8 +341,6 @@ ChromeURLRequestContext* FactoryForMedia::Create() {
   context->set_proxy_service(main_context->proxy_service());
   // Also share the cookie store of the common profile.
   context->set_cookie_store(main_context->cookie_store());
-  context->set_cookie_policy(
-      static_cast<ChromeCookiePolicy*>(main_context->cookie_policy()));
 
   // Create a media cache with default size.
   // TODO(hclam): make the maximum size of media cache configurable.
@@ -614,6 +606,8 @@ void ChromeURLRequestContextGetter::GetCookieStoreAsyncHelper(
 ChromeURLRequestContext::ChromeURLRequestContext() {
   CheckCurrentlyOnIOThread();
 
+  cookie_policy_ = this;  // We implement CookiePolicy
+
   url_request_tracker()->SetGraveyardFilter(
       &ChromeURLRequestContext::ShouldTrackRequest);
 }
@@ -646,9 +640,6 @@ ChromeURLRequestContext::~ChromeURLRequestContext() {
   delete ftp_transaction_factory_;
   delete http_transaction_factory_;
 
-  // cookie_policy_'s lifetime is auto-managed by chrome_cookie_policy_.  We
-  // null this out here to avoid a dangling reference to chrome_cookie_policy_
-  // when ~URLRequestContext runs.
   cookie_policy_ = NULL;
 }
 
@@ -765,10 +756,48 @@ bool ChromeURLRequestContext::AreCookiesEnabled() const {
   return setting != CONTENT_SETTING_BLOCK;
 }
 
+bool ChromeURLRequestContext::CanGetCookies(const GURL& url,
+                                            const GURL& first_party) {
+  if (host_content_settings_map_->BlockThirdPartyCookies()) {
+    net::StaticCookiePolicy policy(
+        net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES);
+    if (!policy.CanGetCookies(url, first_party))
+      return false;
+  }
+
+  ContentSetting setting = host_content_settings_map_->GetContentSetting(
+      url.host(), CONTENT_SETTINGS_TYPE_COOKIES);
+  if (setting == CONTENT_SETTING_BLOCK)
+    return false;
+
+  // TODO(darin): Implement CONTENT_SETTING_ASK
+  return true;
+}
+
+bool ChromeURLRequestContext::CanSetCookie(const GURL& url,
+                                           const GURL& first_party) {
+  if (host_content_settings_map_->BlockThirdPartyCookies()) {
+    net::StaticCookiePolicy policy(
+        net::StaticCookiePolicy::BLOCK_THIRD_PARTY_COOKIES);
+    if (!policy.CanSetCookie(url, first_party))
+      return false;
+  }
+
+  ContentSetting setting = host_content_settings_map_->GetContentSetting(
+      url.host(), CONTENT_SETTINGS_TYPE_COOKIES);
+  if (setting == CONTENT_SETTING_BLOCK)
+    return false;
+
+  // TODO(darin): Implement CONTENT_SETTING_ASK
+  return true;
+}
+
 ChromeURLRequestContext::ChromeURLRequestContext(
     ChromeURLRequestContext* other) {
   CheckCurrentlyOnIOThread();
 
+  cookie_policy_ = this;  // We implement CookiePolicy
+
   // Set URLRequestContext members
   host_resolver_ = other->host_resolver_;
   proxy_service_ = other->proxy_service_;
@@ -776,7 +805,6 @@ ChromeURLRequestContext::ChromeURLRequestContext(
   http_transaction_factory_ = other->http_transaction_factory_;
   ftp_transaction_factory_ = other->ftp_transaction_factory_;
   cookie_store_ = other->cookie_store_;
-  cookie_policy_ = other->cookie_policy_;
   transport_security_state_ = other->transport_security_state_;
   accept_language_ = other->accept_language_;
   accept_charset_ = other->accept_charset_;
@@ -786,7 +814,6 @@ ChromeURLRequestContext::ChromeURLRequestContext(
   extension_info_ = other->extension_info_;
   user_script_dir_path_ = other->user_script_dir_path_;
   appcache_service_ = other->appcache_service_;
-  chrome_cookie_policy_ = other->chrome_cookie_policy_;
   host_content_settings_map_ = other->host_content_settings_map_;
   host_zoom_map_ = other->host_zoom_map_;
   privacy_blacklist_ = other->privacy_blacklist_;
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index f52029e..8d4186d 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -11,7 +11,6 @@
 #include "chrome/browser/host_content_settings_map.h"
 #include "chrome/browser/host_zoom_map.h"
 #include "chrome/browser/privacy_blacklist/blacklist.h"
-#include "chrome/browser/net/chrome_cookie_policy.h"
 #include "chrome/browser/net/url_request_context_getter.h"
 #include "chrome/common/appcache/chrome_appcache_service.h"
 #include "chrome/common/extensions/extension.h"
@@ -35,7 +34,8 @@ class IOThread;
 //
 // All methods of this class must be called from the IO thread,
 // including the constructor and destructor.
-class ChromeURLRequestContext : public URLRequestContext {
+class ChromeURLRequestContext : public URLRequestContext,
+                                public net::CookiePolicy {
  public:
   // Maintains some extension-related state we need on the IO thread.
   // TODO(aa): It would be cool if the Extension objects in ExtensionsService
@@ -121,6 +121,10 @@ class ChromeURLRequestContext : public URLRequestContext {
   // False only if cookies are globally blocked without exception.
   bool AreCookiesEnabled() const;
 
+  // CookiePolicy methods:
+  virtual bool CanGetCookies(const GURL& url, const GURL& first_party);
+  virtual bool CanSetCookie(const GURL& url, const GURL& first_party);
+
  protected:
   // Copies the dependencies from |other| into |this|. If you use this
   // constructor, then you should hold a reference to |other|, as we
@@ -163,10 +167,6 @@ class ChromeURLRequestContext : public URLRequestContext {
   void set_cookie_store(net::CookieStore* cookie_store) {
     cookie_store_ = cookie_store;
   }
-  void set_cookie_policy(ChromeCookiePolicy* cookie_policy) {
-    chrome_cookie_policy_ = cookie_policy;  // Take a strong reference.
-    cookie_policy_ = cookie_policy;
-  }
   void set_proxy_service(net::ProxyService* service) {
     proxy_service_ = service;
   }
@@ -206,7 +206,6 @@ class ChromeURLRequestContext : public URLRequestContext {
   FilePath user_script_dir_path_;
 
   scoped_refptr<ChromeAppCacheService> appcache_service_;
-  scoped_refptr<ChromeCookiePolicy> chrome_cookie_policy_;
   scoped_refptr<HostContentSettingsMap> host_content_settings_map_;
   scoped_refptr<HostZoomMap> host_zoom_map_;
   scoped_refptr<Blacklist> privacy_blacklist_;