Title: [147464] trunk/Source/WebCore
Revision
147464
Author
pe...@chromium.org
Date
2013-04-02 11:12:30 -0700 (Tue, 02 Apr 2013)

Log Message

[JSC] Don't create a JSValue if it's not going to be used for nullable attributes
https://bugs.webkit.org/show_bug.cgi?id=112711

Reviewed by Sam Weinig.

When nullable attributes are used, it's possible that we disregard the JSValue
when null should be returned instead. This is a waste, and we should cast the
native type to a JSValue as late as possible.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation):
(NativeValueToLocal):
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::jsTestObjAttrWithGetterException):
(WebCore::jsTestObjStringAttrWithGetterException):
(WebCore::jsTestObjWithScriptStateAttributeRaises):
(WebCore::jsTestObjWithScriptExecutionContextAttributeRaises):
(WebCore::jsTestObjWithScriptExecutionContextAndScriptStateAttributeRaises):
(WebCore::jsTestObjNullableDoubleAttribute):
(WebCore::jsTestObjNullableLongAttribute):
(WebCore::jsTestObjNullableBooleanAttribute):
(WebCore::jsTestObjNullableStringAttribute):
(WebCore::jsTestObjNullableLongSettableAttribute):
(WebCore::jsTestObjNullableStringValue):
* bindings/scripts/test/JS/JSTestTypedefs.cpp:
(WebCore::jsTestTypedefsAttrWithGetterException):
(WebCore::jsTestTypedefsStringAttrWithGetterException):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (147463 => 147464)


--- trunk/Source/WebCore/ChangeLog	2013-04-02 18:10:12 UTC (rev 147463)
+++ trunk/Source/WebCore/ChangeLog	2013-04-02 18:12:30 UTC (rev 147464)
@@ -1,3 +1,33 @@
+2013-04-02  Peter Beverloo  <pe...@chromium.org>
+
+        [JSC] Don't create a JSValue if it's not going to be used for nullable attributes
+        https://bugs.webkit.org/show_bug.cgi?id=112711
+
+        Reviewed by Sam Weinig.
+
+        When nullable attributes are used, it's possible that we disregard the JSValue
+        when null should be returned instead. This is a waste, and we should cast the
+        native type to a JSValue as late as possible.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateImplementation):
+        (NativeValueToLocal):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::jsTestObjAttrWithGetterException):
+        (WebCore::jsTestObjStringAttrWithGetterException):
+        (WebCore::jsTestObjWithScriptStateAttributeRaises):
+        (WebCore::jsTestObjWithScriptExecutionContextAttributeRaises):
+        (WebCore::jsTestObjWithScriptExecutionContextAndScriptStateAttributeRaises):
+        (WebCore::jsTestObjNullableDoubleAttribute):
+        (WebCore::jsTestObjNullableLongAttribute):
+        (WebCore::jsTestObjNullableBooleanAttribute):
+        (WebCore::jsTestObjNullableStringAttribute):
+        (WebCore::jsTestObjNullableLongSettableAttribute):
+        (WebCore::jsTestObjNullableStringValue):
+        * bindings/scripts/test/JS/JSTestTypedefs.cpp:
+        (WebCore::jsTestTypedefsAttrWithGetterException):
+        (WebCore::jsTestTypedefsStringAttrWithGetterException):
+
 2013-04-02  Bem Jones-Bey  <bjone...@adobe.com>
 
         [css exclusions] overflow:hidden undoes shape-outside offsets

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (147463 => 147464)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2013-04-02 18:10:12 UTC (rev 147463)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2013-04-02 18:12:30 UTC (rev 147464)
@@ -1823,6 +1823,7 @@
                 my $name = $attribute->signature->name;
                 my $type = $attribute->signature->type;
                 my $isNullable = $attribute->signature->isNullable;
+                my $isCachedAttribute = $attribute->signature->extendedAttributes->{"CachedAttribute"};
                 $codeGenerator->AssertNotSequenceType($type);
                 my $getFunctionName = GetAttributeGetterName($interfaceName, $className, $attribute);
                 my $implGetterFunctionName = $codeGenerator->WK_lcfirst($name);
