Teach URLRequest about initiator checks for First-Party-Only cookies.

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index bbfee654d48b8018a0b5e4a65e6a688db3d8cd93..12ce2d8a72e1722f68616a3d0148f21dc780cacf 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -659,13 +659,22 @@ void URLRequestHttpJob::DoLoadCookies() {
   CookieOptions options;
   options.set_include_httponly();
 
-  // TODO(mkwst): Drop this `if` once we decide whether or not to ship
-  // first-party cookies: https://crbug.com/459154
-  if (network_delegate() &&
-      network_delegate()->AreExperimentalCookieFeaturesEnabled())
-    options.set_first_party(url::Origin(request_->first_party_for_cookies()));
-  else
-    options.set_include_first_party_only();
+  url::Origin origin(request_->url());
+  if (origin.IsSameOriginWith(
+          url::Origin(request_->first_party_for_cookies())) &&
+      (request_->IsMethodSafe() ||
+       origin.IsSameOriginWith(request_->initiator()))) {
+    options.set_include_first_party_only_cookies();

[mmenke, 2015/10/22 19:57:21]

What about requests that don't set either of these fields on the request?  A
fair number of internal Chrome requests want cookies, I believe (Safebrowsing
certainly does).  This would effectively not allow first party only cookies for
those requests.

[Mike West, 2016/01/13 08:10:21]

If requests aren't being made in a web context, it's not clear what value
first-party-only would have. I think it's reasonable to lock out
first-party-only cookies in those contexts, honestly.

For SafeBrowsing in particular, there's actually a separate cookie jar entirely,
and the cookies are never exposed to the web (at least, not in the content
area). I don't think they'd see much value in FPO.

If they decide there's value in adding the flag, we can evaluate from what
context those requests should be made. Until they do so, I'd prefer to treat
those requests as blocking FPO cookies.

+  }
+
+  // TODO(mkwst): If first-party-only cookies aren't enabled, pretend the
+  // request is first-party regardless, in order to include all cookies. Drop
+  // this check once we decide whether or not we're shipping this feature:
+  // https://crbug.com/459154
+  if (!network_delegate() ||
+      !network_delegate()->AreExperimentalCookieFeaturesEnabled()) {
+    options.set_include_first_party_only_cookies();
+  }

[mmenke, 2015/10/22 19:57:21]

Think this is clearer as:

if (!network_delegate() || !...) {
  options.set_include_first_party_only_cookies();
} else if (origin.IsSameOriginWith...) {
  options.set_include_first_party_only_cookies();
}

Having this block just overwrite the work done by the previous block when
triggered seems a bit confusing to me.

Or could just swap the order of the if's, and start the second with a check that
include_first_party_only_cookies is not set, if you think that's clearer.

[Mike West, 2016/01/13 08:10:21]

My goal was to have something that I could just delete when we ship. I guess
putting it into an if/else block isn't any worse than the current state. Done in
the latest patchset.

 
   request_->context()->cookie_store()->GetCookiesWithOptionsAsync(
       request_->url(), options, base::Bind(&URLRequestHttpJob::OnCookiesLoaded,