PlzNavigate: store POST data in the FrameNavigationEntry

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 132 matching lines @@
 #include "media/blink/webencryptedmediaclient_impl.h"
 #include "media/blink/webmediaplayer_impl.h"
 #include "media/renderers/gpu_video_accelerator_factories.h"
 #include "mojo/common/url_type_converters.h"
 #include "mojo/edk/js/core.h"
 #include "mojo/edk/js/support.h"
 #include "net/base/data_url.h"
 #include "net/base/net_errors.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/http/http_util.h"
+#include "storage/common/data_element.h"
 #include "third_party/WebKit/public/platform/URLConversion.h"
 #include "third_party/WebKit/public/platform/WebCachePolicy.h"
 #include "third_party/WebKit/public/platform/WebData.h"
 #include "third_party/WebKit/public/platform/WebMediaPlayer.h"
 #include "third_party/WebKit/public/platform/WebMediaPlayerSource.h"
 #include "third_party/WebKit/public/platform/WebSecurityOrigin.h"
 #include "third_party/WebKit/public/platform/WebStorageQuotaCallbacks.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/platform/WebURLError.h"
@@ skipping 367 matching lines @@
   // passed back to the browser in the DidCommitProvisionalLoad and the
   // DocumentLoadComplete IPCs.
   base::TimeDelta ui_timestamp = common_params.ui_timestamp - base::TimeTicks();
   request.setUiStartTime(ui_timestamp.InSecondsF());
   request.setInputPerfMetricReportPolicy(
       static_cast<WebURLRequest::InputToLoadPerfMetricReportPolicy>(
           common_params.report_type));
   return request;
 }
 
+// Converts the HTTP body data stored in ResourceRequestBody format to a
+// WebHTTPBody, which is then added to the WebURLRequest.
+// PlzNavigate: used to add the POST data sent by the renderer at commit time
+// to the WebURLRequest used to commit the navigation. This ensures that the
+// POST data will be in the PageState sent to the browser on commit.
+void AddHTTPBodyToRequest(WebURLRequest* request,
+                          scoped_refptr<ResourceRequestBody> body) {

[Łukasz Anforowicz, 2016/05/12 02:53:59]

:-)  I've started creating my own version of this code (OTOH, I see that your
version is better - my version for example I didn't copy the identifier :-/). 
https://codereview.chromium.org/1956383003/patch/20001/30017

[clamy, 2016/05/12 08:53:13]

This is based on the code from content/renderer/history_serialization.cc.

[Łukasz Anforowicz, 2016/05/12 19:44:12]

