Remove dup'ed code for RequestorOrigin and FirstPartyCookie

content/renderer/render_frame_impl.cc

 // Copyright 2013 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 "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 4161 matching lines @@
 void RenderFrameImpl::SaveImageFromDataURL(const blink::WebString& data_url) {
   // Note: We should basically send GURL but we use size-limited string instead
   // in order to send a larger data url to save a image for <canvas> or <img>.
   if (data_url.length() < kMaxLengthOfDataURLString) {
     Send(new FrameHostMsg_SaveImageFromDataURL(render_view_->GetRoutingID(),
                                                routing_id_, data_url.Utf8()));
   }
 }
 
 void RenderFrameImpl::WillSendRequest(blink::WebURLRequest& request) {
-  // 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
-  if (request.FirstPartyForCookies().IsEmpty()) {
-    if (request.GetFrameType() == blink::WebURLRequest::kFrameTypeTopLevel)
-      request.SetFirstPartyForCookies(request.Url());
-    else
-      request.SetFirstPartyForCookies(
-          frame_->GetDocument().FirstPartyForCookies());
-  }
-
-  // Set the requestor origin to the same origin as the frame's document if it
-  // hasn't yet been set.
-  //
-  // 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

[tyoshino (SeeGerritForStatus), 2017/06/08 04:45:44]

Add 625969 to the BUG line

[kinuko, 2017/06/08 04:55:30]

Done.

-  WebDocument frame_document = frame_->GetDocument();
-  if (request.RequestorOrigin().IsUnique() &&
-      !frame_document.GetSecurityOrigin().IsUnique()) {
-    request.SetRequestorOrigin(frame_document.GetSecurityOrigin());
-  }
-
   WebDataSource* provisional_data_source = frame_->ProvisionalDataSource();
   WebDataSource* data_source =
       provisional_data_source ? provisional_data_source : frame_->DataSource();
 
   DocumentState* document_state = DocumentState::FromDataSource(data_source);
   DCHECK(document_state);
   InternalDocumentStateData* internal_data =
       InternalDocumentStateData::FromDocumentState(document_state);
   NavigationStateImpl* navigation_state =
       static_cast<NavigationStateImpl*>(document_state->navigation_state());
@@ skipping 50 matching lines @@
 
   // Attach |should_replace_current_entry| state to requests so that, should
   // this navigation later require a request transfer, all state is preserved
   // when it is re-created in the new process.
   bool should_replace_current_entry = data_source->ReplacesCurrentHistoryItem();
 
   WebFrame* parent = frame_->Parent();
   int parent_routing_id =
       parent ? RenderFrame::GetRoutingIdForWebFrame(parent) : -1;
 
+  WebDocument frame_document = frame_->GetDocument();
   RequestExtraData* extra_data =
       static_cast<RequestExtraData*>(request.GetExtraData());
   if (!extra_data)
     extra_data = new RequestExtraData();
   extra_data->set_visibility_state(VisibilityState());
   extra_data->set_custom_user_agent(custom_user_agent);
   extra_data->set_requested_with(requested_with);
   extra_data->set_render_frame_id(routing_id_);
   extra_data->set_is_main_frame(!parent);
   extra_data->set_frame_origin(url::Origin(frame_document.GetSecurityOrigin()));
@@ skipping 1955 matching lines @@
   if (request_params.should_clear_history_list) {
     CHECK_EQ(-1, render_view_->history_list_offset_);
     CHECK_EQ(0, render_view_->history_list_length_);
   }
 }
 
 void RenderFrameImpl::BeginNavigation(const NavigationPolicyInfo& info) {
   CHECK(IsBrowserSideNavigationEnabled());
   browser_side_navigation_pending_ = true;
 
+  blink::WebURLRequest& request = info.url_request;
+
   // Note: At this stage, the goal is to apply all the modifications the
   // renderer wants to make to the request, and then send it to the browser, so
   // that the actual network request can be started. Ideally, all such
   // modifications should take place in willSendRequest, and in the
   // implementation of willSendRequest for the various InspectorAgents
   // (devtools).
   //
   // TODO(clamy): Apply devtools override.
   // TODO(clamy): Make sure that navigation requests are not modified somewhere
   // else in blink.
-  WillSendRequest(info.url_request);
+  WillSendRequest(request);
+
+  // Set RequestorInfo and FirstPartyForCookies.

[tyoshino (SeeGerritForStatus), 2017/06/08 04:45:45]

RequestorInfo -> RequestorOrigin

[kinuko, 2017/06/08 04:55:30]

Done.

+  WebDocument frame_document = frame_->GetDocument();
+  if (request.GetFrameType() == blink::WebURLRequest::kFrameTypeTopLevel)
+    request.SetFirstPartyForCookies(request.Url());
+  else
+    request.SetFirstPartyForCookies(frame_document.FirstPartyForCookies());
+  request.SetRequestorOrigin(frame_document.GetSecurityOrigin());
 
   // Update the transition type of the request for client side redirects.
   if (!info.url_request.GetExtraData())

[tyoshino (SeeGerritForStatus), 2017/06/08 04:45:45]

use request everywhere in this function?

[kinuko, 2017/06/08 04:55:30]

Was a bit afraid it might make the change unnecessarily bigger, let me do it in
a follow-up patch.

     info.url_request.SetExtraData(new RequestExtraData());
   if (info.is_client_redirect) {
     RequestExtraData* extra_data =
         static_cast<RequestExtraData*>(info.url_request.GetExtraData());
     extra_data->set_transition_type(ui::PageTransitionFromInt(
         extra_data->transition_type() | ui::PAGE_TRANSITION_CLIENT_REDIRECT));
   }
 
   // TODO(clamy): Same-document navigations should not be sent back to the
   // browser.
   // TODO(clamy): Data urls should not be sent back to the browser either.
   // These values are assumed on the browser side for navigations. These checks
   // ensure the renderer has the correct values.
   DCHECK_EQ(FETCH_REQUEST_MODE_NAVIGATE,
             GetFetchRequestModeForWebURLRequest(info.url_request));
   DCHECK_EQ(FETCH_CREDENTIALS_MODE_INCLUDE,
             GetFetchCredentialsModeForWebURLRequest(info.url_request));
   DCHECK(GetFetchRedirectModeForWebURLRequest(info.url_request) ==
          FetchRedirectMode::MANUAL_MODE);
   DCHECK(frame_->Parent() ||
          GetRequestContextFrameTypeForWebURLRequest(info.url_request) ==
              REQUEST_CONTEXT_FRAME_TYPE_TOP_LEVEL);
   DCHECK(!frame_->Parent() ||
          GetRequestContextFrameTypeForWebURLRequest(info.url_request) ==
              REQUEST_CONTEXT_FRAME_TYPE_NESTED);
 
+  DCHECK(!info.url_request.RequestorOrigin().IsNull());
   base::Optional<url::Origin> initiator_origin =
-      info.url_request.RequestorOrigin().IsNull()
-          ? base::Optional<url::Origin>()
-          : base::Optional<url::Origin>(info.url_request.RequestorOrigin());
+      base::Optional<url::Origin>(info.url_request.RequestorOrigin());
 
   int load_flags = GetLoadFlagsForWebURLRequest(info.url_request);
 
   // Requests initiated via devtools can have caching disabled.
   if (info.is_cache_disabled) {
     // Turn off all caching related flags and set LOAD_BYPASS_CACHE.
     load_flags &= ~(net::LOAD_VALIDATE_CACHE | net::LOAD_SKIP_CACHE_VALIDATION |
                     net::LOAD_ONLY_FROM_CACHE | net::LOAD_DISABLE_CACHE);
     load_flags |= net::LOAD_BYPASS_CACHE;
   }
@@ skipping 510 matching lines @@
       policy(info.default_policy),
       replaces_current_history_item(info.replaces_current_history_item),
       history_navigation_in_new_child_frame(
           info.is_history_navigation_in_new_child_frame),
       client_redirect(info.is_client_redirect),
       cache_disabled(info.is_cache_disabled),
       form(info.form),
       source_location(info.source_location) {}
 
 }  // namespace content