PlzNavigate: inform the renderer that a navigation is a POST

content/browser/loader/resource_dispatcher_host_impl.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.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 
 #include <stddef.h>
 #include <algorithm>
@@ skipping 1470 matching lines @@
     AbortRequestBeforeItStarts(filter_, sync_result, request_id);
     return;
   }
 
   // Construct the request.
   scoped_ptr<net::URLRequest> new_request = request_context->CreateRequest(
       is_navigation_stream_request ? request_data.resource_body_stream_url
                                    : request_data.url,
       request_data.priority, nullptr);
 
-  new_request->set_method(request_data.method);
+  // PlzNavigate: always set the method to GET when gaining access to the

[Charlie Reis, 2016/02/17 05:55:28]

nit: Capitalize Always

+  // stream that contains the response body of a navigation.

[Charlie Reis, 2016/02/17 05:55:28]

I think this comment needs a bit more elaboration.  It took me a while to
remember what is_navigation_stream_request was about, and why it would make
sense for it to be GET even if the actual navigation was POST.

(Also, I'm not sure what the concrete reason is-- what goes wrong if it's POST?)

[clamy, 2016/02/17 12:39:38]

I've added a sentence that explains what happens if we don't do it. Is it now
clearer?

[Charlie Reis, 2016/02/17 21:52:06]

Yes, thanks.

+  if (is_navigation_stream_request)
+    new_request->set_method("GET");
+  else
+    new_request->set_method(request_data.method);
+
   new_request->set_first_party_for_cookies(
       request_data.first_party_for_cookies);
   new_request->set_initiator(request_data.request_initiator);
 
   // If the request is a MAIN_FRAME request, the first-party URL gets updated on
   // redirects.
   if (request_data.resource_type == RESOURCE_TYPE_MAIN_FRAME) {
     new_request->set_first_party_url_policy(
         net::URLRequest::UPDATE_FIRST_PARTY_URL_ON_REDIRECT);
   }
@@ skipping 705 matching lines @@
   CHECK(IsBrowserSideNavigationEnabled());
 
   ResourceType resource_type = info.is_main_frame ?
       RESOURCE_TYPE_MAIN_FRAME : RESOURCE_TYPE_SUB_FRAME;
 
   if (is_shutdown_ ||
       // TODO(davidben): Check ShouldServiceRequest here. This is important; it
       // needs to be checked relative to the child that /requested/ the
       // navigation. It's where file upload checks, etc., come in.
       (delegate_ && !delegate_->ShouldBeginRequest(
-          info.begin_params.method,
+          info.common_params.is_post ? "POST" : "GET",

[Charlie Reis, 2016/02/17 05:55:28]

There might be some loss of generality here.  Was it ever possible for method to
be something else before, like PUT or HEAD?  Hopefully not.

[clamy, 2016/02/17 12:39:38]

I changed the boolean to a string, this should not be an issue now.

[Charlie Reis, 2016/02/17 21:52:06]

Ok, thanks.  (Out of curiosity, was it possible for it to be anything else? 
This question comes up from time to time and I don't know the answer.)

[clamy, 2016/02/22 12:48:36]

Honestly I don't know. I know in some places in the code we assume it, and it's
not possible to have a browser-initiated navigation with a method other than
POST or GET. However, I'm not sure that this is true for render-initiated
navigations.

           info.common_params.url,
           resource_type,
           resource_context))) {
     loader->NotifyRequestFailed(false, net::ERR_ABORTED);
     return;
   }
 
   // Save the URL on the stack to help catch URLRequests which outlive their
   // URLRequestContexts. See https://crbug.com/90971
   char url_buf[128];
@@ skipping 14 matching lines @@
   // CanSendCookiesForOrigin and CanReadRawCookies. Is this needed here?
 
   // Sync loads should have maximum priority and should be the only
   // requests that have the ignore limits flag set.
   DCHECK(!(load_flags & net::LOAD_IGNORE_LIMITS));
 
   scoped_ptr<net::URLRequest> new_request;
   new_request = request_context->CreateRequest(
       info.common_params.url, net::HIGHEST, nullptr);
 
-  new_request->set_method(info.begin_params.method);
+  new_request->set_method(info.common_params.is_post ? "POST" : "GET");
   new_request->set_first_party_for_cookies(
       info.first_party_for_cookies);
   new_request->set_initiator(info.request_initiator);
   if (info.is_main_frame) {
     new_request->set_first_party_url_policy(
         net::URLRequest::UPDATE_FIRST_PARTY_URL_ON_REDIRECT);
   }
 
   SetReferrerForRequest(new_request.get(), info.common_params.referrer);
 
@@ skipping 419 matching lines @@
     load_flags |= net::LOAD_PREFETCH;
   }
 
   if (is_sync_load)
     load_flags |= net::LOAD_IGNORE_LIMITS;
 
   return load_flags;
 }
 
 }  // namespace content