Eliminate unnecessary String-->AtomicString conversions from generated V8 bindings,
authorsnej@chromium.org <snej@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Nov 2009 19:57:06 +0000 (19:57 +0000)
committersnej@chromium.org <snej@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Nov 2009 19:57:06 +0000 (19:57 +0000)
by causing the right v8-to-WebCore conversion function to be called for every parameter.
This no longer requires any IDL metadata, so I've removed the [HintAtomic] annotations.
To enforce correctness, I added a mode that disables implicit
String-->AtomicString conversions while compiling the generated bindings.
https://bugs.webkit.org/show_bug.cgi?id=31168

Reviewed by Darin Adler.

* bindings/scripts/CodeGeneratorV8.pm:  Generate usage of V8Parameter class.
* bindings/v8/DerivedSourcesAllInOne.cpp:  Enable NO_IMPLICIT_ATOMICSTRING.
* bindings/v8/V8Binding.h:  Add V8Parameter class.
* css/WebKitCSSKeyframesRule.h:  Make AtomicString conversions explicit.
* dom/Document.idl:  Remove obsolete [HintAtomic] annotation.
* platform/text/AtomicString.h:  Added NO_IMPLICIT_ATOMICSTRING option.
* svg/SVGAnimatedTemplate.h:  Change some return types to String to avoid implicit conversion.
* svg/SVGAnimatedProperty.h: Adapt to changed return types in SVGAnimatedTemplate.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@51125 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bindings/scripts/CodeGeneratorV8.pm
WebCore/bindings/v8/DerivedSourcesAllInOne.cpp
WebCore/bindings/v8/V8Binding.h
WebCore/css/WebKitCSSKeyframesRule.h
WebCore/dom/Document.idl
WebCore/platform/text/AtomicString.h
WebCore/svg/SVGAnimatedProperty.h
WebCore/svg/SVGAnimatedTemplate.h

index f675e9a41a6dd0f74288cacb5b4fba87521eae85..0ed838d6cfdbd2032b8df250f161f101350ac721 100644 (file)
@@ -1,3 +1,23 @@
+2009-11-17  Jens Alfke  <snej@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Eliminate unnecessary String-->AtomicString conversions from generated V8 bindings,
+        by causing the right v8-to-WebCore conversion function to be called for every parameter.
+        This no longer requires any IDL metadata, so I've removed the [HintAtomic] annotations.
+        To enforce correctness, I added a mode that disables implicit
+        String-->AtomicString conversions while compiling the generated bindings.
+        https://bugs.webkit.org/show_bug.cgi?id=31168
+
+        * bindings/scripts/CodeGeneratorV8.pm:  Generate usage of V8Parameter class.
+        * bindings/v8/DerivedSourcesAllInOne.cpp:  Enable NO_IMPLICIT_ATOMICSTRING.
+        * bindings/v8/V8Binding.h:  Add V8Parameter class.
+        * css/WebKitCSSKeyframesRule.h:  Make AtomicString conversions explicit.
+        * dom/Document.idl:  Remove obsolete [HintAtomic] annotation.
+        * platform/text/AtomicString.h:  Added NO_IMPLICIT_ATOMICSTRING option.
+        * svg/SVGAnimatedTemplate.h:  Change some return types to String to avoid implicit conversion.
+        * svg/SVGAnimatedProperty.h: Adapt to changed return types in SVGAnimatedTemplate.
+
 2009-11-18  Darin Adler  <darin@apple.com>
 
         Reviewed by Dan Bernstein.
index 1fef83eb1006df3650374395be8c92870ee1e06a..8f63e0dbf2734ef766b378fd55d750d78e765ecf 100644 (file)
@@ -468,7 +468,7 @@ sub GenerateNormalAttrGetter
     my $attrType = GetTypeFromSignature($attribute->signature);
     my $attrIsPodType = IsPodType($attrType);
 
-    my $nativeType = GetNativeTypeFromSignature($attribute->signature, 0);
+    my $nativeType = GetNativeTypeFromSignature($attribute->signature, -1);
     my $isPodType = IsPodType($implClassName);
     my $skipContext = 0;
 
@@ -947,7 +947,7 @@ END
             push(@implContentDecls, "    bool ${parameterName}Ok;\n");
         }
 
-        push(@implContentDecls, "    " . GetNativeTypeFromSignature($parameter, 1) . " $parameterName = ");
+        push(@implContentDecls, "    " . GetNativeTypeFromSignature($parameter, $paramIndex) . " $parameterName = ");
         push(@implContentDecls, JSValueToNative($parameter, "args[$paramIndex]",
            BasicTypeCanFailConversion($parameter) ?  "${parameterName}Ok" : undef) . ";\n");
 
