2010-06-04 Anton Muhin <antonm@chromium.org>

        Reviewed by Nate Chapin.

        [Chromium] get rid of named interceptor on HTMLDocument and introduce/remove accessors when named items get deleted/removed
        https://bugs.webkit.org/show_bug.cgi?id=39877

        This patch makes callbacks invoked on named items addition/removal
        install API accessors and thus there is no more need in
        named and indexed interceptors on HTMLDocument which
        speeds up invocation of methods on document.

        * bindings/scripts/CodeGeneratorV8.pm:
        * bindings/v8/ScriptController.cpp:
        (WebCore::ScriptController::namedItemAdded):
        (WebCore::ScriptController::namedItemRemoved):
        * bindings/v8/V8DOMWindowShell.cpp:
        (WebCore::checkDocumentWrapper):
        (WebCore::V8DOMWindowShell::updateDocumentWrapperCache):
        (WebCore::getter):
        (WebCore::V8DOMWindowShell::namedItemAdded):
        (WebCore::V8DOMWindowShell::namedItemRemoved):
        * bindings/v8/V8DOMWindowShell.h:
        * bindings/v8/V8DOMWrapper.cpp:
        (WebCore::V8DOMWrapper::instantiateV8Object):
        * bindings/v8/custom/V8HTMLDocumentCustom.cpp:
        (WebCore::V8HTMLDocument::WrapInShadowObject):
        (WebCore::V8HTMLDocument::GetNamedProperty):
        (WebCore::V8HTMLDocument::allAccessorSetter):
        (WebCore::toV8):

Canonical link: https://commits.webkit.org/51694@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@60670 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog

@@ -1,3 +1,34 @@
+2010-06-04  Anton Muhin  <antonm@chromium.org>
+
+        Reviewed by Nate Chapin.
+
+        [Chromium] get rid of named interceptor on HTMLDocument and introduce/remove accessors when named items get deleted/removed
+        https://bugs.webkit.org/show_bug.cgi?id=39877
+
+        This patch makes callbacks invoked on named items addition/removal
+        install API accessors and thus there is no more need in
+        named and indexed interceptors on HTMLDocument which
+        speeds up invocation of methods on document.
+
+        * bindings/scripts/CodeGeneratorV8.pm:
+        * bindings/v8/ScriptController.cpp:
+        (WebCore::ScriptController::namedItemAdded):
+        (WebCore::ScriptController::namedItemRemoved):
+        * bindings/v8/V8DOMWindowShell.cpp:
+        (WebCore::checkDocumentWrapper):
+        (WebCore::V8DOMWindowShell::updateDocumentWrapperCache):
+        (WebCore::getter):
+        (WebCore::V8DOMWindowShell::namedItemAdded):
+        (WebCore::V8DOMWindowShell::namedItemRemoved):
+        * bindings/v8/V8DOMWindowShell.h:
+        * bindings/v8/V8DOMWrapper.cpp:
+        (WebCore::V8DOMWrapper::instantiateV8Object):
+        * bindings/v8/custom/V8HTMLDocumentCustom.cpp:
+        (WebCore::V8HTMLDocument::WrapInShadowObject):
+        (WebCore::V8HTMLDocument::GetNamedProperty):
+        (WebCore::V8HTMLDocument::allAccessorSetter):
+        (WebCore::toV8):
+
 2010-06-04  Kwang Yul Seo  <skyul@company100.net>
 
         Reviewed by Kent Tamura.

WebCore/bindings/scripts/CodeGeneratorV8.pm

@@ -273,6 +273,13 @@ END
 END
     }
 
