Query the network delegate for cookies even if there is no cookie monster

net/url_request/url_request_http_job.cc

 // Copyright (c) 2012 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 "net/url_request/url_request_http_job.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 490 matching lines @@
   // No matter what, we want to report our status as IO pending since we will
   // be notifying our consumer asynchronously via OnStartCompleted.
   SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
 
   // If the request was destroyed, then there is no more work to do.
   if (!request_)
     return;
 
   CookieStore* cookie_store = request_->context()->cookie_store();
   if (cookie_store && !(request_info_.load_flags & LOAD_DO_NOT_SEND_COOKIES)) {
     net::CookieMonster* cookie_monster = cookie_store->GetCookieMonster();

[cbentzel, 2012/11/28 14:33:13]

Question more for Erik: I find all the GetCookieMonster calls to be a bit gross
(implicit down-cast). Is there a reason we can't just make
GetAllCookiesForURLAsync part of the CookieStore interface and get rid of the if
condition here?

[erikwright (departed), 2012/11/30 16:43:52]

GetAllCookiesForURLAsync is returning full metadata that is specific to the
cookie monster.

     if (cookie_monster) {
       cookie_monster->GetAllCookiesForURLAsync(
           request_->url(),
           base::Bind(&URLRequestHttpJob::CheckCookiePolicyAndLoad,
                      weak_factory_.GetWeakPtr()));
     } else {
-      DoLoadCookies();
+      CheckCookiePolicyAndLoad(CookieList());

[droger, 2012/11/23 16:00:12]

I pass an empty list as 'default' argument here, as it's not possible to compute
the actual list using only the CookieStore API.

Note: browsing the code, it seems that no implementation of the NetworkDelegate
actually use the |cookie_list| argument. If it's true, maybe we could remove it?
If we do this, we could even remove the call to
cookie_monster->GetAllCookiesForURLAsync() (line 513 above) altogether!

[erikwright (departed), 2012/11/27 05:17:17]

ChromeNetworkDelegate passes the list to TabSpecificContentSettings, which
stores them for reporting cookies that have been sent/blocked to the user.

[cbentzel, 2012/11/28 14:36:42]

Out of curiosity: is the Cookie header already supplied at this point for
non-CookieMonster based requests? If so, should this parse the results and pass
it in as a CookieList?

[droger, 2012/11/28 14:47:44]

Looking at the code, the headers are set a bit after, in OnCookiesLoaded, which
is triggered by DoLoadCookies().

I understand why it is desirable to unify the behavior, but also note that on
iOS I don't currently need the actual list of cookies, because the cookie
setting is "all or nothing". 

[erikwright (departed), 2012/11/30 16:43:52]

The problem is that the CookieList is cookies with all metadata, whereas the
cookie header is just the name-value pairs of cookies that matched the request.
It's pig vs. sausage.

     }
   } else {
     DoStartTransaction();
   }
 }
 
 void URLRequestHttpJob::DoLoadCookies() {
   CookieOptions options;
   options.set_include_httponly();
   request_->context()->cookie_store()->GetCookiesWithInfoAsync(
@@ skipping 1047 matching lines @@
 
 void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 }
 
 void URLRequestHttpJob::OnDetachRequest() {
   http_transaction_delegate_->OnDetachRequest();
 }
 
 }  // namespace net