@@ -1718,19 +1718,14 @@ sub GetTypeFromSignature
 {
     my $signature = shift;
 
-    my $type = $codeGenerator->StripModule($signature->type);
-    if (($type eq "DOMString") && ($signature->extendedAttributes->{"HintAtomic"} || $signature->extendedAttributes->{"Reflect"})) {
-        $type = "AtomicString";
-    }
-
-    return $type;
+    return $codeGenerator->StripModule($signature->type);
 }
 
 
 sub GetNativeTypeFromSignature
 {
     my $signature = shift;
-    my $isParameter = shift;
+    my $parameterIndex = shift;
 
     my $type = GetTypeFromSignature($signature);
 
@@ -1739,7 +1734,19 @@ sub GetNativeTypeFromSignature
         return "int";
     }
 
-    return GetNativeType($type, $isParameter);
+    $type = GetNativeType($type, $parameterIndex >= 0 ? 1 : 0);
+    
+    if ($parameterIndex >= 0 && $type eq "V8Parameter") {
+        my $mode = "";
+        if ($signature->extendedAttributes->{"ConvertUndefinedOrNullToNullString"}) {
+            $mode = "WithUndefinedOrNullCheck";
+        } elsif ($signature->extendedAttributes->{"ConvertNullToNullString"}) {
+            $mode = "WithNullCheck";
+        }
+        $type .= "<$mode>";
+    }
+    
+    return $type;
 }
 
 sub IsRefPtrType
@@ -1857,10 +1864,11 @@ sub GetNativeType
     my $type = shift;
     my $isParameter = shift;
 
