Commit cb77d7d5 authored by Yuki Shiino's avatar Yuki Shiino Committed by Commit Bot

v8bindings: Remove ToV8(DOMWindow*) hack

A set of overloading of ToV8({ScriptWrappable,EventTarget,Node,
DOMWindow}*) has been expected to be used for each class,
however, it turned out that ToV8(ScriptWrappable* value) is
used even when |value| is a DOMWindow.

Example case)

  void Func(ScriptWrappable* value) {
    ToV8(value);  // resolved to ToV8(ScriptWrappable*)
  }

  Func(window);  // ToV8(ScriptWrappable*) is invoked

This patch removes ToV8(DOMWindow*) overloading and moves the
special-handling of DOMWindow from ToV8 into DOMWindow::Wrap
so that the special-handling always works.

Change-Id: I483ba3293ce6736428947525e7a13d38c6071783
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1996111Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737732}
parent 6eac0441
......@@ -123,7 +123,6 @@ bindings_core_v8_files =
"core/v8/serialization/v8_script_value_serializer.h",
"core/v8/source_location.cc",
"core/v8/source_location.h",
"core/v8/to_v8_for_core.cc",
"core/v8/to_v8_for_core.h",
"core/v8/use_counter_callback.cc",
"core/v8/use_counter_callback.h",
......
......@@ -325,9 +325,8 @@ void LocalWindowProxy::SetupWindowPrototypeChain() {
// The global object, aka window wrapper object.
v8::Local<v8::Object> window_wrapper =
global_proxy->GetPrototype().As<v8::Object>();
v8::Local<v8::Object> associated_wrapper =
AssociateWithWrapper(window, wrapper_type_info, window_wrapper);
DCHECK(associated_wrapper == window_wrapper);
V8DOMWrapper::SetNativeInfo(GetIsolate(), window_wrapper, wrapper_type_info,
window);
// The prototype object of Window interface.
v8::Local<v8::Object> window_prototype =
......
......@@ -145,9 +145,8 @@ void RemoteWindowProxy::SetupWindowPrototypeChain() {
// The global object, aka window wrapper object.
v8::Local<v8::Object> window_wrapper =
global_proxy->GetPrototype().As<v8::Object>();
v8::Local<v8::Object> associated_wrapper =
AssociateWithWrapper(window, wrapper_type_info, window_wrapper);
DCHECK(associated_wrapper == window_wrapper);
V8DOMWrapper::SetNativeInfo(GetIsolate(), window_wrapper, wrapper_type_info,
window);
}
} // namespace blink
// Copyright 2017 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 "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h"
#include "third_party/blink/renderer/bindings/core/v8/window_proxy.h"
#include "third_party/blink/renderer/core/dom/events/event_target.h"
#include "third_party/blink/renderer/core/dom/node.h"
#include "third_party/blink/renderer/core/frame/dom_window.h"
#include "third_party/blink/renderer/core/frame/frame.h"
#include "third_party/blink/renderer/platform/bindings/runtime_call_stats.h"
namespace blink {
v8::Local<v8::Value> ToV8(DOMWindow* window,
v8::Local<v8::Object> creation_context,
v8::Isolate* isolate) {
RUNTIME_CALL_TIMER_SCOPE(isolate,
RuntimeCallStats::CounterId::kToV8DOMWindow);
// Notice that we explicitly ignore creationContext because the DOMWindow
// has its own creationContext.
if (UNLIKELY(!window))
return v8::Null(isolate);
// TODO(yukishiino): Get understanding of why it's possible to initialize
// the context after the frame is detached. And then, remove the following
// lines. See also https://crbug.com/712638 .
Frame* frame = window->GetFrame();
if (!frame)
return v8::Local<v8::Object>();
// TODO(yukishiino): Make this function always return the non-empty handle
// even if the frame is detached because the global proxy must always exist
// per spec.
return frame->GetWindowProxy(DOMWrapperWorld::Current(isolate))
->GlobalProxyIfNotDetached();
}
v8::Local<v8::Value> ToV8(EventTarget* impl,
v8::Local<v8::Object> creation_context,
v8::Isolate* isolate) {
if (UNLIKELY(!impl))
return v8::Null(isolate);
if (impl->InterfaceName() == event_target_names::kWindow)
return ToV8(static_cast<DOMWindow*>(impl), creation_context, isolate);
return ToV8(static_cast<ScriptWrappable*>(impl), creation_context, isolate);
}
v8::Local<v8::Value> ToV8(Node* node,
v8::Local<v8::Object> creation_context,
v8::Isolate* isolate) {
return ToV8(static_cast<ScriptWrappable*>(node), creation_context, isolate);
}
} // namespace blink
......@@ -18,18 +18,6 @@
namespace blink {
class Dictionary;
class DOMWindow;
class EventTarget;
CORE_EXPORT v8::Local<v8::Value> ToV8(DOMWindow*,
v8::Local<v8::Object> creation_context,
v8::Isolate*);
CORE_EXPORT v8::Local<v8::Value> ToV8(EventTarget*,
v8::Local<v8::Object> creation_context,
v8::Isolate*);
CORE_EXPORT v8::Local<v8::Value> ToV8(Node* node,
v8::Local<v8::Object> creation_context,
v8::Isolate* isolate);
inline v8::Local<v8::Value> ToV8(const Dictionary& value,
v8::Local<v8::Object> creation_context,
......
......@@ -63,7 +63,6 @@ namespace blink {
// dependencies to core/.
class DOMWindow;
class EventTarget;
class ExceptionState;
class ExecutionContext;
class FlexibleArrayBufferView;
......@@ -82,88 +81,6 @@ enum class UnionTypeConversionMode {
kNotNullable,
};
template <typename CallbackInfo>
inline void V8SetReturnValue(const CallbackInfo& callback_info,
DOMWindow* impl) {
V8SetReturnValue(callback_info, ToV8(impl, callback_info.Holder(),
callback_info.GetIsolate()));
}
template <typename CallbackInfo>
inline void V8SetReturnValue(const CallbackInfo& callback_info,
EventTarget* impl) {
V8SetReturnValue(callback_info, ToV8(impl, callback_info.Holder(),
callback_info.GetIsolate()));
}
template <typename CallbackInfo>
inline void V8SetReturnValue(const CallbackInfo& callback_info, Node* impl) {
V8SetReturnValue(callback_info, static_cast<ScriptWrappable*>(impl));
}
template <typename CallbackInfo>
inline void V8SetReturnValueForMainWorld(const CallbackInfo& callback_info,
DOMWindow* impl) {
V8SetReturnValue(callback_info, ToV8(impl, callback_info.Holder(),
callback_info.GetIsolate()));
}
template <typename CallbackInfo>
inline void V8SetReturnValueForMainWorld(const CallbackInfo& callback_info,
EventTarget* impl) {
V8SetReturnValue(callback_info, ToV8(impl, callback_info.Holder(),
callback_info.GetIsolate()));
}
template <typename CallbackInfo>
inline void V8SetReturnValueForMainWorld(const CallbackInfo& callback_info,
Node* impl) {
// Since EventTarget has a special version of ToV8 and V8EventTarget.h
// defines its own v8SetReturnValue family, which are slow, we need to
// override them with optimized versions for Node and its subclasses.
// Without this overload, V8SetReturnValueForMainWorld for Node would be
// very slow.
//
// class hierarchy:
// ScriptWrappable <-- EventTarget <--+-- Node <-- ...
// +-- Window
// overloads:
// V8SetReturnValueForMainWorld(ScriptWrappable*)
// Optimized and very fast.
// V8SetReturnValueForMainWorld(EventTarget*)
// Uses custom ToV8 function and slow.
// V8SetReturnValueForMainWorld(Node*)
// Optimized and very fast.
// V8SetReturnValueForMainWorld(Window*)
// Uses custom ToV8 function and slow.
V8SetReturnValueForMainWorld(callback_info,
static_cast<ScriptWrappable*>(impl));
}
template <typename CallbackInfo>
inline void V8SetReturnValueFast(const CallbackInfo& callback_info,
DOMWindow* impl,
const ScriptWrappable*) {
V8SetReturnValue(callback_info, ToV8(impl, callback_info.Holder(),
callback_info.GetIsolate()));
}
template <typename CallbackInfo>
inline void V8SetReturnValueFast(const CallbackInfo& callback_info,
EventTarget* impl,
const ScriptWrappable*) {
V8SetReturnValue(callback_info, ToV8(impl, callback_info.Holder(),
callback_info.GetIsolate()));
}
template <typename CallbackInfo>
inline void V8SetReturnValueFast(const CallbackInfo& callback_info,
Node* impl,
const ScriptWrappable* wrappable) {
V8SetReturnValueFast(callback_info, static_cast<ScriptWrappable*>(impl),
wrappable);
}
template <typename CallbackInfo, typename T>
inline void V8SetReturnValue(const CallbackInfo& callbackInfo,
NotShared<T> notShared) {
......
......@@ -160,18 +160,4 @@ void WindowProxy::InitializeIfNeeded() {
}
}
v8::Local<v8::Object> WindowProxy::AssociateWithWrapper(
DOMWindow* window,
const WrapperTypeInfo* wrapper_type_info,
v8::Local<v8::Object> wrapper) {
if (world_->DomDataStore().Set(isolate_, window, wrapper_type_info,
wrapper)) {
WrapperTypeInfo::WrapperCreated();
V8DOMWrapper::SetNativeInfo(isolate_, wrapper, wrapper_type_info, window);
DCHECK(V8DOMWrapper::HasInternalFieldsSet(wrapper));
}
SECURITY_CHECK(ToScriptWrappable(wrapper) == window);
return wrapper;
}
} // namespace blink
......@@ -42,7 +42,6 @@ namespace blink {
class DOMWindow;
class Frame;
struct WrapperTypeInfo;
// WindowProxy implements the split window model of a window for a frame. In the
// HTML standard, the split window model is composed of the Window interface
......@@ -258,11 +257,6 @@ class WindowProxy : public GarbageCollected<WindowProxy> {
virtual void DisposeContext(Lifecycle next_status, FrameReuseStatus) = 0;
WARN_UNUSED_RESULT v8::Local<v8::Object> AssociateWithWrapper(
DOMWindow*,
const WrapperTypeInfo*,
v8::Local<v8::Object> wrapper);
v8::Isolate* GetIsolate() const { return isolate_; }
Frame* GetFrame() const { return frame_.Get(); }
......
......@@ -46,8 +46,19 @@ DOMWindow::~DOMWindow() {
v8::Local<v8::Value> DOMWindow::Wrap(v8::Isolate* isolate,
v8::Local<v8::Object> creation_context) {
NOTREACHED();
return v8::Local<v8::Value>();
// TODO(yukishiino): Get understanding of why it's possible to initialize
// the context after the frame is detached. And then, remove the following
// lines. See also https://crbug.com/712638 .
Frame* frame = GetFrame();
if (!frame)
return v8::Null(isolate);
// TODO(yukishiino): Make this function always return the non-empty handle
// even if the frame is detached because the global proxy must always exist
// per spec.
ScriptState* script_state = ScriptState::From(isolate->GetCurrentContext());
return frame->GetWindowProxy(script_state->World())
->GlobalProxyIfNotDetached();
}
v8::Local<v8::Object> DOMWindow::AssociateWithWrapper(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment