Changeset 125251 in webkit


Ignore:
Timestamp:
Aug 9, 2012 10:12:51 PM (12 years ago)
Author:
dominicc@chromium.org
Message:

SVGElementInstance should have EventTarget on the prototype chain
​https://bugs.webkit.org/show_bug.cgi?id=88232

Reviewed by Adam Barth.

SVG 1.1 specifies that SVGElementInstance has EventTarget as its
parent interface:
<​http://www.w3.org/TR/SVG/struct.html#InterfaceSVGElementInstance>
Match the spec by putting EventTarget on the prototype chain of
SVGElementInstance instead of redundantly declaring
addEventListener, removeEventListener and dispatchEvent on the
SVGElementInstance interface. This is an incremental step to make
all EventTargets do it this way, being tracked in bug 67312.

Covered by existing tests, eg
svg/custom/use-instanceRoot-as-event-target.xhtml

  • CMakeLists.txt: Finding base interface types requires

searching directories with the IDLs of base interfaces.

  • DerivedSources.cpp: Add generated JSEventTarget.cpp.
  • DerivedSources.make: (Search paths again -- see CMakeLists.txt)
  • DerivedSources.pri: "
  • bindings/js/JSEventTargetCustom.cpp:

(WebCore::toEventTarget): Try to unwrap EventTargets simply as
EventTargets. When all EventTargets do this consistently this
function will be simplified.

  • bindings/js/JSSVGElementInstanceCustom.cpp:

(WebCore::JSSVGElementInstance::visitChildren): SVGElementInstance
skips walking its event listener list because it forwards
listeners to its corresponding element.

  • bindings/scripts/CodeGenerator.pm:

(IsStrictSubtype): For finding what is an EventTarget based on
parent interface.

  • bindings/scripts/CodeGeneratorJS.pm:

(GenerateHeader): Extend the heuristic for what is an
EventTarget. When all EventTargets are handled consistently this
will be simplified.
(GenerateImplementation):

  • bindings/scripts/CodeGeneratorV8.pm: Extend the heuristic for

what is an EventTarget, and remove some of the special-casing for
EventTarget.
(GenerateHeader):
(GetInternalFields):
(GenerateImplementation):
(BaseInterfaceName):
(GenerateToV8Converters):
(GetNativeType):
(JSValueToNative):
(GetV8HeaderName):

  • bindings/scripts/test/V8/V8Float64Array.cpp:

(WebCore::V8Float64Array::wrapSlow): Add assertions.

  • bindings/scripts/test/V8/V8TestNode.cpp:

(WebCore::V8TestNode::wrapSlow):

  • svg/SVGElementInstance.h: Must extend EventTarget first so that

static_cast<EventTarget*>(elementInstance) is the same pointer as
elementInstance, similar to how static_cast<Node*>(element) is the
same pointer as element.

  • svg/SVGElementInstance.idl: Extend EventTarget; no longer need

to declare add/removeEventListener and dispatchEvent.

