Make a deserializing function

third_party/WebKit/Source/bindings/core/v8/GeneratedCodeHelper.h

Index: third_party/WebKit/Source/bindings/core/v8/GeneratedCodeHelper.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/GeneratedCodeHelper.h b/third_party/WebKit/Source/bindings/core/v8/GeneratedCodeHelper.h
index 1f09d19c04d0e9d505d829ddbde1ea16936c6272..7696019ec4455b360504ac6620cf7022cdbcae4d 100644
--- a/third_party/WebKit/Source/bindings/core/v8/GeneratedCodeHelper.h
+++ b/third_party/WebKit/Source/bindings/core/v8/GeneratedCodeHelper.h
@@ -11,6 +11,7 @@
 #ifndef GeneratedCodeHelper_h
 #define GeneratedCodeHelper_h
 
+#include "bindings/core/v8/SerializedScriptValue.h"

[Yuki, 2016/08/10 02:16:34]

I'm not sure if it's possible or not, but could you try to use a forward
declaration for SerializedScriptValue?  i.e. Can you move this #include into
.cpp file?

[lkawai, 2016/08/10 05:28:30]

Done.

 #include "core/CoreExport.h"
 #include <v8.h>
 
@@ -18,6 +19,8 @@ namespace blink {
 
 CORE_EXPORT void v8ConstructorAttributeGetter(v8::Local<v8::Name> propertyName, const v8::PropertyCallbackInfo<v8::Value>&);
 
+CORE_EXPORT v8::Local<v8::Value> v8Deserialize(v8::Isolate*, PassRefPtr<SerializedScriptValue>);

[haraken, 2016/08/10 02:19:36]

I'm getting confused about what functions you're planning to move to
GeneratedCodeHelper. value->deserialize() doesn't need to be limited to
generated code (it's used by hand-written bindings as well).

Also if you really want to create a helper function for this, I'd prefer
creating toV8(PassRefPtr<SerializedScriptValue>) and put it in ToV8.h.

[Yuki, 2016/08/10 02:30:16]

For hand-written code, value->deserialize() is preferred, and no need to use
this helper function.

This helper function is only needed for code generator to evaluate the value
only once.
I'm on the fence with that idea.  Serialization / deserialization are a pair
concept.  I think "deserialize" in the name is clearer.

For example,
    v8SetReturnValue(v8Deserialize(serializedValue));
would make more sense than
    v8SetReturnValue(toV8(serializedValue));

The latter case is still understandable, but could be a bit more confusing.

[peria, 2016/08/10 02:31:42]

The objective of CL is to avoid running needless deserialize(),
not to wrap value->deserialize().
So this suggestion makes sense to me.

[Yuki, 2016/08/10 02:37:32]

Note that toV8() is ideally supposed to satisfy the following things.
- toV8(x) == toV8(x) : returns the same V8 object for the same |x|
- does not make any side effect on the value |x|
- just be a simple type conversion

I know some of the actual implementation of toV8() violates some of them.  But
we never wanted to do so.

I guess, deserialization itself is a bit out of scope of toV8().

[lkawai, 2016/08/10 05:28:30]

Acknowledged.

+
 } // namespace blink
 
 #endif // GeneratedCodeHelper_h