PlzNavigate: Fix form submission to OOPIF frame.

content/browser/frame_host/navigation_request.cc

Index: content/browser/frame_host/navigation_request.cc
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index 55eb1ee156e9d58516b5d4eafbb640d93c2d3119..4d5cfc86e7bd145febc1b8ccade449833ad39124 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -199,12 +199,23 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
     PreviewsState previews_state,
     bool is_same_document_history_load,
     bool is_history_navigation_in_new_child,
+    const scoped_refptr<ResourceRequestBodyImpl>& post_body,
     const base::TimeTicks& navigation_start,
     NavigationControllerImpl* controller) {
-  // Fill POST data in the request body.
+  // A form submission happens either because the navigation is a
+  // renderer-initiated form submission that took the OpenURL path or either a

[nasko, 2017/05/16 04:08:21]

nit: s/or either/or/

[arthursonzogni, 2017/05/16 12:45:58]

Done.

+  // back/forward/reload navigation the does a form resubmission.
   scoped_refptr<ResourceRequestBodyImpl> request_body;
-  if (frame_entry.method() == "POST")
+  if (post_body) {
+    // Standard form-submission from the renderer.

[nasko, 2017/05/16 04:08:21]

nit: no need for "-" between form and submission

[arthursonzogni, 2017/05/16 12:45:58]

Done.

+    request_body = post_body;
+  } else if (frame_entry.method() == "POST") {
+    // Form resubmission during a back/forward/reload navigation.
     request_body = frame_entry.GetPostData();
+  }
+  // TODO(arthursonzogni): Form submission with the "GET" method is possible.
+  // This is not currently handled here.
+  bool is_form_submission = bool(request_body);

[nasko, 2017/05/16 04:08:21]

nit: !!request_body?

[arthursonzogni, 2017/05/16 12:45:58]

I would prefer not to. If you don't like: "bool(...)", maybe you would prefer:
"(request_body != nullptr)"

If it is okay, feel free to run the CQ.

[nasko, 2017/05/16 14:02:09]

The main point is to keep code consistent. I haven't seen "bool()" usage around
and it surprised me. I'm fine with "(request_body != nullptr)", though we try to
avoid that too in favor of the "!!ptr" pattern.

 
   base::Optional<url::Origin> initiator =
       frame_tree_node->IsMainFrame()
@@ -227,11 +238,7 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
                             false,  // skip_service_worker
                             REQUEST_CONTEXT_TYPE_LOCATION,
                             blink::WebMixedContentContextType::kBlockable,
-                            // TODO(arthursonzogni): It can be true for form
-                            // resubmission when the user reloads the page. This
-                            // needs to be fixed.
-                            false,  // is_form_submission
-                            initiator),
+                            is_form_submission, initiator),
       entry.ConstructRequestNavigationParams(
           frame_entry, common_params.url, common_params.method,
           is_history_navigation_in_new_child,