Remove dup'ed code for RequestorOrigin and FirstPartyCookie

third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 278 matching lines @@
     ResourceLoadPriority load_priority,
     int intra_priority_value) {
   TRACE_EVENT1(
       "devtools.timeline", "ResourceChangePriority", "data",
       InspectorChangeResourcePriorityEvent::Data(identifier, load_priority));
   probe::didChangeResourcePriority(GetFrame(), identifier, load_priority);
 }
 
 void FrameFetchContext::PrepareRequest(ResourceRequest& request,
                                        RedirectType redirect_type) {
+  SetFirstPartyCookieAndRequestorOrigin(request);

[kinuko, 2017/06/07 02:00:49]

Moved here so this is also called in redirects (so that it's called at the same
timing as RenderFrameImpl::WillSendRequest)

+
   GetFrame()->Loader().ApplyUserAgent(request);
   GetLocalFrameClient()->DispatchWillSendRequest(request);
 
   // ServiceWorker hook ups.
   if (MasterDocumentLoader()->GetServiceWorkerNetworkProvider()) {
     WrappedResourceRequest webreq(request);
     MasterDocumentLoader()->GetServiceWorkerNetworkProvider()->WillSendRequest(
         webreq);
   }
 
@@ skipping 371 matching lines @@
 }
 
 void FrameFetchContext::PopulateResourceRequest(
     const KURL& url,
     Resource::Type type,
     const ClientHintsPreferences& hints_preferences,
     const FetchParameters::ResourceWidth& resource_width,
     const ResourceLoaderOptions& options,
     SecurityViolationReportingPolicy reporting_policy,
     ResourceRequest& request) {
-  SetFirstPartyCookieAndRequestorOrigin(request);
-
   // Before modifying the request for CSP, evaluate report-only headers. This
   // allows site owners to learn about requests that are being modified
   // (e.g. mixed content that is being upgraded by upgrade-insecure-requests).
   CheckCSPForRequest(request, url, options, reporting_policy,
                      request.GetRedirectStatus(),
                      ContentSecurityPolicy::CheckHeaderType::kCheckReportOnly);
 
   ModifyRequestForCSP(request);
   AddClientHintsIfNecessary(hints_preferences, resource_width, request);
   AddCSPHeaderIfNecessary(type, request);
 }
 
 void FrameFetchContext::SetFirstPartyCookieAndRequestorOrigin(

[kinuko, 2017/06/07 02:00:49]

Now this does mostly same as what RenderFrameImpl::WillSendRequest +
FrameFetchContext were doing (I tried to preserve the original behavior but has
slight changes)

     ResourceRequest& request) {
-  if (!GetDocument())
-    return;
-
+  // Set the first party for cookies url if it has not been set yet (new
+  // requests). This value will be updated during redirects, consistent with
+  // https://tools.ietf.org/html/draft-west-first-party-cookies-04#section-2.1.1

[Mike West, 2017/06/07 06:48:44]

Would you mind updating this to
https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00#section-2....

[kinuko, 2017/06/07 06:57:49]

Done.

   if (request.FirstPartyForCookies().IsNull()) {
-    request.SetFirstPartyForCookies(
-        GetDocument() ? GetDocument()->FirstPartyForCookies()
-                      : SecurityOrigin::UrlWithUniqueSecurityOrigin());
+    if (request.GetFrameType() == WebURLRequest::kFrameTypeTopLevel) {
+      request.SetFirstPartyForCookies(request.Url());
+    } else {
+      // Use GetDocument() for subresource or nested frame cases,
+      // GetFrame()->GetDocument() otherwise.
+      Document* document =
+          GetDocument() ? GetDocument() : GetFrame()->GetDocument();
+      request.SetFirstPartyForCookies(document->FirstPartyForCookies());
+    }
   }
 
-  // Subresource requests inherit their requestor origin from |m_document|
-  // directly. Top-level and nested frame types are taken care of in
-  // 'FrameLoadRequest()'. Auxiliary frame types in 'createWindow()' and
-  // 'FrameLoader::load'.
-  // TODO(mkwst): It would be cleaner to adjust blink::ResourceRequest to
-  // initialize itself with a `nullptr` initiator so that this can be a simple
-  // `isNull()` check. https://crbug.com/625969
-  if (request.GetFrameType() == WebURLRequest::kFrameTypeNone &&
-      request.RequestorOrigin()->IsUnique()) {
-    request.SetRequestorOrigin(GetDocument()->IsSandboxed(kSandboxOrigin)
-                                   ? SecurityOrigin::Create(document_->Url())
-                                   : document_->GetSecurityOrigin());
+  // Subresource requests inherit their requestor origin from |document_|
+  // directly.  Top-level frame types are taken care of in 'FrameLoadRequest()'.
+  // Auxiliary frame types in 'CreateWindow()' and 'FrameLoader::Load'.
+  if (!request.RequestorOrigin()) {
+    if (request.GetFrameType() == WebURLRequest::kFrameTypeNone) {
+      Document* document = GetDocument();
+      request.SetRequestorOrigin(document->IsSandboxed(kSandboxOrigin)
+                                     ? SecurityOrigin::Create(document->Url())
+                                     : document->GetSecurityOrigin());
+    } else {
+      // Set the requestor origin to the same origin as the frame's document
+      // if it hasn't yet been set. (We may hit here for nested frames and
+      // redirect cases)
+      request.SetRequestorOrigin(
+          GetFrame()->GetDocument()->GetSecurityOrigin());

[kinuko, 2017/06/07 02:00:49]

Wasn't bit sure if we should also check IsSandboxed(kSandboxOrigin) here (this
code is from RenderFrameImpl::WillSendRequest)

+    }
   }
 }
 
 MHTMLArchive* FrameFetchContext::Archive() const {
   DCHECK(!IsMainFrame());
   // TODO(nasko): How should this work with OOPIF?
   // The MHTMLArchive is parsed as a whole, but can be constructed from frames
   // in multiple processes. In that case, which process should parse it and how
   // should the output be spread back across multiple processes?
   if (!GetFrame()->Tree().Parent()->IsLocalFrame())
@@ skipping 109 matching lines @@
   return loader;
 }
 
 DEFINE_TRACE(FrameFetchContext) {
   visitor->Trace(document_loader_);
   visitor->Trace(document_);
   BaseFetchContext::Trace(visitor);
 }
 
 }  // namespace blink