Avoid unnecessary byte-swapping in blink's SerializedScriptValue.

third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueTest.cpp

 // Copyright 2014 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 "bindings/core/v8/SerializedScriptValue.h"
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/SerializedScriptValueFactory.h"
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8BindingForTesting.h"
 #include "bindings/core/v8/V8File.h"
+#include "bindings/core/v8/V8ImageData.h"
 #include "core/fileapi/File.h"
 #include "platform/testing/UnitTestHelpers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace blink {
 
+TEST(SerializedScriptValueTest, WireFormatRoundTrip) {
+  V8TestingScope scope;
+
+  v8::Local<v8::Value> v8OriginalTrue =
+      v8::Boolean::New(scope.GetIsolate(), true);

[jbroman, 2017/04/12 18:34:04]

More concisely: v8::True(scope.GetIsolate());

At that point, might as well inline it at it's sole use, below.

[pwnall, 2017/04/12 21:37:30]

Done.
Thank you!

+  RefPtr<SerializedScriptValue> sourceSerializedScriptValue =
+      SerializedScriptValue::Serialize(
+          scope.GetIsolate(), v8OriginalTrue,
+          SerializedScriptValue::SerializeOptions(), ASSERT_NO_EXCEPTION);
+
+  Vector<char> wireData;
+  sourceSerializedScriptValue->ToWireBytes(wireData);
+
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::Create(wireData.Data(), wireData.size());
+  v8::Local<v8::Value> deserialized =
+      serializedScriptValue->Deserialize(scope.GetIsolate());
+  EXPECT_TRUE(deserialized->IsTrue());
+}
+
+TEST(SerializedScriptValueTest, WireFormatVersion17NoByteFlipping) {
+  V8TestingScope scope;
+
+  const uint8_t data[] = {0xFF, 0x11, 0xFF, 0x0D, 0x54, 0x00};
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::Create(reinterpret_cast<const char*>(data),
+                                    sizeof(data));
+  v8::Local<v8::Value> deserialized =
+      serializedScriptValue->Deserialize(scope.GetIsolate());
+  EXPECT_TRUE(deserialized->IsTrue());
+}
+
+TEST(SerializedScriptValueTest, WireFormatVersion16ByteFlipping) {
+  V8TestingScope scope;
+
+  // Using UChar instead of uint8_t to get ntohs() byte flipping.
+  const UChar data[] = {0xFF10, 0xFF0D, 0x5400};
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::Create(reinterpret_cast<const char*>(data),
+                                    sizeof(data));
+  v8::Local<v8::Value> deserialized =
+      serializedScriptValue->Deserialize(scope.GetIsolate());
+  EXPECT_TRUE(deserialized->IsTrue());
+}
+
+TEST(SerializedScriptValueTest, WireFormatVersion13ByteFlipping) {
+  V8TestingScope scope;
+
+  // Using UChar instead of uint8_t to get ntohs() byte flipping.
+  const UChar data[] = {0xFF0D, 0x5400};
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::Create(reinterpret_cast<const char*>(data),
+                                    sizeof(data));
+  v8::Local<v8::Value> deserialized =
+      serializedScriptValue->Deserialize(scope.GetIsolate());
+  EXPECT_TRUE(deserialized->IsTrue());
+}
+
+TEST(SerializedScriptValueTest, WireFormatVersion0ByteFlipping) {
+  V8TestingScope scope;
+
+  // Using UChar instead of uint8_t to get ntohs() byte flipping.
+  const UChar data[] = {0x5400};
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::Create(reinterpret_cast<const char*>(data),
+                                    sizeof(data));
+  v8::Local<v8::Value> deserialized =
+      serializedScriptValue->Deserialize(scope.GetIsolate());
+  EXPECT_TRUE(deserialized->IsTrue());
+}
+
+TEST(SerializedScriptValueTest, WireFormatVersion0ImageData) {
+  V8TestingScope scope;
+  v8::Isolate* isolate = scope.GetIsolate();
+
+  // Using UChar instead of uint8_t to get ntohs() byte flipping.
+  //
+  // This builds the smallest possible ImageData whose first data byte is 0xFF,
+  // as follows.
+  //
+  // width = 127, encoded as 0xFF 0x00 (degenerate varint)
+  // height = 1, encoded as 0x01 (varint)
+  // pixelLength = 508 (127 * 1 * 4), encoded as 0xFC 0x03 (varint)
+  // pixel data = 508 bytes, all zero
+  Vector<UChar> data;
+  data.push_back(0x23FF);
+  data.push_back(0x001);
+  data.push_back(0xFC03);
+  data.Resize(257);  // (508 pixel data + 6 header bytes) / 2
+
+  // Using UChar instead of uint8_t to get ntohs() byte flipping.
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::Create(reinterpret_cast<const char*>(data.Data()),
+                                    data.size() * sizeof(UChar));
+  v8::Local<v8::Value> deserialized =
+      serializedScriptValue->Deserialize(isolate);
+  ASSERT_TRUE(deserialized->IsObject());
+  v8::Local<v8::Object> deserializedObject = deserialized.As<v8::Object>();
+  ASSERT_TRUE(V8ImageData::hasInstance(deserializedObject, isolate));
+  ImageData* imageData = V8ImageData::toImpl(deserializedObject);
+  EXPECT_EQ(imageData->width(), 127);
+  EXPECT_EQ(imageData->height(), 1);
+}
+
+TEST(SerializedScriptValueTest, NullValue) {

[jbroman, 2017/04/12 18:34:04]

Is this substantially different from
V8ScriptValueSerializerTest.DecodeHardcodedNullValue?

[pwnall, 2017/04/12 21:37:30]

No.

If the intent of the test is to make sure that
SerializedScriptValue::NullValue() returns something reasonable, I'd argue that
it belongs in SerializedScriptValueTest, because NullValue is defined on
SerializedScriptValue.

I removed the test. I can add it back in a follow-up if you agree with my
argument.

+  V8TestingScope scope;
+
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::NullValue();
+  v8::Local<v8::Value> deserialized =
+      serializedScriptValue->Deserialize(scope.GetIsolate());
+  EXPECT_TRUE(deserialized->IsNull());
+}
+
 TEST(SerializedScriptValueTest, UserSelectedFile) {
   V8TestingScope scope;
   String file_path = testing::BlinkRootDir();
   file_path.Append("/Source/bindings/core/v8/SerializedScriptValueTest.cpp");
   File* original_file = File::Create(file_path);
   ASSERT_TRUE(original_file->HasBackingFile());
   ASSERT_EQ(File::kIsUserVisible, original_file->GetUserVisibility());
   ASSERT_EQ(file_path, original_file->GetPath());
 
   v8::Local<v8::Value> v8_original_file =
@@ skipping 30 matching lines @@
       serialized_script_value->Deserialize(scope.GetIsolate());
 
   ASSERT_TRUE(V8File::hasInstance(v8_file, scope.GetIsolate()));
   File* file = V8File::toImpl(v8::Local<v8::Object>::Cast(v8_file));
   EXPECT_FALSE(file->HasBackingFile());
   EXPECT_EQ(File::kIsNotUserVisible, file->GetUserVisibility());
   EXPECT_EQ("hello.txt", file->name());
 }
 
 }  // namespace blink