Title: [200562] trunk/Source
Revision
200562
Author
[email protected]
Date
2016-05-08 14:56:57 -0700 (Sun, 08 May 2016)

Log Message

[Bindings] Simplify [RequiresExistingAtomicString] IDL extended attribute handling
https://bugs.webkit.org/show_bug.cgi?id=157465

Reviewed by Darin Adler.

Source/WebCore:

Simplify [RequiresExistingAtomicString] IDL extended attribute handling
in the bindings generator.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateParametersCheck):
Drop code specific to [RequiresExistingAtomicString].

(JSValueToNative):
Deal with [RequiresExistingAtomicString] IDL extended attribute,
similarly to [AtomicString] and [TreatNullAs].

* bindings/scripts/test/JS/JSTestObj.cpp:
* bindings/scripts/test/TestObj.idl:
Add bindings test coverage.

* dom/DocumentFragment.cpp:
(WebCore::DocumentFragment::getElementById):
* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::getElementById):
Add null check for the id parameter. The generated bindings used to do
this null check for us but it is no longer the case (to make the bindings
generator a bit simpler). This extended attribute is not commonly used
and is meant as a micro-optimization for getElementById().

Source/WTF:

Add an AtomicString(RefPtr<AtomicStringImpl>&&) constructor that is now
used by the bindings when [RequiresExistingAtomicString] is used. This
is more efficient than using AtomicString(AtomicStringImpl*) as the
caller has a RefPtr<AtomicStringImpl> and is consistent with the
pre-existing String(RefPtr<AtomicStringImpl>&&) constructor.

* wtf/text/AtomicString.h:
(WTF::AtomicString::AtomicString):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (200561 => 200562)


--- trunk/Source/WTF/ChangeLog	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WTF/ChangeLog	2016-05-08 21:56:57 UTC (rev 200562)
@@ -1,3 +1,19 @@
+2016-05-08  Chris Dumez  <[email protected]>
+
+        [Bindings] Simplify [RequiresExistingAtomicString] IDL extended attribute handling
+        https://bugs.webkit.org/show_bug.cgi?id=157465
+
+        Reviewed by Darin Adler.
+
+        Add an AtomicString(RefPtr<AtomicStringImpl>&&) constructor that is now
+        used by the bindings when [RequiresExistingAtomicString] is used. This
+        is more efficient than using AtomicString(AtomicStringImpl*) as the
+        caller has a RefPtr<AtomicStringImpl> and is consistent with the
+        pre-existing String(RefPtr<AtomicStringImpl>&&) constructor.
+
+        * wtf/text/AtomicString.h:
+        (WTF::AtomicString::AtomicString):
+
 2016-05-08  Darin Adler  <[email protected]>
 
         * wtf/text/WTFString.h: Remove pragma once. Not working for some reason.

Modified: trunk/Source/WTF/wtf/text/AtomicString.h (200561 => 200562)


--- trunk/Source/WTF/wtf/text/AtomicString.h	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WTF/wtf/text/AtomicString.h	2016-05-08 21:56:57 UTC (rev 200562)
@@ -56,6 +56,7 @@
     }
 
     AtomicString(AtomicStringImpl*);
+    AtomicString(RefPtr<AtomicStringImpl>&&);
     ATOMICSTRING_CONVERSION AtomicString(StringImpl*);
     ATOMICSTRING_CONVERSION AtomicString(const String&);
     AtomicString(StringImpl* baseString, unsigned start, unsigned length);
@@ -259,6 +260,11 @@
 {
 }
 