@@ -1839,7 +1840,7 @@
                     push(@implContent, "    UNUSED_PARAM(slotBase);\n");
                 }
 
-                if ($attribute->signature->extendedAttributes->{"CachedAttribute"}) {
+                if ($isCachedAttribute) {
                     $needsMarkChildren = 1;
                 }
 
@@ -1888,7 +1889,7 @@
                     push(@implContent, "    bool isNull = false;\n") if $isNullable;
 
                     my $cacheIndex = 0;
-                    if ($attribute->signature->extendedAttributes->{"CachedAttribute"}) {
+                    if ($isCachedAttribute) {
                         $cacheIndex = $currentCachedAttribute;
                         $currentCachedAttribute++;
                         push(@implContent, "    if (JSValue cachedValue = castedThis->m_" . $attribute->signature->name . ".get())\n");
@@ -1896,6 +1897,7 @@
                     }
 
                     my @callWithArgs = GenerateCallWith($attribute->signature->extendedAttributes->{"CallWith"}, \@implContent, "jsUndefined()");
+                    my $returnValue = "result";
 
                     if ($svgListPropertyType) {
                         push(@implContent, "    JSValue result =  " . NativeToJSValue($attribute->signature, 0, $interfaceName, "castedThis->impl()->$implGetterFunctionName(" . (join ", ", @callWithArgs) . ")", "castedThis") . ";\n");
@@ -1923,23 +1925,30 @@
 
                         unshift(@arguments, @callWithArgs);
 
-                        my $jsType = NativeToJSValue($attribute->signature, 0, $interfaceName, "${functionName}(" . join(", ", @arguments) . ")", "castedThis");
                         push(@implContent, "    $interfaceName* impl = static_cast<$interfaceName*>(castedThis->impl());\n") if !$attribute->isStatic;
-                        if ($codeGenerator->IsSVGAnimatedType($type)) {
-                            push(@implContent, "    RefPtr<$type> obj = $jsType;\n");
-                            push(@implContent, "    JSValue result =  toJS(exec, castedThis->globalObject(), obj.get());\n");
-                        } else {
-                            push(@implContent, "    JSValue result = $jsType;\n");
-                        }
-
                         if ($isNullable) {
+                            my $nativeType = GetNativeType($type);
+                            push(@implContent, "    $nativeType nativeResult = " . NativeValueToLocal("$functionName(" . join(", ", @arguments) . ")", $nativeType) . ";\n");
                             push(@implContent, "    if (isNull)\n");
                             push(@implContent, "        return jsNull();\n");
+                            if ($isCachedAttribute) {
+                                push(@implContent, "    JSValue result = " . NativeToJSValue($attribute->signature, 0, $interfaceName, "nativeResult", "castedThis") . ";\n");
+                            } else {
+                                $returnValue = NativeToJSValue($attribute->signature, 0, $interfaceName, "nativeResult", "castedThis");
+                            }
+                        } else {
+                            my $jsType = NativeToJSValue($attribute->signature, 0, $interfaceName, "${functionName}(" . join(", ", @arguments) . ")", "castedThis");
+                            if ($codeGenerator->IsSVGAnimatedType($type)) {
+                                push(@implContent, "    RefPtr<$type> obj = $jsType;\n");
+                                push(@implContent, "    JSValue result = toJS(exec, castedThis->globalObject(), obj.get());\n");
+                            } else {
+                                push(@implContent, "    JSValue result = $jsType;\n");
+                            }
                         }
                     }
 
-                    push(@implContent, "    castedThis->m_" . $attribute->signature->name . ".set(exec->globalData(), castedThis, result);\n") if ($attribute->signature->extendedAttributes->{"CachedAttribute"});
-                    push(@implContent, "    return result;\n");
+                    push(@implContent, "    castedThis->m_" . $attribute->signature->name . ".set(exec->globalData(), castedThis, result);\n") if $isCachedAttribute;
+                    push(@implContent, "    return $returnValue;\n");
 
                 } else {
                     my @arguments = ("ec");
@@ -1951,13 +1960,14 @@
                     }
 
                     unshift(@arguments, GenerateCallWith($attribute->signature->extendedAttributes->{"CallWith"}, \@implContent, "jsUndefined()"));
+                    my $nativeType = GetNativeType($type);
 
                     if ($svgPropertyOrListPropertyType) {
                         push(@implContent, "    $svgPropertyOrListPropertyType impl(*castedThis->impl());\n");
-                        push(@implContent, "    JSC::JSValue result = " . NativeToJSValue($attribute->signature, 0, $interfaceName, "impl.$implGetterFunctionName(" . join(", ", @arguments) . ")", "castedThis") . ";\n");
+                        push(@implContent, "    $nativeType nativeResult = " . NativeValueToLocal("impl.$implGetterFunctionName(" . join(", ", @arguments) . ")", $nativeType) . ";\n");
                     } else {
                         push(@implContent, "    $interfaceName* impl = static_cast<$interfaceName*>(castedThis->impl());\n");
-                        push(@implContent, "    JSC::JSValue result = " . NativeToJSValue($attribute->signature, 0, $interfaceName, "impl->$implGetterFunctionName(" . join(", ", @arguments) . ")", "castedThis") . ";\n");
+                        push(@implContent, "    $nativeType nativeResult = " . NativeValueToLocal("impl->$implGetterFunctionName(" . join(", ", @arguments) . ")", $nativeType) . ";\n");
                     }
 
                     if ($isNullable) {
@@ -1966,7 +1976,7 @@
                     }
 
                     push(@implContent, "    setDOMException(exec, ec);\n");
-                    push(@implContent, "    return result;\n");
+                    push(@implContent, "    return " . NativeToJSValue($attribute->signature, 0, $interfaceName, "nativeResult", "castedThis") . ";\n");
                 }
 
                 push(@implContent, "}\n\n");
