Deduping conversions between ResourceRequestBody/WebHTTPBody/ExplodedHttpBody.

content/public/common/page_state.cc

Index: content/public/common/page_state.cc
diff --git a/content/public/common/page_state.cc b/content/public/common/page_state.cc
index 74f65df7b97383ed190a6412f334d0ead3a7667f..ccdfc0cda4242281618bf24d13e33f692ce53af0 100644
--- a/content/public/common/page_state.cc
+++ b/content/public/common/page_state.cc
@@ -85,16 +85,13 @@ PageState PageState::CreateForTesting(
   state.top.url_string = ToNullableString16(url.possibly_invalid_spec());
 
   if (optional_body_data || optional_body_file_path) {
-    state.top.http_body.is_null = false;
     if (optional_body_data) {
-      ExplodedHttpBodyElement element;
       std::string body_data(optional_body_data);
-      element.SetToBytes(body_data.data(), body_data.size());
-      state.top.http_body.elements.push_back(element);
+      state.top.http_body.request_body = new ResourceRequestBody();
+      state.top.http_body.request_body->AppendBytes(body_data.data(),
+                                                    body_data.size());
     }
     if (optional_body_file_path) {

[Łukasz Anforowicz, 2016/05/26 16:26:52]

This branch is taken by some tests.  I assume that these tests only care about
|state.referenced_files|.

[Charlie Reis, 2016/05/27 21:45:53]

Oh, interesting.  I see that this old code never appended the element to
|state|, which suggests that they didn't bother to check it.

However, I suspect that will be a headache as I try to fix
https://crbug.com/613260.  In that bug (which I'll soon post an update to),
we're failing to track the referenced files per frame, so we end up killing
OOPIFs when they have files in the PageState.  I'm hoping to eliminate
state.referenced_files (which fundamentally can't track files per process in an
OOPIF world) in favor of looking at the actual files in the ResourceRequestBody.

Can you go ahead and add a file element here?  (If it causes problems, I can
revisit it later.)

[Łukasz Anforowicz, 2016/05/31 16:38:04]

Done - I should have written it this way from the start - this more faithfully
mimics what the production code would have done.  I guess I was surprised by the
no-op code here and this is why I've just removed it initially...
I don't foresee problems for this CL, because currently no test looks at the
files inside the request body being created here.  OTOH, this means that the
code I've added doesn't have test coverage and can be already incorrect and/or
rot in the future... :-/

Still - any verification I could add right now would be tautological (i.e. I
would just verify that the test matches what I wrote), so probably best to just
let it be.

-      ExplodedHttpBodyElement element;
-      element.SetToFilePath(*optional_body_file_path);
       state.referenced_files.push_back(base::NullableString16(
           optional_body_file_path->AsUTF16Unsafe(), false));
     }