+inline AtomicString::AtomicString(RefPtr<AtomicStringImpl>&& imp)
+    : m_string(WTFMove(imp))
+{
+}
+
 inline AtomicString::AtomicString(StringImpl* imp)
     : m_string(AtomicStringImpl::add(imp))
 {

Modified: trunk/Source/WebCore/ChangeLog (200561 => 200562)


--- trunk/Source/WebCore/ChangeLog	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WebCore/ChangeLog	2016-05-08 21:56:57 UTC (rev 200562)
@@ -1,3 +1,34 @@
+2016-05-08  Chris Dumez  <[email protected]>
+
+        [Bindings] Simplify [RequiresExistingAtomicString] IDL extended attribute handling
+        https://bugs.webkit.org/show_bug.cgi?id=157465
+
+        Reviewed by Darin Adler.
+
+        Simplify [RequiresExistingAtomicString] IDL extended attribute handling
+        in the bindings generator.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateParametersCheck):
+        Drop code specific to [RequiresExistingAtomicString].
+
+        (JSValueToNative):
+        Deal with [RequiresExistingAtomicString] IDL extended attribute,
+        similarly to [AtomicString] and [TreatNullAs].
+
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        * bindings/scripts/test/TestObj.idl:
+        Add bindings test coverage.
+
+        * dom/DocumentFragment.cpp:
+        (WebCore::DocumentFragment::getElementById):
+        * svg/SVGSVGElement.cpp:
+        (WebCore::SVGSVGElement::getElementById):
+        Add null check for the id parameter. The generated bindings used to do
+        this null check for us but it is no longer the case (to make the bindings
+        generator a bit simpler). This extended attribute is not commonly used
+        and is meant as a micro-optimization for getElementById().
+
 2016-05-08  David Kilzer  <[email protected]>
 
         Roll out: ThreadSanitizer: Data race and thread leak in WebCore::ScrollingThread::createThreadIfNeeded

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (200561 => 200562)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-05-08 21:56:57 UTC (rev 200562)
@@ -3748,56 +3748,47 @@
                 }
             }
 
