Stop storing ScriptValue in MessageEvent

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

 /*
  * Copyright (C) 2007 Henry Mason (hmason@mac.com)
  * Copyright (C) 2003, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 22 matching lines @@
 #include "bindings/core/v8/V8PrivateProperty.h"
 #include <memory>
 
 namespace blink {
 
 static inline bool IsValidSource(EventTarget* source) {
   return !source || source->ToLocalDOMWindow() || source->ToMessagePort() ||
          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())
     last_event_id_ = initializer.lastEventId();
   if (initializer.hasSource() && IsValidSource(initializer.source()))
     source_ = initializer.source();
   if (initializer.hasPorts())
     ports_ = new MessagePortArray(initializer.ports());
   DCHECK(IsValidSource(source_.Get()));
 }
 
 MessageEvent::MessageEvent(const String& origin,
                            const String& last_event_id,
                            EventTarget* source,
                            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),
       ports_(ports) {
   DCHECK(IsValidSource(source_.Get()));
 }
 
 MessageEvent::MessageEvent(PassRefPtr<SerializedScriptValue> data,
                            const String& origin,
                            const String& last_event_id,
                            EventTarget* source,
                            MessagePortArray* ports,
                            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),
       source_(source),
       ports_(ports) {
   if (data_as_serialized_script_value_)
     data_as_serialized_script_value_
         ->RegisterMemoryAllocatedWithCurrentScriptContext();
   DCHECK(IsValidSource(source_.Get()));
 }
 
 MessageEvent::MessageEvent(PassRefPtr<SerializedScriptValue> data,
                            const String& origin,
                            const String& last_event_id,
                            EventTarget* source,
                            MessagePortChannelArray channels,
                            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),
       source_(source),
       channels_(std::move(channels)),
       suborigin_(suborigin) {
   if (data_as_serialized_script_value_)
     data_as_serialized_script_value_
         ->RegisterMemoryAllocatedWithCurrentScriptContext();
   DCHECK(IsValidSource(source_.Get()));
 }
 
 MessageEvent::MessageEvent(const String& data,
                            const String& origin,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeString),
+      data_as_v8_value_reference_(this),
       data_as_string_(data),
       origin_(origin) {}
 
 MessageEvent::MessageEvent(Blob* data,
                            const String& origin,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeBlob),
+      data_as_v8_value_reference_(this),
       data_as_blob_(data),
       origin_(origin) {}
 
 MessageEvent::MessageEvent(DOMArrayBuffer* data,
                            const String& origin,
                            const String& suborigin)
     : Event(EventTypeNames::message, false, false),
       data_type_(kDataTypeArrayBuffer),
+      data_as_v8_value_reference_(this),
       data_as_array_buffer_(data),
       origin_(origin) {}
 
 MessageEvent::~MessageEvent() {}
 
 MessageEvent* MessageEvent::Create(const AtomicString& type,
                                    const MessageEventInit& initializer,
                                    ExceptionState& exception_state) {
   if (initializer.source() && !IsValidSource(initializer.source())) {
     exception_state.ThrowTypeError(
         "The optional 'source' property is neither a Window nor MessagePort.");
     return nullptr;
   }
   return new MessageEvent(type, initializer);
 }
 
 void MessageEvent::initMessageEvent(const AtomicString& type,
                                     bool can_bubble,
                                     bool cancelable,
                                     ScriptValue data,
                                     const String& origin,
                                     const String& last_event_id,
                                     EventTarget* source,
                                     MessagePortArray* ports) {
   if (IsBeingDispatched())
     return;
 
   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;
   ports_ = ports;
   suborigin_ = "";
 }
 
 void MessageEvent::initMessageEvent(const AtomicString& type,
                                     bool can_bubble,
                                     bool cancelable,
@@ skipping 39 matching lines @@
   last_event_id_ = last_event_id;
   source_ = source;
   ports_ = ports;
   suborigin_ = "";
 }
 
 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.
   // Avoid copying once we can make sure that the binding layer won't
   // modify the content.
   if (ports_) {
     is_null = false;
     return *ports_;
   }
   is_null = true;
@@ skipping 10 matching lines @@
 }
 
 DEFINE_TRACE(MessageEvent) {
   visitor->Trace(data_as_blob_);
   visitor->Trace(data_as_array_buffer_);
   visitor->Trace(source_);
   visitor->Trace(ports_);
   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,
     v8::Local<v8::Object> wrapper) {
   wrapper = Event::AssociateWithWrapper(isolate, wrapper_type, wrapper);
 
   // Ensures a wrapper is created for the data to return now so that V8 knows
   // 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:
       V8PrivateProperty::GetMessageEventCachedData(isolate).Set(
           wrapper, V8String(isolate, DataAsString()));
       break;
     case MessageEvent::kDataTypeBlob:
       break;
     case MessageEvent::kDataTypeArrayBuffer:
       V8PrivateProperty::GetMessageEventCachedData(isolate).Set(
           wrapper, ToV8(DataAsArrayBuffer(), wrapper, isolate));
       break;
   }
 
   return wrapper;
 }
 
 }  // namespace blink