I see.  Fascinating that we have 3 separate structs for essentially the same
thing - ExplodedHttpBodyElement, WebHTTPBody::Element and
ResourceRequestBody::Element.  If those structs truly need to stay the same then
I guess there is no way around having to have separate convertion code... :-(

+  WebHTTPBody http_body;
+  http_body.initialize();
+  http_body.setIdentifier(body->identifier());
+  for (auto element : *(body->elements())) {

[Łukasz Anforowicz, 2016/05/12 02:53:59]

nit: Could you replace auto with: const ResourceRequestBody::Element& ?

[clamy, 2016/05/12 08:53:13]

Done.

+    switch (element.type()) {
+      case storage::DataElement::TYPE_BYTES:
+        http_body.appendData(WebData(
+            reinterpret_cast<const char*>(element.bytes()), element.length()));

[Łukasz Anforowicz, 2016/05/12 02:54:00]

Why is reinterpret_cast needed here?  reinterpret_casts are evil, right?  FWIW,
similar code compiled fine without the cast at
https://codereview.chromium.org/1956383003/patch/20001/30017?

[clamy, 2016/05/12 08:53:13]

Done.

+        break;
+      case storage::DataElement::TYPE_FILE:
+        http_body.appendFileRange(
+            element.path().AsUTF16Unsafe(), element.offset(), element.length(),

[Łukasz Anforowicz, 2016/05/12 02:54:00]

Should we translate back std::numeric_limits<uint64_t>::max() into -1, as done
in the other translation direction at
https://codereview.chromium.org/1907443006/patch/180001/190014 ?  Or will Blink
understand / gracefully deal with this?  I guess this might already work today
because in 2s-complement there is (-1) == static_cast<long
long>(std::numeric_limits<uint64_t>::max()), but it feels better to be explicit.

[clamy, 2016/05/12 08:53:13]

Done.

+            element.expected_modification_time().ToDoubleT());
+        break;
+      case storage::DataElement::TYPE_FILE_FILESYSTEM:
+        http_body.appendFileSystemURLRange(
+            element.filesystem_url(), element.offset(), element.length(),
+            element.expected_modification_time().ToDoubleT());
+        break;
+      case storage::DataElement::TYPE_BLOB:
+        http_body.appendBlob(WebString::fromUTF8(element.blob_uuid()));
+        break;
+      default:

[Łukasz Anforowicz, 2016/05/12 02:53:59]

nit: I wonder if we should explicitly spell out types that we don't expect to
encounter - spelling them out will show future maintainers that we've thought
about these types (rather than having future maintainers wonder if some types
were added after this switch statement).  FWIW, this is what is done in
UploadDataStreamBuilder::Build -
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/lo...

[clamy, 2016/05/12 08:53:13]

Done.

+        NOTREACHED();
+        break;
+    }
+  }
+  request->setHTTPBody(http_body);
+}
+
 // Sanitizes the navigation_start timestamp for browser-initiated navigations,
 // where the browser possibly has a better notion of start time than the
 // renderer. In the case of cross-process navigations, this carries over the
 // time of finishing the onbeforeunload handler of the previous page.
 // TimeTicks is sometimes not monotonic across processes, and because
 // |browser_navigation_start| is likely before this process existed,
 // InterProcessTimeTicksConverter won't help. The timestamp is sanitized by
 // clamping it to renderer_navigation_start, initialized earlier in the call
 // stack.
 base::TimeTicks SanitizeNavigationTiming(
@@ skipping 944 matching lines @@
   }
 
   RenderThreadImpl* render_thread_impl = RenderThreadImpl::current();
   // Can be NULL in tests.
   if (render_thread_impl)
     render_thread_impl->GetRendererScheduler()->OnNavigationStarted();
   DCHECK(!IsBrowserSideNavigationEnabled());
   TRACE_EVENT2("navigation", "RenderFrameImpl::OnNavigate", "id", routing_id_,
                "url", common_params.url.possibly_invalid_spec());
   NavigateInternal(common_params, start_params, request_params,
-                   std::unique_ptr<StreamOverrideParameters>());
+                   std::unique_ptr<StreamOverrideParameters>(), nullptr);
 }
 
 void RenderFrameImpl::BindServiceRegistry(
     shell::mojom::InterfaceProviderRequest services,
     shell::mojom::InterfaceProviderPtr exposed_services) {
   service_registry_.Bind(std::move(services));
   service_registry_.BindRemoteServiceProvider(std::move(exposed_services));
 }
 
 ManifestManager* RenderFrameImpl::manifest_manager() {
@@ skipping 3116 matching lines @@
 void RenderFrameImpl::FocusedNodeChangedForAccessibility(const WebNode& node) {
   if (renderer_accessibility())
     renderer_accessibility()->AccessibilityFocusedNodeChanged(node);
 }
 
 // PlzNavigate
 void RenderFrameImpl::OnCommitNavigation(
     const ResourceResponseHead& response,
     const GURL& stream_url,
     const CommonNavigationParams& common_params,
-    const RequestNavigationParams& request_params) {
+    const RequestNavigationParams& request_params,
+    scoped_refptr<ResourceRequestBody> post_data) {
   CHECK(IsBrowserSideNavigationEnabled());
   // This will override the url requested by the WebURLLoader, as well as
   // provide it with the response to the request.
   std::unique_ptr<StreamOverrideParameters> stream_override(
       new StreamOverrideParameters());
   stream_override->stream_url = stream_url;
   stream_override->response = response;
 
   NavigateInternal(common_params, StartNavigationParams(), request_params,
-                   std::move(stream_override));
+                   std::move(stream_override), post_data);
 }
 
 // PlzNavigate
 void RenderFrameImpl::OnFailedNavigation(
     const CommonNavigationParams& common_params,
     const RequestNavigationParams& request_params,
     bool has_stale_copy_in_cache,
     int error_code) {
   DCHECK(IsBrowserSideNavigationEnabled());
   bool is_reload = IsReload(common_params.navigation_type);
@@ skipping 575 matching lines @@
     params.frame_unique_name = frame_->uniqueName().utf8();
   }
 
   Send(new FrameHostMsg_OpenURL(routing_id_, params));
 }
 
 void RenderFrameImpl::NavigateInternal(
     const CommonNavigationParams& common_params,
     const StartNavigationParams& start_params,
     const RequestNavigationParams& request_params,
-    std::unique_ptr<StreamOverrideParameters> stream_params) {
+    std::unique_ptr<StreamOverrideParameters> stream_params,
+    scoped_refptr<ResourceRequestBody> post_data) {
   bool browser_side_navigation = IsBrowserSideNavigationEnabled();
 
   // Lower bound for browser initiated navigation start time.
   base::TimeTicks renderer_navigation_start = base::TimeTicks::Now();
   bool is_reload = IsReload(common_params.navigation_type);
   bool is_history_navigation = request_params.page_state.IsValid();
   WebCachePolicy cache_policy = WebCachePolicy::UseProtocolCachePolicy;
   RenderFrameImpl::PrepareRenderViewForNavigation(
       common_params.url, request_params);
 
@@ skipping 40 matching lines @@
           ? blink::WebFrameLoadType::ReplaceCurrentItem
           : blink::WebFrameLoadType::Standard;
   blink::WebHistoryLoadType history_load_type =
       blink::WebHistoryDifferentDocumentLoad;
   bool should_load_request = false;
   WebHistoryItem item_for_history_navigation;
   WebURLRequest request =
       CreateURLRequestForNavigation(common_params, std::move(stream_params),
                                     frame_->isViewSourceModeEnabled());
 
+  if (IsBrowserSideNavigationEnabled() && post_data)
+    AddHTTPBodyToRequest(&request, post_data);
+
   // Used to determine whether this frame is actually loading a request as part
   // of a history navigation.
   bool has_history_navigation_in_frame = false;
 
 #if defined(OS_ANDROID)
   request.setHasUserGesture(start_params.has_user_gesture);
 #endif
 
   // PlzNavigate: Make sure that Blink's loader will not try to use browser side
   // navigation for this request (since it already went to the browser).
@@ skipping 730 matching lines @@
                                     int match_count,
                                     int ordinal,
                                     const WebRect& selection_rect,
                                     bool final_status_update) {
   Send(new FrameHostMsg_Find_Reply(routing_id_, request_id, match_count,
                                    selection_rect, ordinal,
                                    final_status_update));
 }
 
 }  // namespace content