Deduplicating code performing WebHTTPBody::Element conversions.

content/common/page_state_serialization.cc

Index: content/common/page_state_serialization.cc
diff --git a/content/common/page_state_serialization.cc b/content/common/page_state_serialization.cc
index 3715ab7437622184077ad9406dd1179a6c42e8b4..18f272f2608d9cfec994b519a549afdf250d8f80 100644
--- a/content/common/page_state_serialization.cc
+++ b/content/common/page_state_serialization.cc
@@ -10,6 +10,7 @@
 #include <limits>
 
 #include "base/pickle.h"
+#include "base/strings/nullable_string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -30,8 +31,7 @@ float g_device_scale_factor_for_testing = 0.0;
 void AppendDataToHttpBody(ExplodedHttpBody* http_body, const char* data,
                           int data_length) {
   ExplodedHttpBodyElement element;
-  element.type = blink::WebHTTPBody::Element::TypeData;
-  element.data.assign(data, data_length);
+  element.SetToBytes(data, data_length);
   http_body->elements.push_back(element);
 }
 
@@ -41,11 +41,10 @@ void AppendFileRangeToHttpBody(ExplodedHttpBody* http_body,
                                int file_length,
                                double file_modification_time) {
   ExplodedHttpBodyElement element;
-  element.type = blink::WebHTTPBody::Element::TypeFile;
-  element.file_path = file_path;
-  element.file_start = file_start;
-  element.file_length = file_length;
-  element.file_modification_time = file_modification_time;
+  element.SetToFilePathRange(
+      base::FilePath::FromUTF16Unsafe(file_path.string()),

[Charlie Reis, 2016/05/23 19:58:30]

There's a lot of UTF16Unsafe calls in this CL.  Just want to sanity check that
we're not introducing new problems here-- would we have this these before as
well?  (At a quick glance, I see we do the same thing within
PageState::GetReferencedFiles, so hopefully it isn't an issue.)

Similar for the signed to unsigned int conversions below.

[Łukasz Anforowicz, 2016/05/23 22:13:34]

We would have these UnsafeUTF16 calls before:

- Before this CL, the conversions were present in
content/common/resource_request_body.cc (inside the recently added
ResourceRequestBody::AppendExplodedHTTPBodyElement method).

- The conversions did not have to happen in
content/renderer/history_serialization.cc (because before this CL,
ExplodedHttpBodyElement used to declare field types in a way that was more
compatible with Blink types - i.e. |base::NullableString16 file_path| rather
than |base::FilePath|).

Also - AFAICT, these UTF conversion functions are safe as long as one can
guarantee that input/output truly is UTF16 or UTF8 (and not some other
encoding).  I believe that such guarantee is present here.
Before this CL, the conversions were present in
content/common/resource_request_body.cc (inside the recently added
ResourceRequestBody::AppendExplodedHTTPBodyElement method).  OTOH, the
conversions did not have to happen in content/renderer/history_serialization.cc
(because before this CL, ExplodedHttpBodyElement used to declare field types in
a way that was more compatible with Blink types - i.e. |int64_t| instead of
|uint64_t|).

At the same time - it seems worth to have a dedicated discussion for these
issues separately - I've opened https://crbug.com/614179 to for this.  If we
decide to do something here, it probably would be in a CL separate from this one
(since the current CL just preserves status quo).

+      static_cast<uint64_t>(file_start), static_cast<uint64_t>(file_length),
+      base::Time::FromDoubleT(file_modification_time));
   http_body->elements.push_back(element);
 }
 
@@ -55,19 +54,17 @@ void AppendURLRangeToHttpBody(ExplodedHttpBody* http_body,
                               int file_length,
                               double file_modification_time) {
   ExplodedHttpBodyElement element;
-  element.type = blink::WebHTTPBody::Element::TypeFileSystemURL;
-  element.filesystem_url = url;
-  element.file_start = file_start;
-  element.file_length = file_length;
-  element.file_modification_time = file_modification_time;
+  element.SetToFileSystemUrlRange(
+      url, static_cast<uint64_t>(file_start),
+      static_cast<uint64_t>(file_length),
+      base::Time::FromDoubleT(file_modification_time));
   http_body->elements.push_back(element);
 }
 
 void AppendBlobToHttpBody(ExplodedHttpBody* http_body,
                           const std::string& uuid) {
   ExplodedHttpBodyElement element;
-  element.type = blink::WebHTTPBody::Element::TypeBlob;
-  element.blob_uuid = uuid;
+  element.SetToBlob(uuid);
   http_body->elements.push_back(element);
 }
 
@@ -77,8 +74,9 @@ void AppendReferencedFilesFromHttpBody(
     const std::vector<ExplodedHttpBodyElement>& elements,
     std::vector<base::NullableString16>* referenced_files) {
   for (size_t i = 0; i < elements.size(); ++i) {
-    if (elements[i].type == blink::WebHTTPBody::Element::TypeFile)
-      referenced_files->push_back(elements[i].file_path);
+    if (elements[i].type() == ExplodedHttpBodyElement::TYPE_FILE)
+      referenced_files->push_back(
+          base::NullableString16(elements[i].path().AsUTF16Unsafe(), false));
   }
 }
 
@@ -407,24 +405,35 @@ void WriteHttpBody(const ExplodedHttpBody& http_body, SerializeObject* obj) {
   WriteAndValidateVectorSize(http_body.elements, obj);
   for (size_t i = 0; i < http_body.elements.size(); ++i) {
     const ExplodedHttpBodyElement& element = http_body.elements[i];
-    WriteInteger(element.type, obj);
-    if (element.type == blink::WebHTTPBody::Element::TypeData) {
-      WriteData(element.data.data(), static_cast<int>(element.data.size()),
-                obj);
-    } else if (element.type == blink::WebHTTPBody::Element::TypeFile) {
-      WriteString(element.file_path, obj);
-      WriteInteger64(element.file_start, obj);
-      WriteInteger64(element.file_length, obj);
-      WriteReal(element.file_modification_time, obj);
-    } else if (element.type ==
-               blink::WebHTTPBody::Element::TypeFileSystemURL) {
-      WriteGURL(element.filesystem_url, obj);
-      WriteInteger64(element.file_start, obj);
-      WriteInteger64(element.file_length, obj);
-      WriteReal(element.file_modification_time, obj);
-    } else {
-      DCHECK(element.type == blink::WebHTTPBody::Element::TypeBlob);
-      WriteStdString(element.blob_uuid, obj);
+    switch (element.type()) {
+      case ExplodedHttpBodyElement::TYPE_BYTES:
+        WriteInteger(blink::WebHTTPBody::Element::TypeData, obj);
+        WriteData(element.bytes(), static_cast<int>(element.length()), obj);
+        break;
+      case ExplodedHttpBodyElement::TYPE_FILE:
+        WriteInteger(blink::WebHTTPBody::Element::TypeFile, obj);
+        WriteString(
+            base::NullableString16(element.path().AsUTF16Unsafe(), false), obj);
+        WriteInteger64(static_cast<int64_t>(element.offset()), obj);
+        WriteInteger64(static_cast<int64_t>(element.length()), obj);
+        WriteReal(element.expected_modification_time().ToDoubleT(), obj);
+        break;
+      case ExplodedHttpBodyElement::TYPE_FILE_FILESYSTEM:
+        WriteInteger(blink::WebHTTPBody::Element::TypeFileSystemURL, obj);
+        WriteGURL(element.filesystem_url(), obj);
+        WriteInteger64(static_cast<int64_t>(element.offset()), obj);
+        WriteInteger64(static_cast<int64_t>(element.length()), obj);
+        WriteReal(element.expected_modification_time().ToDoubleT(), obj);
+        break;
+      case ExplodedHttpBodyElement::TYPE_BLOB:
+        WriteInteger(blink::WebHTTPBody::Element::TypeBlob, obj);
+        WriteStdString(element.blob_uuid(), obj);
+        break;
+      case ExplodedHttpBodyElement::TYPE_BYTES_DESCRIPTION:
+      case ExplodedHttpBodyElement::TYPE_DISK_CACHE_ENTRY:
+      default:
+        NOTREACHED();
+        continue;
     }
   }
   WriteInteger64(http_body.identifier, obj);
@@ -667,19 +676,6 @@ void ReadPageState(SerializeObject* obj, ExplodedPageState* state) {
 
 }  // namespace
 
-ExplodedHttpBodyElement::ExplodedHttpBodyElement()
-    : type(blink::WebHTTPBody::Element::TypeData),
-      file_start(0),
-      file_length(-1),
-      file_modification_time(std::numeric_limits<double>::quiet_NaN()) {
-}
-
-ExplodedHttpBodyElement::ExplodedHttpBodyElement(
-    const ExplodedHttpBodyElement& other) = default;
-
-ExplodedHttpBodyElement::~ExplodedHttpBodyElement() {
-}
-
 ExplodedHttpBody::ExplodedHttpBody()
     : identifier(0),
       contains_passwords(false),
@@ -758,7 +754,7 @@ bool GeneratePostData(const ExplodedHttpBody& exploded,
 
   http_body->set_identifier(exploded.identifier);
   for (auto element : exploded.elements)
-    http_body->AppendExplodedHTTPBodyElement(element);
+    http_body->elements_mutable()->push_back(element);
 
   return true;
 }