PlzNavigate: inform the renderer that a navigation is a POST

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index 172cd5efb478a1199f6d1d595a21af892f792928..f1d9f15fe18fbd4143eef4bd12278f3298171c3f 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -1488,7 +1488,13 @@ void ResourceDispatcherHostImpl::BeginRequest(
                                    : 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);
@@ -2214,7 +2220,7 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
       // 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))) {
@@ -2249,7 +2255,7 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
   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);