+    if ($implClassName eq "HTMLDocument") {
+      push(@headerContent, <<END);
+  static v8::Local<v8::Object> WrapInShadowObject(v8::Local<v8::Object> wrapper, Node* impl);
+  static v8::Handle<v8::Value> GetNamedProperty(HTMLDocument* htmlDocument, const AtomicString& key);
+END
+    }
+
     my @enabledAtRuntime;
     foreach my $function (@{$dataNode->functions}) {
         my $name = $function->signature->name;
@@ -360,9 +367,6 @@ sub GetInternalFields
 
     if (IsSubType($dataNode, "Document")) {
         push(@customInternalFields, "implementationIndex");
-        if ($name eq "HTMLDocument") {
-            push(@customInternalFields, ("markerIndex", "shadowIndex"));
-        }
     } elsif ($name eq "DOMWindow") {
         push(@customInternalFields, "enteredIsolatedWorldIndex");
     }
@@ -399,7 +403,6 @@ END
 my %indexerSpecialCases = (
     "Storage" => 1,
     "HTMLAppletElement" => 1,
-    "HTMLDocument" => 1,
     "HTMLEmbedElement" => 1,
     "HTMLObjectElement" => 1
 );
@@ -426,6 +429,10 @@ sub GenerateHeaderNamedAndIndexedPropertyAccessors
     if ($interfaceName eq "HTMLSelectElement" || $interfaceName eq "HTMLAppletElement" || $interfaceName eq "HTMLEmbedElement" || $interfaceName eq "HTMLObjectElement") {
         $hasCustomNamedGetter = 1;
     }
+    if ($interfaceName eq "HTMLDocument") {
+        $hasCustomNamedGetter = 0;
+        $hasCustomIndexedGetter = 0;
+    }
     my $isIndexerSpecialCase = exists $indexerSpecialCases{$interfaceName};
 
     if ($hasCustomIndexedGetter || $isIndexerSpecialCase) {
@@ -454,7 +461,7 @@ END
     static v8::Handle<v8::Value> namedPropertySetter(v8::Local<v8::String>, v8::Local<v8::Value>, const v8::AccessorInfo&);
 END
     }
-    if ($hasCustomDeleters || $interfaceName eq "HTMLDocument") {
+    if ($hasCustomDeleters) {
         push(@headerContent, <<END);
     static v8::Handle<v8::Boolean> namedPropertyDeleter(v8::Local<v8::String>, const v8::AccessorInfo&);
 END
@@ -1505,6 +1512,10 @@ sub GenerateImplementationNamedPropertyGetter
         $hasCustomGetter = 1;
     }
 
+    if ($interfaceName eq "HTMLDocument") {
+        $hasCustomGetter = 0;
+    }
+
     my $hasGetter = $dataNode->extendedAttributes->{"HasNameGetter"} || $hasCustomGetter || $namedPropertyGetter;
     if (!$hasGetter) {
         return;
@@ -1520,8 +1531,7 @@ END
     }
 
     my $hasSetter = $dataNode->extendedAttributes->{"DelegatingPutFunction"};
-    # FIXME: Try to remove hard-coded HTMLDocument reference by aligning handling of document.all with JSC bindings.
-    my $hasDeleter = $dataNode->extendedAttributes->{"CustomDeleteProperty"} || $interfaceName eq "HTMLDocument";
+    my $hasDeleter = $dataNode->extendedAttributes->{"CustomDeleteProperty"};
     my $hasEnumerator = $dataNode->extendedAttributes->{"CustomGetPropertyNames"};
     my $setOn = "Instance";
 
@@ -2003,6 +2013,11 @@ END
     // When a context is detached from a frame, turn on the access check.
     // Turning on checks also invalidates inline caches of the object.
     instance->SetAccessCheckCallbacks(V8DOMWindow::namedSecurityCheck, V8DOMWindow::indexedSecurityCheck, v8::External::Wrap(&V8DOMWindow::info), false);
+END
+    }
+    if ($interfaceName eq "HTMLDocument") {
+        push(@implContent, <<END);
+    desc->SetHiddenPrototype(true);
 END
     }
     if ($interfaceName eq "Location") {

WebCore/bindings/v8/ScriptController.cpp

@@ -456,10 +456,12 @@ void ScriptController::updateDocument()
 
 void ScriptController::namedItemAdded(HTMLDocument* doc, const AtomicString& name)
 {
+    m_proxy->windowShell()->namedItemAdded(doc, name);
 }
 
 void ScriptController::namedItemRemoved(HTMLDocument* doc, const AtomicString& name)
 {
+    m_proxy->windowShell()->namedItemRemoved(doc, name);
 }
 
 } // namespace WebCore

WebCore/bindings/v8/V8DOMWindowShell.cpp

@@ -50,6 +50,7 @@
 #include "V8DOMWindow.h"
 #include "V8Document.h"
 #include "V8GCForContextDispose.h"
+#include "V8HTMLDocument.h"
 #include "V8HiddenPropertyName.h"
 #include "V8History.h"
 #include "V8Location.h"
@@ -402,6 +403,12 @@ void V8DOMWindowShell::clearDocumentWrapper()
     }
 }
 
