Add a callback for setting cookies in URLRequestHttpJob

Eventually that callback will have a call to send the blocked cookies to
DevTools. For now, it just calls NotifiyHeadersComplete on the last
call.

I've also created a couple new private variables, including
|cs_status_list_| so we can actually use the cookie strings
and list when we want to without too much fuss.

Bug: 856777
Change-Id: Ic71b1523ea244e4fbfd9cb16c07f716ba14448bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1479299
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Aaron Tagliaboschi <aarontag@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638590}

net/cookies/canonical_cookie.cc

@@ -44,6 +44,8 @@
 
 #include "net/cookies/canonical_cookie.h"
 
+#include <utility>
+
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ -552,4 +554,9 @@ std::string CanonicalCookie::DomainWithoutDot() const {
   return domain_.substr(1);
 }
 
+CookieLineWithStatus::CookieLineWithStatus(
+    std::string cookie_string,
+    CanonicalCookie::CookieInclusionStatus status)
+    : cookie_string(std::move(cookie_string)), status(status) {}
+
 }  // namespace net

net/cookies/canonical_cookie.h

@@ -66,7 +66,8 @@ class NET_EXPORT CanonicalCookie {
     EXCLUDE_OVERWRITE_SECURE,
     EXCLUDE_OVERWRITE_HTTP_ONLY,
     EXCLUDE_INVALID_DOMAIN,
-    EXCLUDE_INVALID_PREFIX
+    EXCLUDE_INVALID_PREFIX,
+    EXCLUDE_THIRD_PARTY_POLICY
   };
 
   // Creates a new |CanonicalCookie| from the |cookie_line| and the
@@ -265,6 +266,15 @@ struct CookieWithStatus {
   CanonicalCookie::CookieInclusionStatus status;
 };
 
+// Just used for the next function to send the cookie string with it's status
+struct CookieLineWithStatus {
+  CookieLineWithStatus(std::string cookie_string,
+                       CanonicalCookie::CookieInclusionStatus status);
+
+  std::string cookie_string;
+  CanonicalCookie::CookieInclusionStatus status;
+};
+
 typedef std::vector<CanonicalCookie> CookieList;
 
 typedef std::vector<CookieWithStatus> CookieStatusList;

net/test/embedded_test_server/default_handlers.cc

@@ -376,6 +376,9 @@ std::unique_ptr<HttpResponse> HandleAuthBasic(const HttpRequest& request) {
     return std::move(http_response);
   }
 