Location:
trunk/Source/WebCore
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/CMakeLists.txt

    r125126 r125251  
    108108
    109109SET(WebCore_IDL_INCLUDES
     110    Modules/battery
    110111    Modules/filesystem
    111112    Modules/gamepad
    112113    Modules/geolocation
    113114    Modules/indexeddb
     115    Modules/intents
     116    Modules/mediastream
     117    Modules/networkinfo
    114118    Modules/quota
     119    Modules/speech
     120    Modules/vibration
    115121    Modules/webaudio
    116122    Modules/webdatabase
    … …  
    122128    html
    123129    html/canvas
     130    html/shadow
     131    html/track
    124132    inspector
    125133    loader/appcache
     134    notifications
    126135    page
    127136    plugins

  • trunk/Source/WebCore/ChangeLog

    r125249 r125251  
     12012-08-02  Dominic Cooney  <dominicc@chromium.org>
     2
     3        SVGElementInstance should have EventTarget on the prototype chain
     4        https://bugs.webkit.org/show_bug.cgi?id=88232
     5
     6        Reviewed by Adam Barth.
     7
     8        SVG 1.1 specifies that SVGElementInstance has EventTarget as its
     9        parent interface:
     10        <http://www.w3.org/TR/SVG/struct.html#InterfaceSVGElementInstance>
     11        Match the spec by putting EventTarget on the prototype chain of
     12        SVGElementInstance instead of redundantly declaring
     13        addEventListener, removeEventListener and dispatchEvent on the
     14        SVGElementInstance interface. This is an incremental step to make
     15        all EventTargets do it this way, being tracked in bug 67312.
     16
     17        Covered by existing tests, eg
     18        svg/custom/use-instanceRoot-as-event-target.xhtml
     19
     20        * CMakeLists.txt: Finding base interface types requires
     21        searching directories with the IDLs of base interfaces.
     22        * DerivedSources.cpp: Add generated JSEventTarget.cpp.
     23        * DerivedSources.make: (Search paths again -- see CMakeLists.txt)
     24        * DerivedSources.pri: "
     25        * bindings/js/JSEventTargetCustom.cpp:
     26        (WebCore::toEventTarget): Try to unwrap EventTargets simply as
     27        EventTargets. When all EventTargets do this consistently this
     28        function will be simplified.
     29        * bindings/js/JSSVGElementInstanceCustom.cpp:
     30        (WebCore::JSSVGElementInstance::visitChildren): SVGElementInstance
     31        skips walking its event listener list because it forwards
     32        listeners to its corresponding element.
     33        * bindings/scripts/CodeGenerator.pm:
     34        (IsStrictSubtype): For finding what is an EventTarget based on
     35        parent interface.
     36        * bindings/scripts/CodeGeneratorJS.pm:
     37        (GenerateHeader): Extend the heuristic for what is an
     38        EventTarget. When all EventTargets are handled consistently this
     39        will be simplified.
     40        (GenerateImplementation):
     41        * bindings/scripts/CodeGeneratorV8.pm: Extend the heuristic for
     42        what is an EventTarget, and remove some of the special-casing for
     43        EventTarget.
     44        (GenerateHeader):
     45        (GetInternalFields):
     46        (GenerateImplementation):
     47        (BaseInterfaceName):
     48        (GenerateToV8Converters):
     49        (GetNativeType):
     50        (JSValueToNative):
     51        (GetV8HeaderName):
     52        * bindings/scripts/test/V8/V8Float64Array.cpp:
     53        (WebCore::V8Float64Array::wrapSlow): Add assertions.
     54        * bindings/scripts/test/V8/V8TestNode.cpp:
     55        (WebCore::V8TestNode::wrapSlow):
     56        * svg/SVGElementInstance.h: Must extend EventTarget first so that
     57        static_cast<EventTarget*>(elementInstance) is the same pointer as
     58        elementInstance, similar to how static_cast<Node*>(element) is the
     59        same pointer as element.
     60        * svg/SVGElementInstance.idl: Extend EventTarget; no longer need
     61        to declare add/removeEventListener and dispatchEvent.
     62
    1632012-08-09  Vivek Galatage  <vivekgalatage@gmail.com>
    264

  • trunk/Source/WebCore/DerivedSources.cpp

    r124953 r125251  
    117117#include "JSEventException.cpp"
    118118#include "JSEventSource.cpp"
     119#include "JSEventTarget.cpp"
    119120#include "JSFile.cpp"
    120121#include "JSFileCallback.cpp"

  • trunk/Source/WebCore/DerivedSources.make

    r124953 r125251  
    958958
    959959IDL_INCLUDES = \
     960    $(WebCore)/Modules/battery \
     961    $(WebCore)/Modules/filesystem \
     962    $(WebCore)/Modules/gamepad \
     963    $(WebCore)/Modules/geolocation \
     964    $(WebCore)/Modules/indexeddb \
     965    $(WebCore)/Modules/intents \
     966    $(WebCore)/Modules/mediasource \
     967    $(WebCore)/Modules/mediastream \
     968    $(WebCore)/Modules/networkinfo \
     969    $(WebCore)/Modules/notifications \
     970    $(WebCore)/Modules/speech \
     971    $(WebCore)/Modules/vibration \
     972    $(WebCore)/Modules/webaudio \
     973    $(WebCore)/Modules/webdatabase \
     974    $(WebCore)/Modules/websockets \
     975    $(WebCore)/css \
    960976    $(WebCore)/dom \
    961977    $(WebCore)/fileapi \
    962978    $(WebCore)/html \
    963     $(WebCore)/css \
    964     $(WebCore)/Modules/mediasource \
    965     $(WebCore)/Modules/notifications \
     979    $(WebCore)/html/canvas \
     980    $(WebCore)/html/shadow \
     981    $(WebCore)/html/track \
     982    $(WebCore)/inspector \
     983    $(WebCore)/loader/appcache \
    966984    $(WebCore)/page \
    967     $(WebCore)/xml \
    968     $(WebCore)/svg
     985    $(WebCore)/plugins \
     986    $(WebCore)/storage \
     987    $(WebCore)/svg \
     988    $(WebCore)/workers \
     989    $(WebCore)/xml
    969990
    970991IDL_COMMON_ARGS = $(IDL_INCLUDES:%=--include %) --write-dependencies --outputDir .

  • trunk/Source/WebCore/DerivedSources.pri

    r124953 r125251  
    715715                            --include $$PWD/Modules/geolocation \
    716716                            --include $$PWD/Modules/indexeddb \
     717                            --include $$PWD/Modules/mediasource \
     718                            --include $$PWD/Modules/notifications \
    717719                            --include $$PWD/Modules/quota \
    718720                            --include $$PWD/Modules/webaudio \
    719721                            --include $$PWD/Modules/webdatabase \
    720722                            --include $$PWD/Modules/websockets \
     723                            --include $$PWD/css \
    721724                            --include $$PWD/dom \
     725                            --include $$PWD/editing \
    722726                            --include $$PWD/fileapi \
    723727                            --include $$PWD/html \
     728                            --include $$PWD/html/canvas \
     729                            --include $$PWD/html/shadow \
     730                            --include $$PWD/html/track \
     731                            --include $$PWD/inspector \
     732                            --include $$PWD/loader/appcache \
     733                            --include $$PWD/page \
     734                            --include $$PWD/plugins \
     735                            --include $$PWD/storage \
     736                            --include $$PWD/svg \
     737                            --include $$PWD/testing \
     738                            --include $$PWD/websockets \
     739                            --include $$PWD/workers \
    724740                            --include $$PWD/xml \
    725                             --include $$PWD/svg \
    726                             --include $$PWD/storage \
    727                             --include $$PWD/css \
    728                             --include $$PWD/testing \
    729                             --include $$PWD/workers \
    730741                            --outputDir ${QMAKE_FUNC_FILE_OUT_PATH} \
    731742                            --supplementalDependencyFile ${QMAKE_FUNC_FILE_OUT_PATH}/$$SUPPLEMENTAL_DEPENDENCY_FILE \

  • trunk/Source/WebCore/bindings/js/JSEventTargetCustom.cpp

    r119367 r125251  
    7474        return jsCast<JSDOMWindowShell*>(asObject(value))->impl();
    7575
     76    TRY_TO_UNWRAP_WITH_INTERFACE(EventTarget)
     77    // FIXME: Remove this once all event targets extend EventTarget
    7678    DOM_EVENT_TARGET_INTERFACES_FOR_EACH(TRY_TO_UNWRAP_WITH_INTERFACE)
    7779    return 0;

  • trunk/Source/WebCore/bindings/js/JSSVGElementInstanceCustom.cpp

    r100006 r125251  
    3131#include "JSSVGElementInstance.h"
    3232
    33 #include "JSNode.h"
    34 #include "SVGElementInstance.h"
     33#include "JSEventTarget.h"
    3534
    3635namespace WebCore {
    … …  
    4241    COMPILE_ASSERT(StructureFlags & JSC::OverridesVisitChildren, OverridesVisitChildrenWithoutSettingFlag);
    4342    ASSERT(thisObject->structure()->typeInfo().overridesVisitChildren());
    44     Base::visitChildren(thisObject, visitor);
     43    // Skip JSEventTarget::visitChildren because event listener registration is
     44    // forwarded to the corresponding element.
     45    JSEventTarget::Base::visitChildren(thisObject, visitor);
    4546    visitor.addOpaqueRoot(root(thisObject->impl()->correspondingElement()));
    4647}

  • trunk/Source/WebCore/bindings/scripts/CodeGenerator.pm

    r120206 r125251  
    709709}
    710710
     711sub IsStrictSubtype
     712{
     713    my $object = shift;
     714    my $dataNode = shift;
     715    my $interfaceName = shift;
     716    my $found = 0;
     717
     718    $object->ForAllParents($dataNode, sub {
     719        my $interface = shift;
     720        if ($object->StripModule($interface->name) eq $interfaceName) {
     721            $found = 1;
     722        }
     723        return "prune" if $found;
     724    }, 0, 1);
     725
     726    return $found;
     727}
     728
    7117291;

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r125152 r125251  
    679679    my $hasParent = $hasLegacyParent || $hasRealParent;
    680680    my $parentClassName = GetParentClassName($dataNode);
    681     my $eventTarget = $dataNode->extendedAttributes->{"EventTarget"};
    682     my $needsMarkChildren = $dataNode->extendedAttributes->{"JSCustomMarkFunction"} || $dataNode->extendedAttributes->{"EventTarget"};
     681    my $needsMarkChildren = $dataNode->extendedAttributes->{"JSCustomMarkFunction"} || $dataNode->extendedAttributes->{"EventTarget"} || $dataNode->name eq "EventTarget";
    683682
    684683    # - Add default header template and header protection
    … …  
    13761375    my $parentClassName = GetParentClassName($dataNode);
    13771376    my $visibleInterfaceName = $codeGenerator->GetVisibleInterfaceName($dataNode);
    1378     my $eventTarget = $dataNode->extendedAttributes->{"EventTarget"};
    1379     my $needsMarkChildren = $dataNode->extendedAttributes->{"JSCustomMarkFunction"} || $dataNode->extendedAttributes->{"EventTarget"};
     1377    my $eventTarget = $dataNode->extendedAttributes->{"EventTarget"} || $codeGenerator->IsStrictSubtype($dataNode, "EventTarget");
     1378    my $needsMarkChildren = $dataNode->extendedAttributes->{"JSCustomMarkFunction"} || $dataNode->extendedAttributes->{"EventTarget"} || $dataNode->name eq "EventTarget";
    13801379
    13811380    # - Add default header template
    … …  
    22762275            push(@implContent, "    ASSERT(thisObject->structure()->typeInfo().overridesVisitChildren());\n");
    22772276            push(@implContent, "    Base::visitChildren(thisObject, visitor);\n");
    2278             if ($dataNode->extendedAttributes->{"EventTarget"}) {
     2277            if ($dataNode->extendedAttributes->{"EventTarget"} || $dataNode->name eq "EventTarget") {
    22792278                push(@implContent, "    thisObject->impl()->visitJSEventListeners(visitor);\n");
    22802279            }

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm

    r125232 r125251  
    290290    my $implClassName = $interfaceName;
    291291
    292     # Copy contents of parent classes except the first parent or if it is
    293     # EventTarget.
     292    # Copy contents of parent classes except the first parent.
    294293    $codeGenerator->AddMethodsConstantsAndAttributesFromParentClasses($dataNode, \@allParents, 1);
    295294    $codeGenerator->LinkOverloadedFunctions($dataNode);
    … …  
    300299        foreach (@{$dataNode->parents}) {
    301300            my $parent = $codeGenerator->StripModule($_);
    302             next if $parent eq "EventTarget";
    303301            $headerIncludes{"V8${parent}.h"} = 1;
    304302        }
    … …  
    366364        foreach (@{$dataNode->parents}) {
    367365            my $parent = $codeGenerator->StripModule($_);
    368             next if $parent eq "EventTarget";
    369366            $headerIncludes{"V8${parent}.h"} = 1;
    370367            push(@headerContent, "${separator}V8${parent}::hasDependentLifetime");
    … …  
    567564    # so special-case AbstractWorker and WorkerContext to include all sub-types.
    568565    # Event listeners on DOM nodes are explicitly supported in the GC controller.
    569     # FIXME: SVGElementInstance should probably have the EventTarget extended attribute, but doesn't.
    570566    # FIXME: Simplify this when all EventTargets are subtypes of EventTarget.
    571567    if (!IsNodeSubType($dataNode)
    … …  
    573569            || $dataNode->extendedAttributes->{"IsWorkerContext"}
    574570            || IsSubType($dataNode, "AbstractWorker")
    575             || $name eq "SVGElementInstance"
    576             || $name eq "EventTarget")) {
     571            || IsSubType($dataNode, "EventTarget"))) {
    577572        push(@customInternalFields, "eventListenerCacheIndex");
    578573    }
    … …  
    25622557    foreach (@{$dataNode->parents}) {
    25632558        my $parent = $codeGenerator->StripModule($_);
    2564         if ($parent eq "EventTarget") {
    2565             next;
    2566         }
    25672559        AddToImplIncludes("V8${parent}.h");
    25682560        $parentClass = "V8" . $parent;
    … …  
    33443336}
    33453337
     3338sub BaseInterfaceName
     3339{
     3340    my $dataNode = shift;
     3341
     3342    while (@{$dataNode->parents}) {
     3343        $dataNode = $codeGenerator->ParseInterface($codeGenerator->StripModule(@{$dataNode->parents}[0]), 1);
     3344    }
     3345
     3346    return $codeGenerator->StripModule($dataNode->name);
     3347}
     3348
    33463349sub GenerateToV8Converters
    33473350{
    … …  
    33553358    my $forceNewObjectCall = IsDOMNodeType($interfaceName) ? ", forceNewObject" : "";
    33563359    my $wrapSlowArgumentType = GetPassRefPtrType($nativeType);
     3360    my $baseType = BaseInterfaceName($dataNode);
    33573361
    33583362    push(@implContent, <<END);
    … …  
    33623366    v8::Handle<v8::Object> wrapper;
    33633367END
     3368    if ($baseType ne $interfaceName) {
     3369        push(@implContent, <<END);
     3370    ASSERT(static_cast<void*>(static_cast<${baseType}*>(impl.get())) == static_cast<void*>(impl.get()));
     3371END
     3372    }
    33643373
    33653374    my $proxyInit;
    … …  
    36983707    return "DOMTimeStamp" if $type eq "DOMTimeStamp";
    36993708    return "unsigned" if $type eq "unsigned int";
     3709    # FIXME: When EventTarget is an interface and not a mixin, fix this so that
     3710    # EventTarget can be passed as a parameter.
    37003711    return "Node*" if $type eq "EventTarget" and $isParameter;
    37013712    return "double" if $type eq "Date";
    … …  
    38153826
    38163827    # Default, assume autogenerated type conversion routines
     3828    # FIXME: When EventTarget is an interface and not a mixin, fix this to use
     3829    # V8EventTarget::HasInstance etc.
    38173830    if ($type eq "EventTarget") {
    38183831        AddToImplIncludes("V8Node.h");
    … …  
    38583871    return "V8Event.h" if $type eq "DOMTimeStamp";
    38593872    return "EventListener.h" if $type eq "EventListener";
    3860     return "EventTarget.h" if $type eq "EventTarget";
    38613873    return "SerializedScriptValue.h" if $type eq "SerializedScriptValue";
    38623874    return "ScriptValue.h" if $type eq "DOMObject";
    … …  
    39503962    'Dictionary' => 1,
    39513963    'EventListener' => 1,
     3964    # FIXME: When EventTarget is an interface and not a mixin, fix this so that
     3965    # EventTarget is treated as a wrapper type.
    39523966    'EventTarget' => 1,
    39533967    'IDBKey' => 1,

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8Float64Array.cpp

    r125216 r125251  
    151151{
    152152    v8::Handle<v8::Object> wrapper;
     153    ASSERT(static_cast<void*>(static_cast<ArrayBufferView*>(impl.get())) == static_cast<void*>(impl.get()));
    153154    V8Proxy* proxy = 0;
    154155    wrapper = V8DOMWrapper::instantiateV8Object(proxy, &info, impl.get());

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestNode.cpp

    r125216 r125251  
    114114{
    115115    v8::Handle<v8::Object> wrapper;
     116    ASSERT(static_cast<void*>(static_cast<Node*>(impl.get())) == static_cast<void*>(impl.get()));
    116117    V8Proxy* proxy = impl->document()->frame() ? impl->document()->frame()->script()->proxy() : 0;
    117118

  • trunk/Source/WebCore/svg/SVGElementInstance.h

    r119937 r125251  
    3939
    4040// SVGElementInstance mimics Node, but without providing all its functionality
    41 class SVGElementInstance : public TreeShared<SVGElementInstance, SVGElementInstance>, public EventTarget {
     41class SVGElementInstance : public EventTarget, public TreeShared<SVGElementInstance, SVGElementInstance> {
    4242public:
    4343    static PassRefPtr<SVGElementInstance> create(SVGUseElement* correspondingUseElement, SVGUseElement* directUseElement, PassRefPtr<SVGElement> originalElement)

  • trunk/Source/WebCore/svg/SVGElementInstance.idl

    r107041 r125251  
    3030        Conditional=SVG,
    3131        JSCustomMarkFunction,
    32         JSGenerateToNativeObject
     32        JSGenerateToNativeObject,
     33        JSGenerateToJSObject
    3334    ] SVGElementInstance
    3435#if defined(LANGUAGE_OBJECTIVE_C) && LANGUAGE_OBJECTIVE_C
    3536        : Object, EventTarget
     37#else
     38        : EventTarget
    3639#endif /* defined(LANGUAGE_OBJECTIVE_C) */
    3740    {
    … …  
    8790        attribute [NotEnumerable] EventListener onsubmit;
    8891        attribute [NotEnumerable] EventListener onunload;
    89 
    90         void addEventListener(in DOMString type,
    91                               in EventListener listener,
    92                               in [Optional] boolean useCapture);
    93         void removeEventListener(in DOMString type,
    94                                  in EventListener listener,
    95                                  in [Optional] boolean useCapture);
    96         boolean dispatchEvent(in Event event)
    97             raises(EventException);
    9892#endif /* defined(LANGUAGE_OBJECTIVE_C) */
    9993    };
Note: See TracChangeset for help on using the changeset viewer.