Do cookie checks in NetworkDelegate instead of the URLRequest::Delegate.

chrome/browser/net/chrome_network_delegate.cc

Index: chrome/browser/net/chrome_network_delegate.cc
diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
index 595ad4b005ce278c89deea5cefeefd4b7206709f..09c5a3701c562817677ad15452d91148f295b5ee 100644
--- a/chrome/browser/net/chrome_network_delegate.cc
+++ b/chrome/browser/net/chrome_network_delegate.cc
@@ -5,6 +5,8 @@
 #include "chrome/browser/net/chrome_network_delegate.h"
 
 #include "base/logging.h"
+#include "chrome/browser/content_settings/cookie_settings.h"
+#include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/extensions/api/webrequest/webrequest_api.h"
 #include "chrome/browser/extensions/extension_event_router_forwarder.h"
@@ -14,6 +16,8 @@
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/common/content_url_request_user_data.h"
+#include "net/base/cookie_monster.h"

[willchan no longer on Chromium, 2012/03/01 20:30:10]

What's this for?

 #include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
@@ -52,14 +56,17 @@ ChromeNetworkDelegate::ChromeNetworkDelegate(
     ExtensionInfoMap* extension_info_map,
     const policy::URLBlacklistManager* url_blacklist_manager,
     void* profile,
+    CookieSettings* cookie_settings,
     BooleanPrefMember* enable_referrers)
     : event_router_(event_router),
       profile_(profile),
+      cookie_settings_(cookie_settings),
       extension_info_map_(extension_info_map),
       enable_referrers_(enable_referrers),
       url_blacklist_manager_(url_blacklist_manager) {
   DCHECK(event_router);
   DCHECK(enable_referrers);
+  DCHECK(!profile || cookie_settings);
 }
 
 ChromeNetworkDelegate::~ChromeNetworkDelegate() {}
@@ -183,3 +190,73 @@ ChromeNetworkDelegate::OnAuthRequired(
       profile_, extension_info_map_.get(), request, auth_info,
       callback, credentials);
 }
+
+bool ChromeNetworkDelegate::OnReadingCookies(
+    const net::URLRequest* request,
+    const net::CookieList& cookie_list) {
+  // NULL during tests, or when we're running in the system context.

[willchan no longer on Chromium, 2012/03/01 20:30:10]

I am not too fond of this. Does anyone else have an opinion here? I'd rather
have a SystemNetworkDelegate and a ChromeNetworkDelegate where they both compose
a shared object. It sort of annoys me that the system NetworkDelegate, which
should have no concept of a Profile, uses this class. I think long-term that the
behavioral difference between the system NetworkDelegate and the profile
specific NetworkDelegate will grow, and thus we shouldn't share
ChromeNetworkDelegate.

That said, this is the first major difference (since
ExtensionWebRequestEventRouter likewise assumes Profile*==NULL implies system
events), so I guess I'm fine with the current solution. But later on, I suspect
we'll need to refactor into two separate classes composing a shared
implementation class.

+  if (!cookie_settings_)
+    return true;
+
+  // If the request isn't annotated with information about the render view, it
+  // must not use cookies.
+  const content::ContentURLRequestUserData* user_data =
+      static_cast<const content::ContentURLRequestUserData*>(
+          request->GetUserData(
+              content::ContentURLRequestUserData::kUserDataKey));
+  if (!user_data)
+    return false;
+
+  // TODO(jochen): Once all URLFetcher consumers are updated, change this to
+  // return false, if no render view is given.
+  if (user_data->process_id() == -1 || user_data->routing_id() == -1)
+    return true;
+
+  bool allow = cookie_settings_->IsReadingCookieAllowed(
+      request->url(), request->first_party_for_cookies());
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&TabSpecificContentSettings::CookiesRead,
+                 user_data->process_id(), user_data->routing_id(),
+                 request->url(), cookie_list, !allow));
+
+  return allow;
+}
+
+bool ChromeNetworkDelegate::OnSettingCookie(
+    const net::URLRequest* request,
+    const std::string& cookie_line,
+    net::CookieOptions* options) {
+  // NULL during tests, or when we're running in the system context.
+  if (!cookie_settings_)
+    return true;
+
+  // If the request isn't annotated with content settings, it must not use
+  // cookies.
+  const content::ContentURLRequestUserData* user_data =
+      static_cast<const content::ContentURLRequestUserData*>(
+          request->GetUserData(
+              content::ContentURLRequestUserData::kUserDataKey));
+  if (!user_data)
+    return false;
+
+  // TODO(jochen): Once all URLFetcher consumers are updated, change this to
+  // return false, if no render view is given.
+  if (user_data->process_id() == -1 || user_data->routing_id() == -1)
+    return true;
+
+  bool allow = cookie_settings_->IsSettingCookieAllowed(
+      request->url(), request->first_party_for_cookies());
+
+  if (cookie_settings_->IsCookieSessionOnly(request->url()))
+    options->set_force_session();
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&TabSpecificContentSettings::CookieChanged,
+                 user_data->process_id(), user_data->routing_id(),
+                 request->url(), cookie_line, *options, !allow));
+
+  return allow;
+}