Stop storing ScriptValue in MessageEvent

third_party/WebKit/Source/core/events/MessageEvent.cpp

Index: third_party/WebKit/Source/core/events/MessageEvent.cpp
diff --git a/third_party/WebKit/Source/core/events/MessageEvent.cpp b/third_party/WebKit/Source/core/events/MessageEvent.cpp
index aba2ca04f8d5357793f78660b892ecbf9b4f28a9..1b3f1cac1f6cfeb9b67a09b85a17af86a0285fdb 100644
--- a/third_party/WebKit/Source/core/events/MessageEvent.cpp
+++ b/third_party/WebKit/Source/core/events/MessageEvent.cpp
@@ -40,15 +40,19 @@ static inline bool IsValidSource(EventTarget* source) {
          source->ToServiceWorker();
 }
 
-MessageEvent::MessageEvent() : data_type_(kDataTypeScriptValue) {}
+MessageEvent::MessageEvent()
+    : data_type_(kDataTypeV8Reference), data_as_v8_value_reference_(this) {}
 
 MessageEvent::MessageEvent(const AtomicString& type,
                            const MessageEventInit& initializer)
     : Event(type, initializer),
-      data_type_(kDataTypeScriptValue),
+      data_type_(kDataTypeV8Reference),
+      data_as_v8_value_reference_(this),
       source_(nullptr) {
-  if (initializer.hasData())
-    data_as_script_value_ = initializer.data();
+  if (initializer.hasData()) {
+    data_as_v8_value_reference_.Set(initializer.data().GetIsolate(),
+                                    initializer.data().V8Value());
+  }
   if (initializer.hasOrigin())
     origin_ = initializer.origin();
   if (initializer.hasLastEventId())
@@ -66,7 +70,8 @@ MessageEvent::MessageEvent(const String& origin,
                            MessagePortArray* ports,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
-      data_type_(kDataTypeScriptValue),
+      data_type_(kDataTypeV8Reference),
+      data_as_v8_value_reference_(this),
       origin_(origin),
       last_event_id_(last_event_id),
       source_(source),
@@ -82,6 +87,7 @@ MessageEvent::MessageEvent(PassRefPtr<SerializedScriptValue> data,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeSerializedScriptValue),
+      data_as_v8_value_reference_(this),
       data_as_serialized_script_value_(std::move(data)),
       origin_(origin),
       last_event_id_(last_event_id),
@@ -101,6 +107,7 @@ MessageEvent::MessageEvent(PassRefPtr<SerializedScriptValue> data,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeSerializedScriptValue),
+      data_as_v8_value_reference_(this),
       data_as_serialized_script_value_(std::move(data)),
       origin_(origin),
       last_event_id_(last_event_id),
@@ -118,6 +125,7 @@ MessageEvent::MessageEvent(const String& data,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeString),
+      data_as_v8_value_reference_(this),
       data_as_string_(data),
       origin_(origin) {}
 
@@ -126,6 +134,7 @@ MessageEvent::MessageEvent(Blob* data,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeBlob),
+      data_as_v8_value_reference_(this),
       data_as_blob_(data),
       origin_(origin) {}
 
@@ -134,6 +143,7 @@ MessageEvent::MessageEvent(DOMArrayBuffer* data,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeArrayBuffer),
+      data_as_v8_value_reference_(this),
       data_as_array_buffer_(data),
       origin_(origin) {}
 
@@ -163,8 +173,9 @@ void MessageEvent::initMessageEvent(const AtomicString& type,
 
   initEvent(type, can_bubble, cancelable);
 
-  data_type_ = kDataTypeScriptValue;
-  data_as_script_value_ = data;
+  data_type_ = kDataTypeV8Reference;
+  if (!data.IsEmpty())
+    data_as_v8_value_reference_.Set(data.GetIsolate(), data.V8Value());
   origin_ = origin;
   last_event_id_ = last_event_id;
   source_ = source;
@@ -224,6 +235,13 @@ const AtomicString& MessageEvent::InterfaceName() const {
   return EventNames::MessageEvent;
 }
 
+ScriptValue MessageEvent::DataAsScriptValue(ScriptState* script_state) const {
+  DCHECK_EQ(data_type_, kDataTypeV8Reference);
+  DCHECK(script_state);
+  return ScriptValue(script_state, data_as_v8_value_reference_.NewLocal(

[haraken, 2017/05/02 07:24:41]

Do you know why we don't need to check the world before returning the v8 value?
The message event may be created and accessed by different worlds.

[bashi, 2017/05/02 07:38:12]

In V8MessageEvent::dataAttributeGetterCustom() we clone the v8 value when world
is different. V8ValueFor(script_state) does the job.

If we remove custom getter we need to check here, but removing the custom getter
doesn't seem to be trivial change.

[bashi, 2017/05/09 08:09:33]

Sorry I was wrong. We need to check world here. Changed this CL to use almost
the same approach as CustomEvent. In a follow-up CL I'll try to reduce the dup
(maybe by introducing WorldAwareReference).

+                                       script_state->GetIsolate()));
+}
+
 MessagePortArray MessageEvent::ports(bool& is_null) const {
   // TODO(bashi): Currently we return a copied array because the binding
   // layer could modify the content of the array while executing JS callbacks.
@@ -254,6 +272,11 @@ DEFINE_TRACE(MessageEvent) {
   Event::Trace(visitor);
 }
 
+DEFINE_TRACE_WRAPPERS(MessageEvent) {
+  visitor->TraceWrappers(data_as_v8_value_reference_);
+  Event::TraceWrappers(visitor);
+}
+
 v8::Local<v8::Object> MessageEvent::AssociateWithWrapper(
     v8::Isolate* isolate,
     const WrapperTypeInfo* wrapper_type,
@@ -264,7 +287,7 @@ v8::Local<v8::Object> MessageEvent::AssociateWithWrapper(
   // how much memory is used via the wrapper. To keep the wrapper alive, it's
   // set to the wrapper of the MessageEvent as a private value.
   switch (GetDataType()) {
-    case MessageEvent::kDataTypeScriptValue:
+    case MessageEvent::kDataTypeV8Reference:
     case MessageEvent::kDataTypeSerializedScriptValue:
       break;
     case MessageEvent::kDataTypeString: