Commit 23d2ae42 authored by yukishiino's avatar yukishiino Committed by Commit bot

binding: Removes Document::wrap that must be equivalent to Node::wrap. (2nd try)

Removes unnecessary Document::wrap and associateWithWrapper.
The special code in these functions were made in order to keep
WindowProxy::m_document updated and consistent with toV8(document).
But we no longer rely on the hack, and we can remove the dead code.

WindowProxy::m_document is used to support named properties on the
document, and only used in WindowProxy::namedItem{Added,Removed}.
These functions are only called from ScriptController::namedItem
{Added,Removed} and ScriptController already guarantees that the
WindowProxy is initialized at once.  So, there is no need for
Document::wrap to call WindowProxy::updateDocumentProperty.

Plus, this CL removes WindowProxy::m_document.  Given that we knew
it's the main world, we should be able to retrieve the wrapper
object of the document in almost the same speed as before.  The
new code to retrieve the wrapper object should be almost equivalent
to
    ScriptWrappable::mainWorldWrapper(isolate)
and it's almost the same as
    m_document.newLocal(isolate)

The first attempt was: http://crrev.com/2525313004

BUG=

Review-Url: https://codereview.chromium.org/2542123002
Cr-Commit-Position: refs/heads/master@{#436243}
parent b887e7c4
...@@ -33,6 +33,7 @@ ...@@ -33,6 +33,7 @@
#include "bindings/core/v8/ConditionalFeatures.h" #include "bindings/core/v8/ConditionalFeatures.h"
#include "bindings/core/v8/DOMWrapperWorld.h" #include "bindings/core/v8/DOMWrapperWorld.h"
#include "bindings/core/v8/ScriptController.h" #include "bindings/core/v8/ScriptController.h"
#include "bindings/core/v8/ToV8.h"
#include "bindings/core/v8/V8Binding.h" #include "bindings/core/v8/V8Binding.h"
#include "bindings/core/v8/V8DOMActivityLogger.h" #include "bindings/core/v8/V8DOMActivityLogger.h"
#include "bindings/core/v8/V8Document.h" #include "bindings/core/v8/V8Document.h"
...@@ -73,11 +74,6 @@ ...@@ -73,11 +74,6 @@
namespace blink { namespace blink {
static void checkDocumentWrapper(v8::Local<v8::Object> wrapper,
Document* document) {
ASSERT(V8Document::toImpl(wrapper) == document);
}
WindowProxy* WindowProxy::create(v8::Isolate* isolate, WindowProxy* WindowProxy::create(v8::Isolate* isolate,
Frame* frame, Frame* frame,
DOMWrapperWorld& world) { DOMWrapperWorld& world) {
...@@ -115,8 +111,6 @@ void WindowProxy::disposeContext(GlobalDetachmentBehavior behavior) { ...@@ -115,8 +111,6 @@ void WindowProxy::disposeContext(GlobalDetachmentBehavior behavior) {
MainThreadDebugger::instance()->contextWillBeDestroyed(m_scriptState.get()); MainThreadDebugger::instance()->contextWillBeDestroyed(m_scriptState.get());
} }
m_document.clear();
if (behavior == DetachGlobal) { if (behavior == DetachGlobal) {
// Clean up state on the global proxy, which will be reused. // Clean up state on the global proxy, which will be reused.
if (!m_globalProxy.isEmpty()) { if (!m_globalProxy.isEmpty()) {
...@@ -424,35 +418,19 @@ bool WindowProxy::setupWindowPrototypeChain() { ...@@ -424,35 +418,19 @@ bool WindowProxy::setupWindowPrototypeChain() {
return true; return true;
} }
void WindowProxy::updateDocumentWrapper(v8::Local<v8::Object> wrapper) {
ASSERT(m_world->isMainWorld());
m_document.set(m_isolate, wrapper);
}
void WindowProxy::updateDocumentProperty() { void WindowProxy::updateDocumentProperty() {
if (!m_world->isMainWorld()) DCHECK(m_world->isMainWorld());
return;
if (m_frame->isRemoteFrame()) { if (m_frame->isRemoteFrame())
return; return;
}
ScriptState::Scope scope(m_scriptState.get()); ScriptState::Scope scope(m_scriptState.get());
v8::Local<v8::Context> context = m_scriptState->context(); v8::Local<v8::Context> context = m_scriptState->context();
LocalFrame* frame = toLocalFrame(m_frame); LocalFrame* frame = toLocalFrame(m_frame);
v8::Local<v8::Value> documentWrapper = v8::Local<v8::Value> documentWrapper =
toV8(frame->document(), context->Global(), context->GetIsolate()); toV8(frame->document(), context->Global(), m_isolate);
if (documentWrapper.IsEmpty()) DCHECK(documentWrapper->IsObject());
return; // Update the cached accessor for window.document.
ASSERT(documentWrapper == m_document.newLocal(m_isolate) ||
m_document.isEmpty());
if (m_document.isEmpty())
updateDocumentWrapper(v8::Local<v8::Object>::Cast(documentWrapper));
checkDocumentWrapper(m_document.newLocal(m_isolate), frame->document());
ASSERT(documentWrapper->IsObject());
// Update cached accessor.
CHECK(V8PrivateProperty::getWindowDocumentCachedAccessor(m_isolate).set( CHECK(V8PrivateProperty::getWindowDocumentCachedAccessor(m_isolate).set(
context, context->Global(), documentWrapper)); context, context->Global(), documentWrapper));
} }
...@@ -518,7 +496,7 @@ void WindowProxy::setSecurityToken(SecurityOrigin* origin) { ...@@ -518,7 +496,7 @@ void WindowProxy::setSecurityToken(SecurityOrigin* origin) {
} }
void WindowProxy::updateDocument() { void WindowProxy::updateDocument() {
ASSERT(m_world->isMainWorld()); DCHECK(m_world->isMainWorld());
if (!isGlobalInitialized()) if (!isGlobalInitialized())
return; return;
if (!isContextInitialized()) if (!isContextInitialized())
...@@ -577,22 +555,24 @@ static void getter(v8::Local<v8::Name> property, ...@@ -577,22 +555,24 @@ static void getter(v8::Local<v8::Name> property,
void WindowProxy::namedItemAdded(HTMLDocument* document, void WindowProxy::namedItemAdded(HTMLDocument* document,
const AtomicString& name) { const AtomicString& name) {
ASSERT(m_world->isMainWorld()); DCHECK(m_world->isMainWorld());
if (!isContextInitialized() || !m_scriptState->contextIsValid()) if (!isContextInitialized())
return; return;
ScriptState::Scope scope(m_scriptState.get()); ScriptState::Scope scope(m_scriptState.get());
ASSERT(!m_document.isEmpty()); v8::Local<v8::Object> documentWrapper =
v8::Local<v8::Context> context = m_scriptState->context(); m_world->domDataStore().get(document, m_isolate);
v8::Local<v8::Object> documentHandle = m_document.newLocal(m_isolate); // TODO(yukishiino,peria): We should check if the own property with the same
checkDocumentWrapper(documentHandle, document); // name already exists or not, and if it exists, we shouldn't define a new
documentHandle->SetAccessor(context, v8String(m_isolate, name), getter); // accessor property (it fails).
documentWrapper->SetAccessor(m_isolate->GetCurrentContext(),
v8String(m_isolate, name), getter);
} }
void WindowProxy::namedItemRemoved(HTMLDocument* document, void WindowProxy::namedItemRemoved(HTMLDocument* document,
const AtomicString& name) { const AtomicString& name) {
ASSERT(m_world->isMainWorld()); DCHECK(m_world->isMainWorld());
if (!isContextInitialized()) if (!isContextInitialized())
return; return;
...@@ -601,11 +581,11 @@ void WindowProxy::namedItemRemoved(HTMLDocument* document, ...@@ -601,11 +581,11 @@ void WindowProxy::namedItemRemoved(HTMLDocument* document,
return; return;
ScriptState::Scope scope(m_scriptState.get()); ScriptState::Scope scope(m_scriptState.get());
ASSERT(!m_document.isEmpty()); v8::Local<v8::Object> documentWrapper =
v8::Local<v8::Object> documentHandle = m_document.newLocal(m_isolate); m_world->domDataStore().get(document, m_isolate);
checkDocumentWrapper(documentHandle, document); documentWrapper
documentHandle->Delete(m_isolate->GetCurrentContext(), ->Delete(m_isolate->GetCurrentContext(), v8String(m_isolate, name))
v8String(m_isolate, name)); .ToChecked();
} }
void WindowProxy::updateSecurityOrigin(SecurityOrigin* origin) { void WindowProxy::updateSecurityOrigin(SecurityOrigin* origin) {
......
...@@ -121,7 +121,6 @@ class WindowProxy final : public GarbageCollectedFinalized<WindowProxy> { ...@@ -121,7 +121,6 @@ class WindowProxy final : public GarbageCollectedFinalized<WindowProxy> {
RefPtr<ScriptState> m_scriptState; RefPtr<ScriptState> m_scriptState;
RefPtr<DOMWrapperWorld> m_world; RefPtr<DOMWrapperWorld> m_world;
ScopedPersistent<v8::Object> m_globalProxy; ScopedPersistent<v8::Object> m_globalProxy;
ScopedPersistent<v8::Object> m_document;
}; };
} // namespace blink } // namespace blink
......
...@@ -6328,38 +6328,6 @@ void Document::platformColorsChanged() { ...@@ -6328,38 +6328,6 @@ void Document::platformColorsChanged() {
styleEngine().platformColorsChanged(); styleEngine().platformColorsChanged();
} }
v8::Local<v8::Object> Document::wrap(v8::Isolate* isolate,
v8::Local<v8::Object> creationContext) {
DCHECK(!DOMDataStore::containsWrapper(this, isolate));
const WrapperTypeInfo* wrapperType = wrapperTypeInfo();
if (frame() && frame()->script().initializeMainWorld()) {
// initializeMainWorld may have created a wrapper for the object, retry from
// the start.
v8::Local<v8::Object> wrapper = DOMDataStore::getWrapper(this, isolate);
if (!wrapper.IsEmpty())
return wrapper;
}
v8::Local<v8::Object> wrapper =
V8DOMWrapper::createWrapper(isolate, creationContext, wrapperType);
DCHECK(!wrapper.IsEmpty());
return associateWithWrapper(isolate, wrapperType, wrapper);
}
v8::Local<v8::Object> Document::associateWithWrapper(
v8::Isolate* isolate,
const WrapperTypeInfo* wrapperType,
v8::Local<v8::Object> wrapper) {
wrapper = V8DOMWrapper::associateObjectWithWrapper(isolate, this, wrapperType,
wrapper);
DOMWrapperWorld& world = DOMWrapperWorld::current(isolate);
if (world.isMainWorld() && frame())
frame()->script().windowProxy(world)->updateDocumentWrapper(wrapper);
return wrapper;
}
bool Document::isSecureContext( bool Document::isSecureContext(
String& errorMessage, String& errorMessage,
const SecureContextCheck privilegeContextCheck) const { const SecureContextCheck privilegeContextCheck) const {
......
...@@ -1240,13 +1240,6 @@ class CORE_EXPORT Document : public ContainerNode, ...@@ -1240,13 +1240,6 @@ class CORE_EXPORT Document : public ContainerNode,
DOMTimerCoordinator* timers() final; DOMTimerCoordinator* timers() final;
v8::Local<v8::Object> wrap(v8::Isolate*,
v8::Local<v8::Object> creationContext) override;
WARN_UNUSED_RESULT v8::Local<v8::Object> associateWithWrapper(
v8::Isolate*,
const WrapperTypeInfo*,
v8::Local<v8::Object> wrapper) override;
HostsUsingFeatures::Value& HostsUsingFeaturesValue() { HostsUsingFeatures::Value& HostsUsingFeaturesValue() {
return m_hostsUsingFeaturesValue; return m_hostsUsingFeaturesValue;
} }
......
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