+  if (query.find("set-cookie-if-not-challenged") != query.end())
+    http_response->AddCustomHeader("Set-Cookie", "got_challenged=true");
+
   if (request.headers.find("If-None-Match") != request.headers.end() &&
       request.headers.at("If-None-Match") == kEtag) {
     http_response->set_code(HTTP_NOT_MODIFIED);

net/url_request/url_request_http_job.cc

@@ -363,6 +363,7 @@ URLRequestHttpJob::URLRequestHttpJob(
     NetworkDelegate* network_delegate,
     const HttpUserAgentSettings* http_user_agent_settings)
     : URLRequestJob(request, network_delegate),
+      num_cookie_lines_left_(0),
       priority_(DEFAULT_PRIORITY),
       response_info_(nullptr),
       proxy_auth_state_(AUTH_STATE_DONT_NEED_AUTH),
@@ -474,6 +475,7 @@ void URLRequestHttpJob::NotifyBeforeSendHeadersCallback(
 
 void URLRequestHttpJob::NotifyHeadersComplete() {
   DCHECK(!response_info_);
+  DCHECK_EQ(0, num_cookie_lines_left_);
 
   response_info_ = transaction_->GetResponseInfo();
 
@@ -488,6 +490,10 @@ void URLRequestHttpJob::NotifyHeadersComplete() {
   ProcessStrictTransportSecurityHeader();
   ProcessExpectCTHeader();
 
+  // Clear |cs_status_list_| after any processing in case
+  // SaveCookiesAndNotifyHeadersComplete is called again.
+  cs_status_list_.clear();
+
   // The HTTP transaction may be restarted several times for the purposes
   // of sending authorization information. Each time it restarts, we get
   // notified of the headers completion so that we can update the cookie store.
@@ -723,6 +729,9 @@ void URLRequestHttpJob::SetCookieHeaderAndStart(
 }
 
 void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete(int result) {
+  DCHECK(cs_status_list_.empty());
+  DCHECK_EQ(0, num_cookie_lines_left_);
+
   // End of the call started in OnStartCompleted.
   OnCallToDelegateComplete();
 
@@ -734,34 +743,82 @@ void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete(int result) {
     return;
   }
 
+  if ((request_info_.load_flags & LOAD_DO_NOT_SAVE_COOKIES) ||
+      !request_->context()->cookie_store()) {
+    NotifyHeadersComplete();
+    return;
+  }
+
   base::Time response_date;
   if (!GetResponseHeaders()->GetDateValue(&response_date))
     response_date = base::Time();
 
-  if (!(request_info_.load_flags & LOAD_DO_NOT_SAVE_COOKIES) &&
-      request_->context()->cookie_store()) {
-    CookieOptions options;
-    options.set_include_httponly();
-    options.set_server_time(response_date);
-
-    // Set all cookies, without waiting for them to be set. Any subsequent read
-    // will see the combined result of all cookie operation.
-    const base::StringPiece name("Set-Cookie");
-    std::string cookie_line;
-    size_t iter = 0;
-    HttpResponseHeaders* headers = GetResponseHeaders();
-    while (headers->EnumerateHeader(&iter, name, &cookie_line)) {
-      std::unique_ptr<CanonicalCookie> cookie = net::CanonicalCookie::Create(
-          request_->url(), cookie_line, base::Time::Now(), options);
-      if (!cookie || !CanSetCookie(*cookie, &options))
-        continue;
-      request_->context()->cookie_store()->SetCookieWithOptionsAsync(
-          request_->url(), cookie_line, options,
-          CookieStore::SetCookiesCallback());
+  CookieOptions options;
+  options.set_include_httponly();
+  options.set_server_time(response_date);
+
+  // Set all cookies, without waiting for them to be set. Any subsequent read
+  // will see the combined result of all cookie operation.
+  const base::StringPiece name("Set-Cookie");
+  std::string cookie_string;
+  size_t iter = 0;
+  HttpResponseHeaders* headers = GetResponseHeaders();
+
+  // NotifyHeadersComplete needs to be called once and only once after the
+  // list has been fully processed, and it can either be called in the
+  // callback or after the loop is called, depending on how the last element
+  // was handled. |num_cookie_lines_left_| keeps track of how many async
+  // callbacks are currently out (starting from 1 to make sure the loop runs all
+  // the way through before trying to exit). If there are any callbacks still
+  // waiting when the loop ends, then NotifyHeadersComplete will be called when
+  // it reaches 0 in the callback itself.
+  num_cookie_lines_left_ = 1;
+  while (headers->EnumerateHeader(&iter, name, &cookie_string)) {
+    CanonicalCookie::CookieInclusionStatus returned_status;
+
+    num_cookie_lines_left_++;
+
+    std::unique_ptr<CanonicalCookie> cookie = net::CanonicalCookie::Create(
+        request_->url(), cookie_string, base::Time::Now(), options,
+        &returned_status);
+
+    if (returned_status != CanonicalCookie::CookieInclusionStatus::INCLUDE) {
+      OnSetCookieResult(std::move(cookie_string), returned_status);
+      continue;
     }
+
+    if (!CanSetCookie(*cookie, &options)) {
+      OnSetCookieResult(
+          std::move(cookie_string),
+          CanonicalCookie::CookieInclusionStatus::EXCLUDE_THIRD_PARTY_POLICY);
+      continue;
+    }
+
+    request_->context()->cookie_store()->SetCookieWithOptionsAsync(
+        request_->url(), cookie_string, options,
+        base::BindOnce(&URLRequestHttpJob::OnSetCookieResult,
+                       weak_factory_.GetWeakPtr(), cookie_string));
   }
+  // Removing the 1 that |num_cookie_lines_left| started with, signifing that
+  // loop has been exited.
+  num_cookie_lines_left_--;
+
+  if (num_cookie_lines_left_ == 0)
+    NotifyHeadersComplete();
+}
+
+void URLRequestHttpJob::OnSetCookieResult(
+    std::string cookie_string,
+    CanonicalCookie::CookieInclusionStatus status) {
+  if (status != CanonicalCookie::CookieInclusionStatus::INCLUDE)
+    cs_status_list_.emplace_back(std::move(cookie_string), status);
+
+  num_cookie_lines_left_--;
 
-  NotifyHeadersComplete();
+  // If all the cookie lines have been handled, |cs_status_list_| now reflects
+  // the result of all Set-Cookie lines, and the request can be continued.
+  if (num_cookie_lines_left_ == 0)
+    NotifyHeadersComplete();
 }
 
 void URLRequestHttpJob::ProcessStrictTransportSecurityHeader() {

net/url_request/url_request_http_job.h

@@ -10,6 +10,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -152,6 +153,12 @@ class NET_EXPORT_PRIVATE URLRequestHttpJob : public URLRequestJob {
   void SetCookieHeaderAndStart(const CookieList& cookie_list,
                                const CookieStatusList& excluded_list);
 
+  // Another Cookie Monster callback
+  void OnSetCookieResult(std::string cookie_string,
+                         CanonicalCookie::CookieInclusionStatus status);
+  int num_cookie_lines_left_;
+  std::vector<CookieLineWithStatus> cs_status_list_;
+
   // Some servers send the body compressed, but specify the content length as
   // the uncompressed size. If this is the case, we return true in order
   // to request to work around this non-adherence to the HTTP standard.