@@ -3197,6 +3207,15 @@
     return exists $nativeType{$type};
 }
 
+sub NativeValueToLocal
+{
+    my $value = shift;
+    my $type = shift;
+
+    return "WTF::getPtr($value)" if $type =~ /\*$/;
+    return $value;
+}
+
 sub JSValueToNative
 {
     my $signature = shift;

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


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2013-04-02 18:10:12 UTC (rev 147463)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2013-04-02 18:12:30 UTC (rev 147464)
@@ -657,9 +657,9 @@
     JSTestObj* castedThis = jsCast<JSTestObj*>(asObject(slotBase));
     ExceptionCode ec = 0;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSC::JSValue result = jsNumber(impl->attrWithGetterException(ec));
+    int nativeResult = impl->attrWithGetterException(ec);
     setDOMException(exec, ec);
-    return result;
+    return jsNumber(nativeResult);
 }
 
 
@@ -678,9 +678,9 @@
     JSTestObj* castedThis = jsCast<JSTestObj*>(asObject(slotBase));
     ExceptionCode ec = 0;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSC::JSValue result = jsStringWithCache(exec, impl->stringAttrWithGetterException(ec));
+    const String& nativeResult = impl->stringAttrWithGetterException(ec);
     setDOMException(exec, ec);
-    return result;
+    return jsStringWithCache(exec, nativeResult);
 }
 
 
@@ -727,9 +727,9 @@
     JSTestObj* castedThis = jsCast<JSTestObj*>(asObject(slotBase));
     ExceptionCode ec = 0;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSC::JSValue result = toJS(exec, castedThis->globalObject(), WTF::getPtr(impl->withScriptStateAttributeRaises(exec, ec)));
+    TestObj* nativeResult = WTF::getPtr(impl->withScriptStateAttributeRaises(exec, ec));
     setDOMException(exec, ec);
-    return result;
+    return toJS(exec, castedThis->globalObject(), WTF::getPtr(nativeResult));
 }
 
 
@@ -741,9 +741,9 @@
     if (!scriptContext)
         return jsUndefined();
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSC::JSValue result = toJS(exec, castedThis->globalObject(), WTF::getPtr(impl->withScriptExecutionContextAttributeRaises(scriptContext, ec)));
+    TestObj* nativeResult = WTF::getPtr(impl->withScriptExecutionContextAttributeRaises(scriptContext, ec));
     setDOMException(exec, ec);
-    return result;
+    return toJS(exec, castedThis->globalObject(), WTF::getPtr(nativeResult));
 }
 
 