+static void checkDocumentWrapper(v8::Handle<v8::Object> wrapper, Document* document)
+{
+    ASSERT(V8Document::toNative(wrapper) == document);
+    ASSERT(!document->isHTMLDocument() || (V8Document::toNative(v8::Handle<v8::Object>::Cast(wrapper->GetPrototype())) == document));
+}
+
 void V8DOMWindowShell::updateDocumentWrapperCache()
 {
     v8::HandleScope handleScope;
@@ -420,6 +427,10 @@ void V8DOMWindowShell::updateDocumentWrapperCache()
     }
 
     v8::Handle<v8::Value> documentWrapper = toV8(m_frame->document());
+    ASSERT(documentWrapper == m_document || m_document.IsEmpty());
+    if (m_document.IsEmpty())
+        updateDocumentWrapper(v8::Handle<v8::Object>::Cast(documentWrapper));
+    checkDocumentWrapper(m_document, m_frame->document());
 
     // If instantiation of the document wrapper fails, clear the cache
     // and let the DOMWindow accessor handle access to the document.
@@ -497,6 +508,39 @@ void V8DOMWindowShell::updateDocument()
     updateSecurityOrigin();
 }
 
+v8::Handle<v8::Value> getter(v8::Local<v8::String> property, const v8::AccessorInfo& info)
+{
+    // FIXME(antonm): consider passing AtomicStringImpl directly.
+    AtomicString name = v8StringToAtomicWebCoreString(property);
+    HTMLDocument* htmlDocument = V8HTMLDocument::toNative(info.Holder());
+    ASSERT(htmlDocument);
+    return V8HTMLDocument::GetNamedProperty(htmlDocument, name);
+}
+
+void V8DOMWindowShell::namedItemAdded(HTMLDocument* doc, const AtomicString& name)
+{
+    initContextIfNeeded();
+
+    v8::HandleScope handleScope;
+    v8::Context::Scope contextScope(m_context);
+
+    ASSERT(!m_document.IsEmpty());
+    checkDocumentWrapper(m_document, doc);
+    m_document->SetAccessor(v8String(name), getter);
+}
+
+void V8DOMWindowShell::namedItemRemoved(HTMLDocument* doc, const AtomicString& name)
+{
+    initContextIfNeeded();
+
+    v8::HandleScope handleScope;
+    v8::Context::Scope contextScope(m_context);
+
+    ASSERT(!m_document.IsEmpty());
+    checkDocumentWrapper(m_document, doc);
+    m_document->Delete(v8String(name));
+}
+
 void V8DOMWindowShell::updateSecurityOrigin()
 {
     v8::HandleScope scope;

WebCore/bindings/v8/V8DOMWindowShell.h

@@ -31,6 +31,7 @@
 #ifndef V8DOMWindowShell_h
 #define V8DOMWindowShell_h
 
+#include "AtomicString.h"
 #include "WrapperTypeInfo.h"
 #include <wtf/HashMap.h>
 #include <wtf/PassRefPtr.h>
@@ -41,6 +42,7 @@ namespace WebCore {
 
 class DOMWindow;
 class Frame;
+class HTMLDocument;
 class String;
 
 // V8WindowShell represents all the per-global object state for a Frame that
@@ -54,6 +56,9 @@ class V8DOMWindowShell : public RefCounted<V8DOMWindowShell> {
     // Update document object of the frame.
     void updateDocument();
 
+    void namedItemAdded(HTMLDocument*, const AtomicString&);
+    void namedItemRemoved(HTMLDocument*, const AtomicString&);
+
     // Update the security origin of a document
     // (e.g., after setting docoument.domain).
     void updateSecurityOrigin();

WebCore/bindings/v8/V8DOMWrapper.cpp

@@ -286,6 +286,8 @@ v8::Local<v8::Object> V8DOMWrapper::instantiateV8Object(V8Proxy* proxy, WrapperT
     if (!instance.IsEmpty()) {
         // Avoid setting the DOM wrapper for failed allocations.
         setDOMWrapper(instance, type, impl);
+        if (type == &V8HTMLDocument::info)
+            instance = V8HTMLDocument::WrapInShadowObject(instance, static_cast<Node*>(impl));
     }
     return instance;
 }

WebCore/bindings/v8/custom/V8HTMLDocumentCustom.cpp

@@ -49,48 +49,37 @@
 
 namespace WebCore {
 
-v8::Handle<v8::Boolean> V8HTMLDocument::namedPropertyDeleter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
+v8::Local<v8::Object> V8HTMLDocument::WrapInShadowObject(v8::Local<v8::Object> wrapper, Node* impl)
 {
-    // Only handle document.all.  Insert the marker object into the
-    // shadow internal field to signal that document.all is no longer
-    // shadowed.
-    AtomicString key = v8StringToAtomicWebCoreString(name);
-    DEFINE_STATIC_LOCAL(const AtomicString, all, ("all"));
-    if (key != all)
-        return deletionNotHandledByInterceptor();
-
-    ASSERT(info.Holder()->InternalFieldCount() == V8HTMLDocument::internalFieldCount);
-    v8::Local<v8::Value> marker = info.Holder()->GetInternalField(V8HTMLDocument::markerIndex);
-    info.Holder()->SetInternalField(V8HTMLDocument::shadowIndex, marker);
-    return v8::True();
-}
-
-v8::Handle<v8::Value> V8HTMLDocument::namedPropertyGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
-{
-    INC_STATS("DOM.HTMLDocument.NamedPropertyGetter");
-    AtomicString key = v8StringToAtomicWebCoreString(name);
-
-    // Special case for document.all.  If the value in the shadow
-    // internal field is not the marker object, then document.all has
-    // been temporarily shadowed and we return the value.
-    DEFINE_STATIC_LOCAL(const AtomicString, all, ("all"));
-    if (key == all) {
-        ASSERT(info.Holder()->InternalFieldCount() == V8HTMLDocument::internalFieldCount);
-        v8::Local<v8::Value> marker = info.Holder()->GetInternalField(V8HTMLDocument::markerIndex);
-        v8::Local<v8::Value> value = info.Holder()->GetInternalField(V8HTMLDocument::shadowIndex);
-        if (marker != value)
-            return value;
+    DEFINE_STATIC_LOCAL(v8::Persistent<v8::Function>, shadowConstructor, ());
+    if (shadowConstructor.IsEmpty()) {
+        v8::Local<v8::FunctionTemplate> shadowTemplate = v8::FunctionTemplate::New();
+        if (shadowTemplate.IsEmpty())
+            return v8::Local<v8::Object>();
+        shadowTemplate->SetClassName(v8::String::New("HTMLDocument"));
+        shadowTemplate->Inherit(V8HTMLDocument::GetTemplate());
+        shadowTemplate->InstanceTemplate()->SetInternalFieldCount(V8HTMLDocument::internalFieldCount);
+        shadowConstructor = v8::Persistent<v8::Function>::New(shadowTemplate->GetFunction());
+        if (shadowConstructor.IsEmpty())
+            return v8::Local<v8::Object>();
     }
 
-    HTMLDocument* htmlDocument = V8HTMLDocument::toNative(info.Holder());
+    ASSERT(!shadowConstructor.IsEmpty());
+    v8::Local<v8::Object> shadow = shadowConstructor->NewInstance();
+    if (shadow.IsEmpty())
+        return v8::Local<v8::Object>();
+    V8DOMWrapper::setDOMWrapper(shadow, &V8HTMLDocument::info, impl);
+    shadow->SetPrototype(wrapper);
+    return shadow;
+}
 
-    // Fast case for named elements that are not there.
-    if (!htmlDocument->hasNamedItem(key.impl()) && !htmlDocument->hasExtraNamedItem(key.impl()))
-        return v8::Handle<v8::Value>();
+v8::Handle<v8::Value> V8HTMLDocument::GetNamedProperty(HTMLDocument* htmlDocument, const AtomicString& key)
+{
+    ASSERT(htmlDocument->hasNamedItem(key.impl()) || htmlDocument->hasExtraNamedItem(key.impl()));
 
     RefPtr<HTMLCollection> items = htmlDocument->documentNamedItems(key);
     if (!items->length())
-        return notHandledByInterceptor();
+        return v8::Handle<v8::Value>();
 
     if (items->length() == 1) {
         Node* node = items->firstItem();
@@ -104,13 +93,6 @@ v8::Handle<v8::Value> V8HTMLDocument::namedPropertyGetter(v8::Local<v8::String>
     return toV8(items.release());
 }
 
-v8::Handle<v8::Value> V8HTMLDocument::indexedPropertyGetter(uint32_t index, const v8::AccessorInfo &info) 
-{
-    INC_STATS("DOM.HTMLDocument.IndexedPropertyGetter");
-    v8::Local<v8::Integer> indexV8 = v8::Integer::NewFromUnsigned(index);
-    return namedPropertyGetter(indexV8->ToString(), info);
-}
-
 // HTMLDocument ----------------------------------------------------------------
 
 // Concatenates "args" to a string. If args is empty, returns empty string.
@@ -193,10 +175,8 @@ v8::Handle<v8::Value> V8HTMLDocument::allAccessorGetter(v8::Local<v8::String> na
 
 void V8HTMLDocument::allAccessorSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
 {
-    INC_STATS("DOM.HTMLDocument.all._set");
-    v8::Handle<v8::Object> holder = info.Holder();
-    ASSERT(info.Holder()->InternalFieldCount() == V8HTMLDocument::internalFieldCount);
-    info.Holder()->SetInternalField(V8HTMLDocument::shadowIndex, value);
+    // Just emulate a normal JS behaviour---install a property on this.
+    info.This()->ForceSet(name, value);
 }
 
 v8::Handle<v8::Value> toV8(HTMLDocument* impl, bool forceNewObject)
@@ -210,12 +190,6 @@ v8::Handle<v8::Value> toV8(HTMLDocument* impl, bool forceNewObject)
         if (V8Proxy* proxy = V8Proxy::retrieve(impl->frame()))
             proxy->windowShell()->updateDocumentWrapper(wrapper);
     }
-    // Create marker object and insert it in two internal fields.
-    // This is used to implement temporary shadowing of document.all.
-    ASSERT(wrapper->InternalFieldCount() == V8HTMLDocument::internalFieldCount);
-    v8::Local<v8::Object> marker = v8::Object::New();
-    wrapper->SetInternalField(V8HTMLDocument::markerIndex, marker);
-    wrapper->SetInternalField(V8HTMLDocument::shadowIndex, marker);
     return wrapper;
 }