Return null when no promise/reason is given in PromiseRejectionEvent

Source/bindings/core/v8/custom/V8PromiseRejectionEventCustom.cpp

+// Copyright 2015 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 "config.h"
+#include "bindings/core/v8/V8PromiseRejectionEvent.h"
+
+#include "bindings/core/v8/SerializedScriptValue.h"
+#include "bindings/core/v8/SerializedScriptValueFactory.h"
+#include "bindings/core/v8/V8HiddenValue.h"
+#include "bindings/core/v8/V8History.h"

[haraken, 2015/06/18 17:52:09]

This wouldn't be needed.

+#include "core/events/PopStateEvent.h"

[haraken, 2015/06/18 17:52:08]

Ditto.

+#include "core/frame/History.h"

[haraken, 2015/06/18 17:52:09]

Ditto.

+
+namespace blink {
+
+void V8PromiseRejectionEvent::promiseAttributeGetterCustom(const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+    v8::Isolate* isolate = info.GetIsolate();
+    v8::Local<v8::Value> result = V8HiddenValue::getHiddenValue(isolate, info.Holder(), V8HiddenValue::promise(isolate));

[haraken, 2015/06/18 17:52:08]

What is the point of setting a hidden value here? The hidden value makes sense
in CustomEventCustom etc to avoid an unnecessary serialization/deserialization
for CustomEvent::m_serializedDetail.

My proposal would be to introduce PromiseRejectionEvent::m_serializedPromise and
use the same pattern here. It will solve the cross-world-wrapper-leakage
problem.

[jochen (gone - plz use gerrit), 2015/06/18 18:53:14]

i should probably just return null if it's from a different world.

we can't serialize promises

[haraken, 2015/06/18 19:01:00]

Ah, right! Maybe it would be better to have a comment about this -- we don't
want to share the promise among worlds.

However, doesn't it imply that PromiseRejectionEvent doesn't need to hold the
promise? I guess the hidden value would be enough. Also the hidden value on the
event wrapper will keep the promise alive.

+
+    if (!result.IsEmpty()) {
+        v8SetReturnValue(info, result);
+        return;
+    }
+
+    PromiseRejectionEvent* event = V8PromiseRejectionEvent::toImpl(info.Holder());
+    result = event->promise(isolate);
+    if (result.IsEmpty())
+        result = v8::Null(isolate);
+
+    V8HiddenValue::setHiddenValue(isolate, info.Holder(), V8HiddenValue::promise(isolate), result);
+
+    v8SetReturnValue(info, result);
+}
+
+void V8PromiseRejectionEvent::reasonAttributeGetterCustom(const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+    v8::Isolate* isolate = info.GetIsolate();
+    v8::Local<v8::Value> result = V8HiddenValue::getHiddenValue(isolate, info.Holder(), V8HiddenValue::reason(isolate));

[haraken, 2015/06/18 17:52:08]

Ditto.

+
+    if (!result.IsEmpty()) {
+        v8SetReturnValue(info, result);
+        return;
+    }
+
+    PromiseRejectionEvent* event = V8PromiseRejectionEvent::toImpl(info.Holder());
+    result = event->reason(isolate);
+    if (result.IsEmpty())
+        result = v8::Null(isolate);
+
+    V8HiddenValue::setHiddenValue(isolate, info.Holder(), V8HiddenValue::reason(isolate), result);
+
+    v8SetReturnValue(info, result);
+}
+
+} // namespace blink