-            if ($parameter->extendedAttributes->{"RequiresExistingAtomicString"}) {
-                # FIXME: This could be made slightly more efficient if we added an AtomicString(RefPtr<AtomicStringImpl>&&) constructor and removed the call to get() here.
-                push(@$outputArray, "    AtomicString $name = state->argument($argumentIndex).toString(state)->toExistingAtomicString(state).get();\n");
-                push(@$outputArray, "    if ($name.isNull())\n");
-                push(@$outputArray, "        return JSValue::encode(jsNull());\n");
-                push(@$outputArray, "    if (UNLIKELY(state->hadException()))\n");
-                push(@$outputArray, "        return JSValue::encode(jsUndefined());\n");
-            } else {
-                my $outer;
-                my $inner;
-                my $nativeType = GetNativeTypeFromSignature($interface, $parameter);
+            my $outer;
+            my $inner;
+            my $nativeType = GetNativeTypeFromSignature($interface, $parameter);
 
-                if ($parameter->isOptional && defined($parameter->default) && !WillConvertUndefinedToDefaultParameterValue($type, $parameter->default)) {
-                    my $defaultValue = $parameter->default;
+            if ($parameter->isOptional && defined($parameter->default) && !WillConvertUndefinedToDefaultParameterValue($type, $parameter->default)) {
+                my $defaultValue = $parameter->default;
 
-                    # String-related optimizations.
-                    if ($type eq "DOMString") {
-                        my $useAtomicString = $parameter->extendedAttributes->{"AtomicString"};
-                        if ($defaultValue eq "null") {
-                            $defaultValue = $useAtomicString ? "nullAtom" : "String()";
-                        } elsif ($defaultValue eq "\"\"") {
-                            $defaultValue = $useAtomicString ? "emptyAtom" : "emptyString()";
-                        } else {
-                            $defaultValue = $useAtomicString ? "AtomicString($defaultValue, AtomicString::ConstructFromLiteral)" : "ASCIILiteral($defaultValue)";
-                        }
+                # String-related optimizations.
+                if ($type eq "DOMString") {
+                    my $useAtomicString = $parameter->extendedAttributes->{"AtomicString"};
+                    if ($defaultValue eq "null") {
+                        $defaultValue = $useAtomicString ? "nullAtom" : "String()";
+                    } elsif ($defaultValue eq "\"\"") {
+                        $defaultValue = $useAtomicString ? "emptyAtom" : "emptyString()";
                     } else {
-                        $defaultValue = "nullptr" if $defaultValue eq "null";
-                        $defaultValue = "PNaN" if $defaultValue eq "NaN";
-                        $defaultValue = "$nativeType()" if $defaultValue eq "[]";
-                        $defaultValue = "JSValue::JSUndefined" if $defaultValue eq "undefined";
+                        $defaultValue = $useAtomicString ? "AtomicString($defaultValue, AtomicString::ConstructFromLiteral)" : "ASCIILiteral($defaultValue)";
                     }
-
-                    $outer = "state->argument($argumentIndex).isUndefined() ? $defaultValue : ";
-                    $inner = "state->uncheckedArgument($argumentIndex)";
-                } elsif ($parameter->isOptional && !defined($parameter->default)) {
-                    # Use WTF::Optional<>() for optional parameters that are missing or undefined and that do not have a default value in the IDL.
-                    $outer = "state->argument($argumentIndex).isUndefined() ? Optional<$nativeType>() : ";
-                    $inner = "state->uncheckedArgument($argumentIndex)";
                 } else {
-                    $outer = "";
-                    $inner = "state->argument($argumentIndex)";
+                    $defaultValue = "nullptr" if $defaultValue eq "null";
+                    $defaultValue = "PNaN" if $defaultValue eq "NaN";
+                    $defaultValue = "$nativeType()" if $defaultValue eq "[]";
+                    $defaultValue = "JSValue::JSUndefined" if $defaultValue eq "undefined";
                 }
-                my ($nativeValue, $mayThrowException) = JSValueToNative($interface, $parameter, $inner, $function->signature->extendedAttributes->{"Conditional"});
-                push(@$outputArray, "    auto $name = ${outer}${nativeValue};\n");
-                $value = "WTFMove($name)";
-                if ($mayThrowException) {
-                    push(@$outputArray, "    if (UNLIKELY(state->hadException()))\n");
-                    push(@$outputArray, "        return JSValue::encode(jsUndefined());\n");
-                }
+
+                $outer = "state->argument($argumentIndex).isUndefined() ? $defaultValue : ";
+                $inner = "state->uncheckedArgument($argumentIndex)";
+            } elsif ($parameter->isOptional && !defined($parameter->default)) {
+                # Use WTF::Optional<>() for optional parameters that are missing or undefined and that do not have a default value in the IDL.
+                $outer = "state->argument($argumentIndex).isUndefined() ? Optional<$nativeType>() : ";
+                $inner = "state->uncheckedArgument($argumentIndex)";
+            } else {
+                $outer = "";
+                $inner = "state->argument($argumentIndex)";
             }
+            my ($nativeValue, $mayThrowException) = JSValueToNative($interface, $parameter, $inner, $function->signature->extendedAttributes->{"Conditional"});
+            push(@$outputArray, "    auto $name = ${outer}${nativeValue};\n");
+            $value = "WTFMove($name)";
+            if ($mayThrowException) {
+                push(@$outputArray, "    if (UNLIKELY(state->hadException()))\n");
+                push(@$outputArray, "        return JSValue::encode(jsUndefined());\n");
+            }
 
             my $isTearOff = $codeGenerator->IsSVGTypeNeedingTearOff($type) && $interfaceName !~ /List$/;
             my $shouldPassByReference = ShouldPassWrapperByReference($parameter, $interface);
@@ -4388,6 +4379,8 @@
     }
 
     if ($type eq "DOMString") {
+        return ("AtomicString($value.toString(state)->toExistingAtomicString(state))", 1) if $signature->extendedAttributes->{"RequiresExistingAtomicString"};
+
         if ($signature->extendedAttributes->{"TreatNullAs"}) {
             return ("valueToStringTreatingNullAsEmptyString(state, $value)", 1) if $signature->extendedAttributes->{"TreatNullAs"} eq "EmptyString";
             return ("valueToStringWithNullCheck(state, $value)", 1) if $signature->extendedAttributes->{"TreatNullAs"} eq "LegacyNullString";

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp (200561 => 200562)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2016-05-08 21:56:57 UTC (rev 200562)
@@ -25,6 +25,7 @@
 #include "DOMStringList.h"
 #include "Dictionary.h"
 #include "Document.h"
+#include "Element.h"
 #include "ExceptionCode.h"
 #include "Frame.h"
 #include "HTMLNames.h"
@@ -35,6 +36,7 @@
 #include "JSDOMPromise.h"
 #include "JSDOMStringList.h"
 #include "JSDocument.h"
+#include "JSElement.h"
 #include "JSEventListener.h"
 #include "JSFetchRequest.h"
 #include "JSNode.h"
@@ -635,6 +637,7 @@
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionDomStringListFunction(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence2(JSC::ExecState*);
+JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionGetElementById(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionGetSVGDocument(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionConvert1(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionConvert2(JSC::ExecState*);
@@ -1212,6 +1215,7 @@
     { "domStringListFunction", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionDomStringListFunction), (intptr_t) (1) } },
     { "methodWithAndWithoutNullableSequence", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence), (intptr_t) (2) } },
     { "methodWithAndWithoutNullableSequence2", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence2), (intptr_t) (2) } },
+    { "getElementById", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionGetElementById), (intptr_t) (1) } },
     { "getSVGDocument", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionGetSVGDocument), (intptr_t) (0) } },
     { "convert1", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionConvert1), (intptr_t) (1) } },
     { "convert2", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionConvert2), (intptr_t) (1) } },
