More progress towards removing content settings code from the content layer. We can't use CookiePolicy anymore since, being in the network layer, we can't give it render_process_id/render_view_id, which are needed to put up the content settings UI. Instead of the networking code calling CookiePolicy then calling the delegate (ResourceDispatcherHost) to inform it if something is blocked, we directly ask the delegate if something is allowed. ResourceDispatcherHost then calls the embedder, and passes along the IDs to identify the tab. In the next change, I'll use this mechansim in RenderMessageFilter and remove the CookiePolicy class.

BUG=76793
Review URL: http://codereview.chromium.org/6995013

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

8 files changed, 112 insertions, 191 deletions

diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 6fe9bef..6dcd722 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -305,7 +305,7 @@ Time CanonExpiration(const CookieMonster::ParsedCookie& pc,
   Time expiration_time = CanonExpirationInternal(pc, current);
 
   if (options.force_session()) {
-    // Only override the expiry  adte if it's in the future. If the expiry date
+    // Only override the expiry date if it's in the future. If the expiry date
     // is before the creation date, the cookie is supposed to be deleted.
     if (expiration_time.is_null() || expiration_time > current)
       return Time();
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index cb6a4ed..3c805d7 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -100,14 +100,14 @@ void URLRequest::Delegate::OnSSLCertificateError(URLRequest* request,
   request->Cancel();
 }
 
-void URLRequest::Delegate::OnGetCookies(URLRequest* request,
-                                        bool blocked_by_policy) {
+bool URLRequest::Delegate::CanGetCookies(URLRequest* request) {
+  return true;
 }
 
-void URLRequest::Delegate::OnSetCookie(URLRequest* request,
-                                       const std::string& cookie_line,
-                                       const CookieOptions& options,
-                                       bool blocked_by_policy) {
+bool URLRequest::Delegate::CanSetCookie(URLRequest* request,
+                                        const std::string& cookie_line,
+                                        CookieOptions* options) {
+  return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index 80f1edd..c43aa45 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -188,18 +188,17 @@ class URLRequest : public base::NonThreadSafe {
                                        int cert_error,
                                        X509Certificate* cert);
 
-    // Called when reading cookies. |blocked_by_policy| is true if access to
-    // cookies was denied due to content settings. This method will never be
-    // invoked when LOAD_DO_NOT_SEND_COOKIES is specified.
-    virtual void OnGetCookies(URLRequest* request, bool blocked_by_policy);
-
-    // Called when a cookie is set. |blocked_by_policy| is true if the cookie
-    // was rejected due to content settings. This method will never be invoked
-    // when LOAD_DO_NOT_SAVE_COOKIES is specified.
-    virtual void OnSetCookie(URLRequest* request,
-                             const std::string& cookie_line,
-                             const CookieOptions& options,
-                             bool blocked_by_policy);
+    // Called when reading cookies to allow the delegate to block access to the
+    // cookie. This method will never be invoked when LOAD_DO_NOT_SEND_COOKIES
+    // is specified.
+    virtual bool CanGetCookies(URLRequest* request);
+
+    // Called when a cookie is set to allow the delegate to block access to the
+    // cookie. This method will never be invoked when LOAD_DO_NOT_SAVE_COOKIES
+    // is specified.
+    virtual bool CanSetCookie(URLRequest* request,
+                              const std::string& cookie_line,
+                              CookieOptions* options);
 
     // After calling Start(), the delegate will receive an OnResponseStarted
     // callback when the request has completed.  If an error occurred, the
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 8504260..e6b07e8c 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -16,7 +16,6 @@
 #include "base/string_util.h"
 #include "base/time.h"
 #include "net/base/cert_status_flags.h"
-#include "net/base/cookie_policy.h"
 #include "net/base/cookie_store.h"
 #include "net/base/filter.h"
 #include "net/base/host_port_pair.h"
@@ -427,17 +426,18 @@ void URLRequestHttpJob::AddCookieHeaderAndStart() {
   // be notifying our consumer asynchronously via OnStartCompleted.
   SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
 
-  int policy = OK;
+  // If the request was destroyed, then there is no more work to do.
+  if (!request_)
+    return;
 
-  if (request_info_.load_flags & LOAD_DO_NOT_SEND_COOKIES) {
-    policy = ERR_FAILED;
-  } else if (request_->context()->cookie_policy()) {
-    policy = request_->context()->cookie_policy()->CanGetCookies(
-        request_->url(),
-        request_->first_party_for_cookies());
+  bool allow = true;
+  if (request_info_.load_flags & LOAD_DO_NOT_SEND_COOKIES ||
+      (request_->delegate() &&
+       !request_->delegate()->CanGetCookies(request_))) {
+    allow = false;
   }
 
-  OnCanGetCookiesCompleted(policy);
+  OnCanGetCookiesCompleted(allow);
 }
 
 void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete() {
@@ -468,18 +468,23 @@ void URLRequestHttpJob::SaveNextCookie() {
   // be notifying our consumer asynchronously via OnStartCompleted.
   SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
 
-  int policy = OK;
-
+  bool allow = true;
+  CookieOptions options;
   if (request_info_.load_flags & LOAD_DO_NOT_SAVE_COOKIES) {
-    policy = ERR_FAILED;
-  } else if (request_->context()->cookie_policy()) {
-    policy = request_->context()->cookie_policy()->CanSetCookie(
-        request_->url(),
-        request_->first_party_for_cookies(),
-        response_cookies_[response_cookies_save_index_]);
+    allow = false;
+  } else if (request_->delegate() && request_->context()->cookie_store()) {
+    CookieOptions options;
+    options.set_include_httponly();
+    if (request_->delegate()->CanSetCookie(
+            request_,
+            response_cookies_[response_cookies_save_index_], &options)) {
+      request_->context()->cookie_store()->SetCookieWithOptions(
+          request_->url(), response_cookies_[response_cookies_save_index_],
+          options);
+    }
   }
 
-  OnCanSetCookieCompleted(policy);
+  OnCanSetCookieCompleted();
 }
 
 void URLRequestHttpJob::FetchResponseCookies(
@@ -583,69 +588,33 @@ void URLRequestHttpJob::ProcessStrictTransportSecurityHeader() {
   }
 }
 
-void URLRequestHttpJob::OnCanGetCookiesCompleted(int policy) {
-  // If the request was destroyed, then there is no more work to do.
-  if (request_ && request_->delegate()) {
-    if (request_->context()->cookie_store()) {
-      if (policy == ERR_ACCESS_DENIED) {
-        request_->delegate()->OnGetCookies(request_, true);
-      } else if (policy == OK) {
-        request_->delegate()->OnGetCookies(request_, false);
-        CookieOptions options;
-        options.set_include_httponly();
-        std::string cookies =
-            request_->context()->cookie_store()->GetCookiesWithOptions(
-                request_->url(), options);
-        if (!cookies.empty()) {
-          request_info_.extra_headers.SetHeader(
-              HttpRequestHeaders::kCookie, cookies);
-        }
-      }
-    }
-    // We may have been canceled within OnGetCookies.
-    if (GetStatus().is_success()) {
-      StartTransaction();
-    } else {
-      NotifyCanceled();
+void URLRequestHttpJob::OnCanGetCookiesCompleted(bool allow) {
+  if (request_->context()->cookie_store() && allow) {
+    CookieOptions options;
+    options.set_include_httponly();
+    std::string cookies =
+        request_->context()->cookie_store()->GetCookiesWithOptions(
+            request_->url(), options);
+    if (!cookies.empty()) {
+      request_info_.extra_headers.SetHeader(
+          HttpRequestHeaders::kCookie, cookies);
     }
   }
+  // We may have been canceled within CanGetCookies.
+  if (GetStatus().is_success()) {
+    StartTransaction();
+  } else {
+    NotifyCanceled();
+  }
 }
 
-void URLRequestHttpJob::OnCanSetCookieCompleted(int policy) {
-  // If the request was destroyed, then there is no more work to do.
-  if (request_ && request_->delegate()) {
-    if (request_->context()->cookie_store()) {
-      if (policy == ERR_ACCESS_DENIED) {
-        CookieOptions options;
-        options.set_include_httponly();
-        request_->delegate()->OnSetCookie(
-            request_,
-            response_cookies_[response_cookies_save_index_],
-            options,
-            true);
-      } else if (policy == OK || policy == OK_FOR_SESSION_ONLY) {
-        // OK to save the current response cookie now.
-        CookieOptions options;
-        options.set_include_httponly();
-        if (policy == OK_FOR_SESSION_ONLY)
-          options.set_force_session();
-        request_->context()->cookie_store()->SetCookieWithOptions(
-            request_->url(), response_cookies_[response_cookies_save_index_],
-            options);
-        request_->delegate()->OnSetCookie(
-            request_,
-            response_cookies_[response_cookies_save_index_],
-            options,
-            false);
-      }
-    }
-    response_cookies_save_index_++;
-    // We may have been canceled within OnSetCookie.
-    if (GetStatus().is_success()) {
-      SaveNextCookie();
-    } else {
-      NotifyCanceled();
-    }
+void URLRequestHttpJob::OnCanSetCookieCompleted() {
+  response_cookies_save_index_++;
+  // We may have been canceled within OnSetCookie.
+  if (GetStatus().is_success()) {
+    SaveNextCookie();
+  } else {
+    NotifyCanceled();
   }
 }
 
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index 7ac5c97..960db8e 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -53,8 +53,8 @@ class URLRequestHttpJob : public URLRequestJob {
   // Process the Strict-Transport-Security header, if one exists.
   void ProcessStrictTransportSecurityHeader();
 
-  void OnCanGetCookiesCompleted(int result);
-  void OnCanSetCookieCompleted(int result);
+  void OnCanGetCookiesCompleted(bool allow);
+  void OnCanSetCookieCompleted();
   void OnStartCompleted(int result);
   void OnReadCompleted(int result);
 
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index f8ab98a..4f4fa65 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -11,32 +11,6 @@
 #include "net/base/host_port_pair.h"
 #include "net/http/http_network_session.h"
 
-TestCookiePolicy::TestCookiePolicy(int options_bit_mask)
-    : options_(options_bit_mask) {
-}
-
-TestCookiePolicy::~TestCookiePolicy() {}
-
-int TestCookiePolicy::CanGetCookies(const GURL& url,
-                                    const GURL& first_party) const {
-  if (options_ & NO_GET_COOKIES)
-    return net::ERR_ACCESS_DENIED;
-
-  return net::OK;
-}
-
-int TestCookiePolicy::CanSetCookie(const GURL& url,
-                                   const GURL& first_party,
-                                   const std::string& cookie_line) const {
-  if (options_ & NO_SET_COOKIE)
-    return net::ERR_ACCESS_DENIED;
-
-  if (options_ & FORCE_SESSION)
-    return net::OK_FOR_SESSION_ONLY;
-
-  return net::OK;
-}
-
 TestURLRequestContext::TestURLRequestContext()
     : ALLOW_THIS_IN_INITIALIZER_LIST(context_storage_(this)) {
   context_storage_.set_host_resolver(
@@ -113,6 +87,7 @@ TestDelegate::TestDelegate()
       quit_on_complete_(true),
       quit_on_redirect_(false),
       allow_certificate_errors_(false),
+      cookie_options_bit_mask_(0),
       response_started_count_(0),
       received_bytes_count_(0),
       received_redirect_count_(0),
@@ -161,26 +136,40 @@ void TestDelegate::OnSSLCertificateError(net::URLRequest* request,
     request->Cancel();
 }
 
-void TestDelegate::OnGetCookies(net::URLRequest* request,
-                                bool blocked_by_policy) {
-  if (blocked_by_policy) {
+bool TestDelegate::CanGetCookies(net::URLRequest* request) {
+  bool allow = true;
+  if (cookie_options_bit_mask_ & NO_GET_COOKIES)
+    allow = false;
+
+  if (!allow) {
     blocked_get_cookies_count_++;
     if (cancel_in_getcookiesblocked_)
       request->Cancel();
   }
+
+  return allow;
 }
 
-void TestDelegate::OnSetCookie(net::URLRequest* request,
-                               const std::string& cookie_line,
-                               const net::CookieOptions& options,
-                               bool blocked_by_policy) {
-  if (blocked_by_policy) {
+bool TestDelegate::CanSetCookie(net::URLRequest* request,
+                                const std::string& cookie_line,
+                                net::CookieOptions* options) {
+  bool allow = true;
+  if (cookie_options_bit_mask_ & NO_SET_COOKIE)
+    allow = false;
+
+  if (cookie_options_bit_mask_ & FORCE_SESSION)
+    options->set_force_session();
+
+
+  if (!allow) {
     blocked_set_cookie_count_++;
     if (cancel_in_setcookieblocked_)
       request->Cancel();
   } else {
     set_cookie_count_++;
   }
+
+  return allow;
 }
 
 void TestDelegate::OnResponseStarted(net::URLRequest* request) {
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index f312da6..777d060 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -19,7 +19,6 @@
 #include "base/utf_string_conversions.h"
 #include "net/base/cert_verifier.h"
 #include "net/base/cookie_monster.h"
-#include "net/base/cookie_policy.h"
 #include "net/base/host_resolver.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
@@ -45,29 +44,6 @@ class HostPortPair;
 
 //-----------------------------------------------------------------------------
 
-class TestCookiePolicy : public net::CookiePolicy {
- public:
-  enum Options {
-    NO_GET_COOKIES = 1 << 0,
-    NO_SET_COOKIE  = 1 << 1,
-    FORCE_SESSION  = 1 << 2,
-  };
-
-  explicit TestCookiePolicy(int options_bit_mask);
-  virtual ~TestCookiePolicy();
-
-  // net::CookiePolicy:
-  virtual int CanGetCookies(const GURL& url, const GURL& first_party) const;
-  virtual int CanSetCookie(const GURL& url,
-                           const GURL& first_party,
-                           const std::string& cookie_line) const;
-
- private:
-  int options_;
-};
-
-//-----------------------------------------------------------------------------
-
 class TestURLRequestContext : public net::URLRequestContext {
  public:
   TestURLRequestContext();
@@ -96,6 +72,12 @@ class TestURLRequest : public net::URLRequest {
 
 class TestDelegate : public net::URLRequest::Delegate {
  public:
+  enum Options {
+    NO_GET_COOKIES = 1 << 0,
+    NO_SET_COOKIE  = 1 << 1,
+    FORCE_SESSION  = 1 << 2,
+  };
+
   TestDelegate();
   virtual ~TestDelegate();
 
@@ -116,6 +98,7 @@ class TestDelegate : public net::URLRequest::Delegate {
   void set_allow_certificate_errors(bool val) {
     allow_certificate_errors_ = val;
   }
+  void set_cookie_options(int o) {cookie_options_bit_mask_ = o; }
   void set_username(const string16& u) { username_ = u; }
   void set_password(const string16& p) { password_ = p; }
 
@@ -135,19 +118,19 @@ class TestDelegate : public net::URLRequest::Delegate {
 
   // net::URLRequest::Delegate:
   virtual void OnReceivedRedirect(net::URLRequest* request, const GURL& new_url,
-                                  bool* defer_redirect);
+                                  bool* defer_redirect) OVERRIDE;
   virtual void OnAuthRequired(net::URLRequest* request,
-                              net::AuthChallengeInfo* auth_info);
+                              net::AuthChallengeInfo* auth_info) OVERRIDE;
   virtual void OnSSLCertificateError(net::URLRequest* request,
                                      int cert_error,
-                                     net::X509Certificate* cert);
-  virtual void OnGetCookies(net::URLRequest* request, bool blocked_by_policy);
-  virtual void OnSetCookie(net::URLRequest* request,
-                           const std::string& cookie_line,
-                           const net::CookieOptions& options,
-                           bool blocked_by_policy);
-  virtual void OnResponseStarted(net::URLRequest* request);
-  virtual void OnReadCompleted(net::URLRequest* request, int bytes_read);
+                                     net::X509Certificate* cert) OVERRIDE;
+  virtual bool CanGetCookies(net::URLRequest* request) OVERRIDE;
+  virtual bool CanSetCookie(net::URLRequest* request,
+                            const std::string& cookie_line,
+                            net::CookieOptions* options) OVERRIDE;
+  virtual void OnResponseStarted(net::URLRequest* request) OVERRIDE;
+  virtual void OnReadCompleted(net::URLRequest* request,
+                               int bytes_read) OVERRIDE;
 
  private:
   static const int kBufferSize = 4096;
@@ -164,6 +147,7 @@ class TestDelegate : public net::URLRequest::Delegate {
   bool quit_on_complete_;
   bool quit_on_redirect_;
   bool allow_certificate_errors_;
+  int cookie_options_bit_mask_;
 
   string16 username_;
   string16 password_;
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 1c0987f..ba6de05 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1655,10 +1655,8 @@ TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy) {
 
   // Verify that the cookie isn't sent.
   {
-    TestCookiePolicy cookie_policy(TestCookiePolicy::NO_GET_COOKIES);
-    context->set_cookie_policy(&cookie_policy);
-
     TestDelegate d;
+    d.set_cookie_options(TestDelegate::NO_GET_COOKIES);
     TestURLRequest req(test_server.GetURL("echoheader?Cookie"), &d);
     req.set_context(context);
     req.Start();
@@ -1667,8 +1665,6 @@ TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy) {
     EXPECT_TRUE(d.data_received().find("Cookie: CookieToNotSend=1")
                 == std::string::npos);
 
-    context->set_cookie_policy(NULL);
-
     EXPECT_EQ(1, d.blocked_get_cookies_count());
     EXPECT_EQ(0, d.blocked_set_cookie_count());
   }
@@ -1695,10 +1691,8 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy) {
 
   // Try to set-up another cookie and update the previous cookie.
   {
-    TestCookiePolicy cookie_policy(TestCookiePolicy::NO_SET_COOKIE);
-    context->set_cookie_policy(&cookie_policy);
-
     TestDelegate d;
+    d.set_cookie_options(TestDelegate::NO_SET_COOKIE);
     URLRequest req(test_server.GetURL(
         "set-cookie?CookieToNotSave=1&CookieToNotUpdate=1"), &d);
     req.set_context(context);
@@ -1706,8 +1700,6 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy) {
 
     MessageLoop::current()->Run();
 
-    context->set_cookie_policy(NULL);
-
     EXPECT_EQ(0, d.blocked_get_cookies_count());
     EXPECT_EQ(2, d.blocked_set_cookie_count());
   }
@@ -1786,10 +1778,8 @@ TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy_Async) {
 
   // Verify that the cookie isn't sent.
   {
-    TestCookiePolicy cookie_policy(TestCookiePolicy::NO_GET_COOKIES);
-    context->set_cookie_policy(&cookie_policy);
-
     TestDelegate d;
+    d.set_cookie_options(TestDelegate::NO_GET_COOKIES);
     TestURLRequest req(test_server.GetURL("echoheader?Cookie"), &d);
     req.set_context(context);
     req.Start();
@@ -1798,8 +1788,6 @@ TEST_F(URLRequestTest, DoNotSendCookies_ViaPolicy_Async) {
     EXPECT_TRUE(d.data_received().find("Cookie: CookieToNotSend=1")
                 == std::string::npos);
 
-    context->set_cookie_policy(NULL);
-
     EXPECT_EQ(1, d.blocked_get_cookies_count());
     EXPECT_EQ(0, d.blocked_set_cookie_count());
   }
@@ -1826,10 +1814,8 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy_Async) {
 
   // Try to set-up another cookie and update the previous cookie.
   {
-    TestCookiePolicy cookie_policy(TestCookiePolicy::NO_SET_COOKIE);
-    context->set_cookie_policy(&cookie_policy);
-
     TestDelegate d;
+    d.set_cookie_options(TestDelegate::NO_SET_COOKIE);
     URLRequest req(test_server.GetURL(
         "set-cookie?CookieToNotSave=1&CookieToNotUpdate=1"), &d);
     req.set_context(context);
@@ -1837,8 +1823,6 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy_Async) {
 
     MessageLoop::current()->Run();
 
-    context->set_cookie_policy(NULL);
-
     EXPECT_EQ(0, d.blocked_get_cookies_count());
     EXPECT_EQ(2, d.blocked_set_cookie_count());
   }
@@ -1867,12 +1851,10 @@ TEST_F(URLRequestTest, CookiePolicy_ForceSession) {
 
   scoped_refptr<TestURLRequestContext> context(new TestURLRequestContext());
 
-  TestCookiePolicy cookie_policy(TestCookiePolicy::FORCE_SESSION);
-  context->set_cookie_policy(&cookie_policy);
-
   // Set up a cookie.
   {
     TestDelegate d;
+    d.set_cookie_options(TestDelegate::FORCE_SESSION);
     URLRequest req(test_server.GetURL(
         "set-cookie?A=1;expires=\"Fri, 05 Feb 2010 23:42:01 GMT\""), &d);
     req.set_context(context);
@@ -1889,8 +1871,6 @@ TEST_F(URLRequestTest, CookiePolicy_ForceSession) {
       context->cookie_store()->GetCookieMonster()->GetAllCookies();
   EXPECT_EQ(1U, cookies.size());
   EXPECT_FALSE(cookies[0].IsPersistent());
-
-  context->set_cookie_policy(NULL);
 }
 
 // In this test, we do a POST which the server will 302 redirect.