@@ -767,9 +767,9 @@
     if (!scriptContext)
         return jsUndefined();
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSC::JSValue result = toJS(exec, castedThis->globalObject(), WTF::getPtr(impl->withScriptExecutionContextAndScriptStateAttributeRaises(exec, scriptContext, ec)));
+    TestObj* nativeResult = WTF::getPtr(impl->withScriptExecutionContextAndScriptStateAttributeRaises(exec, scriptContext, ec));
     setDOMException(exec, ec);
-    return result;
+    return toJS(exec, castedThis->globalObject(), WTF::getPtr(nativeResult));
 }
 
 
@@ -987,10 +987,10 @@
     UNUSED_PARAM(exec);
     bool isNull = false;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSValue result = jsNumber(impl->nullableDoubleAttribute(isNull));
+    double nativeResult = impl->nullableDoubleAttribute(isNull);
     if (isNull)
         return jsNull();
-    return result;
+    return jsNumber(nativeResult);
 }
 
 
@@ -1000,10 +1000,10 @@
     UNUSED_PARAM(exec);
     bool isNull = false;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSValue result = jsNumber(impl->nullableLongAttribute(isNull));
+    int nativeResult = impl->nullableLongAttribute(isNull);
     if (isNull)
         return jsNull();
-    return result;
+    return jsNumber(nativeResult);
 }
 
 
@@ -1013,10 +1013,10 @@
     UNUSED_PARAM(exec);
     bool isNull = false;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSValue result = jsBoolean(impl->nullableBooleanAttribute(isNull));
+    bool nativeResult = impl->nullableBooleanAttribute(isNull);
     if (isNull)
         return jsNull();
-    return result;
+    return jsBoolean(nativeResult);
 }
 
 
@@ -1026,10 +1026,10 @@
     UNUSED_PARAM(exec);
     bool isNull = false;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSValue result = jsStringWithCache(exec, impl->nullableStringAttribute(isNull));
+    const String& nativeResult = impl->nullableStringAttribute(isNull);
     if (isNull)
         return jsNull();
-    return result;
+    return jsStringWithCache(exec, nativeResult);
 }
 
 
@@ -1039,10 +1039,10 @@
     UNUSED_PARAM(exec);
     bool isNull = false;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSValue result = jsNumber(impl->nullableLongSettableAttribute(isNull));
+    int nativeResult = impl->nullableLongSettableAttribute(isNull);
     if (isNull)
         return jsNull();
-    return result;
+    return jsNumber(nativeResult);
 }
 
 
@@ -1052,11 +1052,11 @@
     ExceptionCode ec = 0;
     bool isNull = false;
     TestObj* impl = static_cast<TestObj*>(castedThis->impl());
-    JSC::JSValue result = jsNumber(impl->nullableStringValue(isNull, ec));
+    int nativeResult = impl->nullableStringValue(isNull, ec);
     if (isNull)
         return jsNull();
     setDOMException(exec, ec);
-    return result;
+    return jsNumber(nativeResult);
 }
 
 

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp (147463 => 147464)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp	2013-04-02 18:10:12 UTC (rev 147463)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp	2013-04-02 18:12:30 UTC (rev 147464)
@@ -225,9 +225,9 @@
     JSTestTypedefs* castedThis = jsCast<JSTestTypedefs*>(asObject(slotBase));
     ExceptionCode ec = 0;
     TestTypedefs* impl = static_cast<TestTypedefs*>(castedThis->impl());
-    JSC::JSValue result = jsNumber(impl->attrWithGetterException(ec));
+    int nativeResult = impl->attrWithGetterException(ec);
     setDOMException(exec, ec);
-    return result;
+    return jsNumber(nativeResult);
 }
 
 
@@ -246,9 +246,9 @@
     JSTestTypedefs* castedThis = jsCast<JSTestTypedefs*>(asObject(slotBase));
     ExceptionCode ec = 0;
     TestTypedefs* impl = static_cast<TestTypedefs*>(castedThis->impl());
-    JSC::JSValue result = jsStringWithCache(exec, impl->stringAttrWithGetterException(ec));
+    const String& nativeResult = impl->stringAttrWithGetterException(ec);
     setDOMException(exec, ec);
-    return result;
+    return jsStringWithCache(exec, nativeResult);
 }
 
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to