@@ -5637,6 +5641,23 @@
     return JSValue::encode(jsUndefined());
 }
 
+EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionGetElementById(ExecState* state)
+{
+    JSValue thisValue = state->thisValue();
+    auto castedThis = jsDynamicCast<JSTestObj*>(thisValue);
+    if (UNLIKELY(!castedThis))
+        return throwThisTypeError(*state, "TestObj", "getElementById");
+    ASSERT_GC_OBJECT_INHERITS(castedThis, JSTestObj::info());
+    auto& impl = castedThis->wrapped();
+    if (UNLIKELY(state->argumentCount() < 1))
+        return throwVMError(state, createNotEnoughArgumentsError(state));
+    auto elementId = AtomicString(state->argument(0).toString(state)->toExistingAtomicString(state));
+    if (UNLIKELY(state->hadException()))
+        return JSValue::encode(jsUndefined());
+    JSValue result = toJS(state, castedThis->globalObject(), WTF::getPtr(impl.getElementById(WTFMove(elementId))));
+    return JSValue::encode(result);
+}
+
 EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionGetSVGDocument(ExecState* state)
 {
     JSValue thisValue = state->thisValue();

Modified: trunk/Source/WebCore/bindings/scripts/test/TestObj.idl (200561 => 200562)


--- trunk/Source/WebCore/bindings/scripts/test/TestObj.idl	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WebCore/bindings/scripts/test/TestObj.idl	2016-05-08 21:56:57 UTC (rev 200562)
@@ -290,6 +290,8 @@
 
     void methodWithAndWithoutNullableSequence(sequence<unsigned long> arrayArg, sequence<unsigned long>? nullableArrayArg);
     void methodWithAndWithoutNullableSequence2(unsigned long[] arrayArg, unsigned long[]? nullableArrayArg);
+
+    Element? getElementById([RequiresExistingAtomicString] DOMString elementId);
 #endif
 
     [CheckSecurityForNode] readonly attribute Document contentDocument;

Modified: trunk/Source/WebCore/dom/DocumentFragment.cpp (200561 => 200562)


--- trunk/Source/WebCore/dom/DocumentFragment.cpp	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WebCore/dom/DocumentFragment.cpp	2016-05-08 21:56:57 UTC (rev 200562)
@@ -93,6 +93,9 @@
 
 Element* DocumentFragment::getElementById(const AtomicString& id) const
 {
+    if (id.isNull())
+        return nullptr;
+
     // Fast path for ShadowRoot, where we are both a DocumentFragment and a TreeScope.
     if (isTreeScope())
         return treeScope().getElementById(id);

Modified: trunk/Source/WebCore/svg/SVGSVGElement.cpp (200561 => 200562)


--- trunk/Source/WebCore/svg/SVGSVGElement.cpp	2016-05-08 19:11:03 UTC (rev 200561)
+++ trunk/Source/WebCore/svg/SVGSVGElement.cpp	2016-05-08 21:56:57 UTC (rev 200562)
@@ -672,6 +672,9 @@
 // See http://www.w3.org/TR/SVG11/struct.html#InterfaceSVGSVGElement
 Element* SVGSVGElement::getElementById(const AtomicString& id)
 {
+    if (id.isNull())
+        return nullptr;
+
     Element* element = treeScope().getElementById(id);
     if (element && element->isDescendantOf(this))
         return element;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to