- Revision
- 108257
- Author
- [email protected]
- Date
- 2012-02-20 12:43:40 -0800 (Mon, 20 Feb 2012)
Log Message
Make JSCSSStyleDeclaration work directly with CSS Property ID
https://bugs.webkit.org/show_bug.cgi?id=79014
Reviewed by Geoffrey Garen.
Source/WebCore:
Previously, accessing the CSS property was done by converting from
the _javascript_ name to the CSS name, then converting that name to a lowercase
character array, and finally getting the CSS property ID.
This patch cut the indirection and make the code go directly from the
_javascript_ name conversion to the CSS property ID.
This improves the performance mainly due to the following:
-avoid dynamic memory allocation
-cut the conversion early when possible
-do not parse the string twice
The previous fast-path optimization was removed because it is no longer
necessary with this change.
The improvement are the following:
-previous fast-path: no change
-previous slow-path: ~3 times faster
Test: fast/dom/CSSStyleDeclaration/access-longest-css-property.html
This just test the edge case of CSSPropertyName.
* bindings/js/JSCSSStyleDeclarationCustom.cpp:
(WebCore::writeWebKitPrefix):
(WebCore::writeEpubPrefix):
(WebCore::cssPropertyIDForJSCSSPropertyName):
(WebCore::isCSSPropertyName):
(WebCore::JSCSSStyleDeclaration::nameGetter):
(WebCore::JSCSSStyleDeclaration::putDelegate):
* css/CSSParser.cpp:
(WebCore::cssPropertyID):
(WebCore):
(WebCore::cssPropertyNameIOSAliasing):
* css/CSSParser.h:
(WebCore):
LayoutTests:
* fast/dom/CSSStyleDeclaration/access-longest-css-property-expected.txt: Added.
* fast/dom/CSSStyleDeclaration/access-longest-css-property.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (108256 => 108257)
--- trunk/LayoutTests/ChangeLog 2012-02-20 19:03:16 UTC (rev 108256)
+++ trunk/LayoutTests/ChangeLog 2012-02-20 20:43:40 UTC (rev 108257)
@@ -1,3 +1,13 @@
+2012-02-20 Benjamin Poulain <[email protected]>
+
+ Make JSCSSStyleDeclaration work directly with CSS Property ID
+ https://bugs.webkit.org/show_bug.cgi?id=79014
+
+ Reviewed by Geoffrey Garen.
+
+ * fast/dom/CSSStyleDeclaration/access-longest-css-property-expected.txt: Added.
+ * fast/dom/CSSStyleDeclaration/access-longest-css-property.html: Added.
+
2012-02-20 Csaba Osztrogonác <[email protected]>
Unreviewed gardening after r108226.
Added: trunk/LayoutTests/fast/dom/CSSStyleDeclaration/access-longest-css-property-expected.txt (0 => 108257)
--- trunk/LayoutTests/fast/dom/CSSStyleDeclaration/access-longest-css-property-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/CSSStyleDeclaration/access-longest-css-property-expected.txt 2012-02-20 20:43:40 UTC (rev 108257)
@@ -0,0 +1,12 @@
+The test parsing of the boundary value by checking the longest CSS property.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(document.body).webkitMatchNearestMailBlockquoteColor is "match"
+PASS document.getElementById("target").style.webkitMatchNearestMailBlockquoteColor is "match"
+PASS document.body.style.webkitMatchNearestMailBlockquoteColor is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/CSSStyleDeclaration/access-longest-css-property.html (0 => 108257)
--- trunk/LayoutTests/fast/dom/CSSStyleDeclaration/access-longest-css-property.html (rev 0)
+++ trunk/LayoutTests/fast/dom/CSSStyleDeclaration/access-longest-css-property.html 2012-02-20 20:43:40 UTC (rev 108257)
@@ -0,0 +1,24 @@
+<html>
+<head>
+<script src=""
+<style>
+* {
+-webkit-match-nearest-mail-blockquote-color: match;
+}
+</style>
+</head>
+<body>
+<div id="target" style="-webkit-match-nearest-mail-blockquote-color: match;"></div>
+<!--
+Note for updating the test:
+Please replace '-webkit-match-nearest-mail-blockquote-color' by the longest name if '-webkit-match-nearest-mail-blockquote-color' is removed.
+-->
+<script>
+description("The test parsing of the boundary value by checking the longest CSS property.");
+shouldBeEqualToString('getComputedStyle(document.body).webkitMatchNearestMailBlockquoteColor', 'match');
+shouldBeEqualToString('document.getElementById("target").style.webkitMatchNearestMailBlockquoteColor', 'match');
+shouldBeEqualToString('document.body.style.webkitMatchNearestMailBlockquoteColor', '');
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (108256 => 108257)
--- trunk/Source/WebCore/ChangeLog 2012-02-20 19:03:16 UTC (rev 108256)
+++ trunk/Source/WebCore/ChangeLog 2012-02-20 20:43:40 UTC (rev 108257)
@@ -1,3 +1,45 @@
+2012-02-20 Benjamin Poulain <[email protected]>
+
+ Make JSCSSStyleDeclaration work directly with CSS Property ID
+ https://bugs.webkit.org/show_bug.cgi?id=79014
+
+ Reviewed by Geoffrey Garen.
+
+ Previously, accessing the CSS property was done by converting from
+ the _javascript_ name to the CSS name, then converting that name to a lowercase
+ character array, and finally getting the CSS property ID.
+
+ This patch cut the indirection and make the code go directly from the
+ _javascript_ name conversion to the CSS property ID.
+
+ This improves the performance mainly due to the following:
+ -avoid dynamic memory allocation
+ -cut the conversion early when possible
+ -do not parse the string twice
+ The previous fast-path optimization was removed because it is no longer
+ necessary with this change.
+
+ The improvement are the following:
+ -previous fast-path: no change
+ -previous slow-path: ~3 times faster
+
+ Test: fast/dom/CSSStyleDeclaration/access-longest-css-property.html
+ This just test the edge case of CSSPropertyName.
+
+ * bindings/js/JSCSSStyleDeclarationCustom.cpp:
+ (WebCore::writeWebKitPrefix):
+ (WebCore::writeEpubPrefix):
+ (WebCore::cssPropertyIDForJSCSSPropertyName):
+ (WebCore::isCSSPropertyName):
+ (WebCore::JSCSSStyleDeclaration::nameGetter):
+ (WebCore::JSCSSStyleDeclaration::putDelegate):
+ * css/CSSParser.cpp:
+ (WebCore::cssPropertyID):
+ (WebCore):
+ (WebCore::cssPropertyNameIOSAliasing):
+ * css/CSSParser.h:
+ (WebCore):
+
2012-02-20 Dan Bernstein <[email protected]>
Updated Localizable.strings after r107440.
Modified: trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp (108256 => 108257)
--- trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp 2012-02-20 19:03:16 UTC (rev 108256)
+++ trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp 2012-02-20 20:43:40 UTC (rev 108257)
@@ -30,6 +30,7 @@
#include "CSSPrimitiveValue.h"
#include "CSSPropertyNames.h"
#include "CSSValue.h"
+#include "HashTools.h"
#include "JSCSSValue.h"
#include "JSNode.h"
#include "PlatformString.h"
@@ -138,46 +139,51 @@
return PropertyNamePrefixNone;
}
-template<typename CharacterType>
-static inline bool containsASCIIUpperChar(const CharacterType* string, size_t length)
+static inline void writeWebKitPrefix(char*& buffer)
{
- for (unsigned i = 0; i < length; ++i) {
- if (isASCIIUpper(string[i]))
- return true;
- }
- return false;
+ *buffer++ = '-';
+ *buffer++ = 'w';
+ *buffer++ = 'e';
+ *buffer++ = 'b';
+ *buffer++ = 'k';
+ *buffer++ = 'i';
+ *buffer++ = 't';
+ *buffer++ = '-';
}
-static inline bool containsASCIIUpperChar(const StringImpl& string)
+static inline void writeEpubPrefix(char*& buffer)
{
- if (string.is8Bit())
- return containsASCIIUpperChar(string.characters8(), string.length());
- return containsASCIIUpperChar(string.characters16(), string.length());
+ *buffer++ = '-';
+ *buffer++ = 'e';
+ *buffer++ = 'p';
+ *buffer++ = 'u';
+ *buffer++ = 'b';
+ *buffer++ = '-';
}
-static String cssPropertyName(const Identifier& propertyName, bool* hadPixelOrPosPrefix = 0)
+static int cssPropertyIDForJSCSSPropertyName(const Identifier& propertyName, bool* hadPixelOrPosPrefix = 0)
{
if (hadPixelOrPosPrefix)
*hadPixelOrPosPrefix = false;
unsigned length = propertyName.length();
if (!length)
- return String();
+ return 0;
StringImpl* propertyNameString = propertyName.impl();
- // If there is no uppercase character in the propertyName, there can
- // be no prefix, nor extension and we can return the same string.
- if (!containsASCIIUpperChar(*propertyNameString))
- return String(propertyNameString);
+ const size_t bufferSize = maxCSSPropertyNameLength + 1;
+ char buffer[bufferSize];
+ char* bufferPtr = buffer;
+ const char* name = bufferPtr;
- StringBuilder builder;
- builder.reserveCapacity(length);
-
unsigned i = 0;
+ // Prefixes CSS, Pixel, Pos are ignored.
+ // Prefixes Apple, KHTML and Webkit are transposed to "-webkit-".
+ // The prefix "Epub" becomes "-epub-".
switch (getCSSPropertyNamePrefix(*propertyNameString)) {
case PropertyNamePrefixNone:
if (isASCIIUpper((*propertyNameString)[0]))
- return String();
+ return 0;
break;
case PropertyNamePrefixCSS:
i += 3;
@@ -193,28 +199,59 @@
*hadPixelOrPosPrefix = true;
break;
case PropertyNamePrefixApple:
+ case PropertyNamePrefixKHTML:
+ writeWebKitPrefix(bufferPtr);
+ i += 5;
+ break;
case PropertyNamePrefixEpub:
- case PropertyNamePrefixKHTML:
+ writeEpubPrefix(bufferPtr);
+ i += 4;
+ break;
case PropertyNamePrefixWebKit:
- builder.append('-');
+ writeWebKitPrefix(bufferPtr);
+ i += 6;
+ break;
}
- builder.append(toASCIILower((*propertyNameString)[i++]));
+ *bufferPtr++ = toASCIILower((*propertyNameString)[i++]);
+ char* bufferEnd = buffer + bufferSize;
+ char* stringEnd = bufferEnd - 1;
+ size_t bufferSizeLeft = stringEnd - bufferPtr;
+ size_t propertySizeLeft = length - i;
+ if (propertySizeLeft > bufferSizeLeft)
+ return 0;
+
for (; i < length; ++i) {
UChar c = (*propertyNameString)[i];
- if (!isASCIIUpper(c))
- builder.append(c);
- else
- builder.append(makeString('-', toASCIILower(c)));
+ if (!c || c >= 0x7F)
+ return 0; // illegal character
+ if (isASCIIUpper(c)) {
+ size_t bufferSizeLeft = stringEnd - bufferPtr;
+ size_t propertySizeLeft = length - i + 1;
+ if (propertySizeLeft > bufferSizeLeft)
+ return 0;
+ *bufferPtr++ = '-';
+ *bufferPtr++ = toASCIILower(c);
+ } else
+ *bufferPtr++ = c;
+ ASSERT(bufferPtr < bufferEnd);
}
+ ASSERT(bufferPtr < bufferEnd);
+ *bufferPtr = '\0';
- return builder.toString();
+ unsigned outputLength = bufferPtr - buffer;
+#if PLATFORM(IOS)
+ cssPropertyNameIOSAliasing(buffer, name, outputLength);
+#endif
+
+ const Property* hashTableEntry = findProperty(name, outputLength);
+ return hashTableEntry ? hashTableEntry->id : 0;
}
static bool isCSSPropertyName(const Identifier& propertyIdentifier)
{
- return cssPropertyID(cssPropertyName(propertyIdentifier));
+ return cssPropertyIDForJSCSSPropertyName(propertyIdentifier);
}
bool JSCSSStyleDeclaration::canGetItemsForName(ExecState*, CSSStyleDeclaration*, const Identifier& propertyName)
@@ -232,8 +269,8 @@
// positioned element. if it is not a positioned element, return 0
// from MSIE documentation FIXME: IMPLEMENT THAT (Dirk)
bool pixelOrPos;
- String prop = cssPropertyName(propertyName, &pixelOrPos);
- RefPtr<CSSValue> v = thisObj->impl()->getPropertyCSSValue(prop);
+ int cssPropertyID = cssPropertyIDForJSCSSPropertyName(propertyName, &pixelOrPos);
+ RefPtr<CSSValue> v = thisObj->impl()->getPropertyCSSValueInternal(static_cast<CSSPropertyID>(cssPropertyID));
if (v) {
if (pixelOrPos && v->isPrimitiveValue())
return jsNumber(static_pointer_cast<CSSPrimitiveValue>(v)->getFloatValue(CSSPrimitiveValue::CSS_PX));
@@ -242,22 +279,21 @@
// If the property is a shorthand property (such as "padding"),
// it can only be accessed using getPropertyValue.
-
- return jsString(exec, thisObj->impl()->getPropertyValue(prop));
+ return jsString(exec, thisObj->impl()->getPropertyValueInternal(static_cast<CSSPropertyID>(cssPropertyID)));
}
bool JSCSSStyleDeclaration::putDelegate(ExecState* exec, const Identifier& propertyName, JSValue value, PutPropertySlot&)
{
bool pixelOrPos;
- String prop = cssPropertyName(propertyName, &pixelOrPos);
- if (!cssPropertyID(prop))
+ int cssPropertyID = cssPropertyIDForJSCSSPropertyName(propertyName, &pixelOrPos);
+ if (!cssPropertyID)
return false;
String propValue = valueToStringWithNullCheck(exec, value);
if (pixelOrPos)
propValue += "px";
ExceptionCode ec = 0;
- impl()->setProperty(prop, propValue, emptyString(), ec);
+ impl()->setPropertyInternal(static_cast<CSSPropertyID>(cssPropertyID), propValue, false, ec);
setDOMException(exec, ec);
return true;
}
Modified: trunk/Source/WebCore/css/CSSParser.cpp (108256 => 108257)
--- trunk/Source/WebCore/css/CSSParser.cpp 2012-02-20 19:03:16 UTC (rev 108256)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2012-02-20 20:43:40 UTC (rev 108257)
@@ -9138,14 +9138,8 @@
memcpy(buffer, "-webkit", 7);
++length;
}
-
#if PLATFORM(IOS)
- if (!strcmp(buffer, "-webkit-hyphenate-locale")) {
- // Worked in iOS 4.2.
- const char* const webkitLocale = "-webkit-locale";
- name = webkitLocale;
- length = strlen(webkitLocale);
- }
+ cssPropertyNameIOSAliasing(buffer, name, length);
#endif
}
@@ -9163,6 +9157,18 @@
return cssPropertyID(string.characters, string.length);
}
+#if PLATFORM(IOS)
+void cssPropertyNameIOSAliasing(const char* propertyName, const char*& propertyNameAlias, unsigned& newLength)
+{
+ if (!strcmp(propertyName, "-webkit-hyphenate-locale")) {
+ // Worked in iOS 4.2.
+ static const char* const webkitLocale = "-webkit-locale";
+ propertyNameAlias = webkitLocale;
+ newLength = strlen(webkitLocale);
+ }
+}
+#endif
+
int cssValueKeywordID(const CSSParserString& string)
{
unsigned length = string.length;
Modified: trunk/Source/WebCore/css/CSSParser.h (108256 => 108257)
--- trunk/Source/WebCore/css/CSSParser.h 2012-02-20 19:03:16 UTC (rev 108256)
+++ trunk/Source/WebCore/css/CSSParser.h 2012-02-20 20:43:40 UTC (rev 108257)
@@ -449,6 +449,9 @@
int cssPropertyID(const CSSParserString&);
int cssPropertyID(const String&);
int cssValueKeywordID(const CSSParserString&);
+#if PLATFORM(IOS)
+void cssPropertyNameIOSAliasing(const char* propertyName, const char*& propertyNameAlias, unsigned& newLength);
+#endif
class ShorthandScope {
WTF_MAKE_FAST_ALLOCATED;