-    if ($type eq "float" or $type eq "AtomicString" or $type eq "double") {
+    if ($type eq "float" or $type eq "double") {
         return $type;
     }
 
+    return "V8Parameter" if ($type eq "DOMString" or $type eq "DOMUserData") and $isParameter;
     return "int" if $type eq "int";
     return "int" if $type eq "short" or $type eq "unsigned short";
     return "unsigned" if $type eq "unsigned long";
@@ -1896,7 +1904,6 @@ sub GetNativeType
 
 
 my %typeCanFailConversion = (
-    "AtomicString" => 0,
     "Attr" => 1,
     "WebGLArray" => 0,
     "WebGLBuffer" => 0,
@@ -2011,21 +2018,8 @@ sub JSValueToNative
     return "static_cast<Range::CompareHow>($value->Int32Value())" if $type eq "CompareHow";
     return "static_cast<SVGPaint::SVGPaintType>($value->ToInt32()->Int32Value())" if $type eq "SVGPaintType";
 
-    if ($type eq "AtomicString") {
-        return "toAtomicWebCoreStringWithNullCheck($value)" if $signature->extendedAttributes->{"ConvertNullToNullString"};
-        return "v8ValueToAtomicWebCoreString($value)";
-    }
-
-    if ($type eq "DOMUserData") {
-        return "toWebCoreString(args, $1)" if $value =~ /args\[(\d+)]/;
-        return "toWebCoreString($value)";
-    }
-    if ($type eq "DOMString") {
-        return "toWebCoreStringWithNullCheck($value)" if $signature->extendedAttributes->{"ConvertNullToNullString"};
-        return "toWebCoreStringWithNullOrUndefinedCheck($value)" if $signature->extendedAttributes->{"ConvertUndefinedOrNullToNullString"};
-        
-        return "toWebCoreString(args, $1)" if $value =~ /args\[(\d+)]/;
-        return "toWebCoreString($value)";
+    if ($type eq "DOMString" or $type eq "DOMUserData") {
+        return $value;
     }
 
     if ($type eq "SerializedScriptValue") {
@@ -2149,7 +2143,6 @@ sub RequiresCustomSignature
 
 my %non_wrapper_types = (
     'float' => 1,
-    'AtomicString' => 1,
     'double' => 1,
     'short' => 1,
     'unsigned short' => 1,
index 4e81c94c80c9f028fc49399d0a6b6d211e42cde3..508cd45af346de2c82b199d02b8fb371f9e0f26a 100644 (file)
 // This source file coalesces the V8 derived sources into a single object file to
 // reduce bloat and allow us to link release builds on 32-bit Windows.
 
+// Require explicit conversion to AtomicString. This helps catch cases where
+// the generated bindings cause an expensive implicit conversion.
+#define NO_IMPLICIT_ATOMICSTRING
+
 #include "bindings/V8Attr.cpp"
 #include "bindings/V8BarInfo.cpp"
 #include "bindings/V8BeforeLoadEvent.cpp"
index 7bf5d9437917c985d5007a3a07a7d32913879a5f..de5bb4cb9812ff9d399d7c00ae28e4fec329a9e3 100644 (file)
@@ -229,6 +229,31 @@ namespace WebCore {
 
     v8::Handle<v8::Value> getElementEventHandlerAttr(const v8::AccessorInfo&,
                                                      const AtomicString&);
+    
+    // V8Parameter is an adapter class that converts V8 values to Strings
+    // or AtomicStrings as appropriate, using multiple typecast operators.
+    enum V8ParameterMode {
+        DefaultMode,
+        WithNullCheck,
+        WithUndefinedOrNullCheck
+    };
+    template <V8ParameterMode MODE = DefaultMode>
+    class V8Parameter {
+    public:
+        V8Parameter (v8::Local<v8::Value> object) :m_v8Object(object) { }
+        operator String();
+        operator AtomicString();
+    private:
+        v8::Local<v8::Value> m_v8Object;
+    };
+    
+    template<> inline V8Parameter<DefaultMode>::operator String() { return toWebCoreString(m_v8Object); }
+    template<> inline V8Parameter<WithNullCheck>::operator String() { return toWebCoreStringWithNullCheck(m_v8Object); }
+    template<> inline V8Parameter<WithUndefinedOrNullCheck>::operator String() { return toWebCoreStringWithNullOrUndefinedCheck(m_v8Object); }
+
+    template<> inline V8Parameter<DefaultMode>::operator AtomicString() { return v8ValueToAtomicWebCoreString(m_v8Object); }
+    template<> inline V8Parameter<WithNullCheck>::operator AtomicString() { return toAtomicWebCoreStringWithNullCheck(m_v8Object); }
+    template<> inline V8Parameter<WithUndefinedOrNullCheck>::operator AtomicString() { return toAtomicWebCoreStringWithNullCheck(m_v8Object); }
 
 } // namespace WebCore
 
index 8c76b613281bf963536968ee1adebdffb07a1305..f58406f10e7000dd0b53538d8efd55fa0f8448c4 100644 (file)
@@ -64,7 +64,7 @@ public:
     // themselves, or know that it will get called later.
     void setNameInternal(const String& name)
     {   
-        m_name = name;
+        m_name = AtomicString(name);
     }
 
     CSSRuleList* cssRules() { return m_lstCSSRules.get(); }
index e9b548057126a05f1478c0b11866d1982e985b46..76a4edb0601dbbd0a622f46537c6b299343cab7c 100644 (file)
@@ -35,7 +35,7 @@ module core {
         readonly attribute [V8Custom] DOMImplementation implementation;
         readonly attribute Element documentElement;
 
-        [ReturnsNew] Element createElement(in [ConvertNullToNullString, HintAtomic] DOMString tagName)
+        [ReturnsNew] Element createElement(in [ConvertNullToNullString] DOMString tagName)
             raises (DOMException);
         DocumentFragment   createDocumentFragment();
         [ReturnsNew] Text createTextNode(in DOMString data);
@@ -64,7 +64,7 @@ module core {
             raises (DOMException);
         [OldStyleObjC] NodeList getElementsByTagNameNS(in [ConvertNullToNullString] DOMString namespaceURI,
                                                        in DOMString localName);
-        Element            getElementById(in [HintAtomic] DOMString elementId);
+        Element            getElementById(in DOMString elementId);
 
         // DOM Level 3 Core
 
index 4bde1b025d3230d99ea7f08416c99f5722439936..47d07c50191b12febbc220f3e88ef85757069551 100644 (file)
 #include "AtomicStringImpl.h"
 #include "PlatformString.h"
 
+// Define 'NO_IMPLICIT_ATOMICSTRING' before including this header,
+// to disallow (expensive) implicit String-->AtomicString conversions.
+#ifdef NO_IMPLICIT_ATOMICSTRING
+#define ATOMICSTRING_CONVERSION explicit
+#else
+#define ATOMICSTRING_CONVERSION
+#endif
+
 namespace WebCore {
 
 struct AtomicStringHash;
@@ -40,9 +48,9 @@ public:
     AtomicString(const JSC::UString& s) : m_string(add(s)) { }
     AtomicString(const JSC::Identifier& s) : m_string(add(s)) { }
 #endif
-    AtomicString(StringImpl* imp) : m_string(add(imp)) { }
+    ATOMICSTRING_CONVERSION AtomicString(StringImpl* imp) : m_string(add(imp)) { }
     AtomicString(AtomicStringImpl* imp) : m_string(imp) { }
-    AtomicString(const String& s) : m_string(add(s.impl())) { }
+    ATOMICSTRING_CONVERSION AtomicString(const String& s) : m_string(add(s.impl())) { }
 
     // Hash table deleted values, which are only constructed and never copied or destroyed.
     AtomicString(WTF::HashTableDeletedValueType) : m_string(WTF::HashTableDeletedValue) { }
index 984046fbe3c59878f7d95be6633c821ceb3b1031..5ae677fb2d65183c772c9ed5e5ceff57e73d1aba 100644 (file)
@@ -474,7 +474,7 @@ namespace WebCore {
     struct PropertySynchronizer<OwnerElement, DecoratedType, true> : Noncopyable {
         static void synchronize(const OwnerElement* ownerElement, const QualifiedName& attributeName, DecoratedType baseValue)
         {
-            AtomicString value(SVGAnimatedTypeValue<DecoratedType>::toString(baseValue));
+            String value(SVGAnimatedTypeValue<DecoratedType>::toString(baseValue));
 
             NamedNodeMap* namedAttrMap = ownerElement->attributes(false); 
             Attribute* old = namedAttrMap->getAttributeItem(attributeName);
@@ -483,7 +483,7 @@ namespace WebCore {
             else if (!old && !value.isNull()) 
                 namedAttrMap->addAttribute(const_cast<OwnerElement*>(ownerElement)->createAttribute(attributeName, value));
             else if (old && !value.isNull()) 
-                old->setValue(value); 
+                old->setValue(AtomicString(value)); 
         }
     };
 
index e7c49c157bcd164beeda7c75e919fe81f3f8bd04..d65fe0bdb6317e173b35436b1705fe4aa087ee85 100644 (file)
@@ -172,7 +172,7 @@ namespace WebCore {
         typedef Type* DecoratedType;
 
         static Type null() { return 0; }
-        static AtomicString toString(Type type) { return type ? AtomicString(type->valueAsString()) : nullAtom; }
+        static String toString(Type type) { return type ? type->valueAsString() : String(); }
     };
 
     template<>
@@ -181,7 +181,7 @@ namespace WebCore {
         typedef bool DecoratedType;
 
         static bool null() { return false; }
-        static AtomicString toString(bool type) { return type ? "true" : "false"; }
+        static String toString(bool type) { return type ? "true" : "false"; }
     };
 
     template<>
@@ -190,7 +190,7 @@ namespace WebCore {
         typedef int DecoratedType;
 
         static int null() { return 0; }
-        static AtomicString toString(int type) { return String::number(type); }
+        static String toString(int type) { return String::number(type); }
     };
 
     template<>
@@ -199,7 +199,7 @@ namespace WebCore {
         typedef long DecoratedType;
 
         static long null() { return 0l; }
-        static AtomicString toString(long type) { return String::number(type); }
+        static String toString(long type) { return String::number(type); }
     };
 
     template<>
@@ -208,7 +208,7 @@ namespace WebCore {
         typedef SVGLength DecoratedType;
 
         static SVGLength null() { return SVGLength(); }
-        static AtomicString toString(const SVGLength& type) { return type.valueAsString(); }
+        static String toString(const SVGLength& type) { return type.valueAsString(); }
     };
 
     template<>
@@ -217,7 +217,7 @@ namespace WebCore {
         typedef float DecoratedType;
 
         static float null() { return 0.0f; }
-        static AtomicString toString(float type) { return String::number(type); }
+        static String toString(float type) { return String::number(type); }
     };
 
     template<>
@@ -226,7 +226,7 @@ namespace WebCore {
         typedef FloatRect DecoratedType;
 
         static FloatRect null() { return FloatRect(); }
-        static AtomicString toString(const FloatRect& type) { return String::format("%f %f %f %f", type.x(), type.y(), type.width(), type.height()); }
+        static String toString(const FloatRect& type) { return String::format("%f %f %f %f", type.x(), type.y(), type.width(), type.height()); }
     };
 
     template<>
@@ -235,7 +235,7 @@ namespace WebCore {
         typedef String DecoratedType;
 
         static String null() { return String(); }
-        static AtomicString toString(const String& type) { return type; }
+        static String toString(const String& type) { return type; }
     };
 
     // Common type definitions, to ease IDL generation.