Don't expose internal CSSValues in API
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Apr 2012 17:01:06 +0000 (17:01 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Apr 2012 17:01:06 +0000 (17:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=83426

Reviewed by Andreas Kling.

The CSSValues returned from functions like CSSStyleDeclaration.getPropertyCSSValue() are currently
the same instances we use internally. This creates various problems. The values can't be shared between
documents as the wrappers would be shared too. Having to maintain per-document CSSValuePools complicate
the architecture and increase memory usage. This also blocks sharing style sheet data structures
between documents.

This patch adds a concept of CSSOM-safe CSSValue. Only the safe values can be wrapped for JS access.
Values are unsafe by default. The CSSOM functions that return CSSValues create safe instances by
cloning the internal values.

The use of APIs that return CSSValues is very rare (the currect CSSOM draft deprecates them) and
cloning is cheap in any case. Future patches will eliminate the per-document value pool in favor
of a global one for a memory win.

In the future we want to replace internally used CSSValues with true internal types (StyleValues) and
use CSSValues exclusively as wrappers (similar to how CSSStyleRule wraps internal StyleRule).

* bindings/js/JSCSSValueCustom.cpp:
(WebCore::toJS):
* css/CSSComputedStyleDeclaration.cpp:
(WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue):
(WebCore::CSSComputedStyleDeclaration::getPropertyCSSValueInternal):
* css/CSSImageSetValue.cpp:
(WebCore::CSSImageSetValue::CSSImageSetValue):
(WebCore):
(WebCore::CSSImageSetValue::cloneForCSSOM):
* css/CSSImageSetValue.h:
(CSSImageSetValue):
* css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::cleanup):

    Opportunistically fix a string leak for CSS_COUNTER_NAME values.
    Add all cases, remove default.

(WebCore::CSSPrimitiveValue::cloneForCSSOM):
(WebCore):
* css/CSSPrimitiveValue.h:
(CSSPrimitiveValue):
(WebCore::CSSPrimitiveValue::setCSSOMSafe):
* css/CSSValue.cpp:
(WebCore):
(TextCloneCSSValue):
(WebCore::TextCloneCSSValue::create):
(WebCore::TextCloneCSSValue::cssText):
(WebCore::TextCloneCSSValue::TextCloneCSSValue):

    Most non-primitive value types are not exposed in CSSOM. For those we create a dummy value
    that contains only the data that is accessible though the base CSSValue interface.

(WebCore::CSSValue::addSubresourceStyleURLs):
(WebCore::CSSValue::cssText):
(WebCore::CSSValue::destroy):
(WebCore::CSSValue::cloneForCSSOM):
* css/CSSValue.h:
(WebCore):
(CSSValue):
(WebCore::CSSValue::isCSSOMSafe):
(WebCore::CSSValue::isSubtypeExposedToCSSOM):
(WebCore::CSSValue::CSSValue):
* css/CSSValueList.cpp:
(WebCore::CSSValueList::CSSValueList):
(WebCore):
(WebCore::CSSValueList::cloneForCSSOM):
* css/CSSValueList.h:
(CSSValueList):
* css/Counter.h:
(Counter):
(WebCore::Counter::cloneForCSSOM):
* css/PropertySetCSSStyleDeclaration.cpp:
(WebCore::PropertySetCSSStyleDeclaration::setCssText):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValue):
(WebCore::PropertySetCSSStyleDeclaration::setProperty):
(WebCore::PropertySetCSSStyleDeclaration::removeProperty):
(WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValueInternal):
(WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal):
(WebCore::PropertySetCSSStyleDeclaration::didMutate):
(WebCore):
(WebCore::PropertySetCSSStyleDeclaration::cloneAndCacheForCSSOM):

    Maintain a map of safe CSSValues so we can maintain object identity.

* css/PropertySetCSSStyleDeclaration.h:
(WebCore::PropertySetCSSStyleDeclaration::setNeedsStyleRecalc):
(PropertySetCSSStyleDeclaration):
* css/RGBColor.cpp:
(WebCore::RGBColor::red):
(WebCore::RGBColor::green):
(WebCore::RGBColor::blue):
(WebCore::RGBColor::alpha):
* css/Rect.h:
(WebCore::RectBase::RectBase):
(RectBase):
(Rect):
(WebCore::Rect::cloneForCSSOM):
(WebCore::Rect::Rect):
(Quad):
(WebCore::Quad::cloneForCSSOM):
(WebCore::Quad::Quad):
* css/WebKitCSSFilterValue.cpp:
(WebCore::WebKitCSSFilterValue::WebKitCSSFilterValue):
(WebCore):
(WebCore::WebKitCSSFilterValue::cloneForCSSOM):
* css/WebKitCSSFilterValue.h:
(WebKitCSSFilterValue):
* css/WebKitCSSTransformValue.cpp:
(WebCore::WebKitCSSTransformValue::WebKitCSSTransformValue):
(WebCore):
(WebCore::WebKitCSSTransformValue::cloneForCSSOM):
* css/WebKitCSSTransformValue.h:
(WebKitCSSTransformValue):
* svg/SVGColor.cpp:
(WebCore::SVGColor::SVGColor):
(WebCore):
(WebCore::SVGColor::cloneForCSSOM):
* svg/SVGColor.h:
(SVGColor):
* svg/SVGPaint.cpp:
(WebCore::SVGPaint::SVGPaint):
(WebCore):
(WebCore::SVGPaint::cloneForCSSOM):
* svg/SVGPaint.h:
(SVGPaint):
* svg/SVGStyledElement.cpp:
(WebCore::SVGStyledElement::getPresentationAttribute):

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

25 files changed:
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSCSSValueCustom.cpp
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/CSSImageSetValue.cpp
Source/WebCore/css/CSSImageSetValue.h
Source/WebCore/css/CSSPrimitiveValue.cpp
Source/WebCore/css/CSSPrimitiveValue.h
Source/WebCore/css/CSSValue.cpp
Source/WebCore/css/CSSValue.h
Source/WebCore/css/CSSValueList.cpp
Source/WebCore/css/CSSValueList.h
Source/WebCore/css/Counter.h
Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp
Source/WebCore/css/PropertySetCSSStyleDeclaration.h
Source/WebCore/css/RGBColor.cpp
Source/WebCore/css/Rect.h
Source/WebCore/css/WebKitCSSFilterValue.cpp
Source/WebCore/css/WebKitCSSFilterValue.h
Source/WebCore/css/WebKitCSSTransformValue.cpp
Source/WebCore/css/WebKitCSSTransformValue.h
Source/WebCore/svg/SVGColor.cpp
Source/WebCore/svg/SVGColor.h
Source/WebCore/svg/SVGPaint.cpp
Source/WebCore/svg/SVGPaint.h
Source/WebCore/svg/SVGStyledElement.cpp

index 3099de4..9e14cd9 100644 (file)
@@ -1,3 +1,135 @@
+2012-04-09  Antti Koivisto  <antti@apple.com>
+
+        Don't expose internal CSSValues in API
+        https://bugs.webkit.org/show_bug.cgi?id=83426
+        
+        Reviewed by Andreas Kling.
+
+        The CSSValues returned from functions like CSSStyleDeclaration.getPropertyCSSValue() are currently
+        the same instances we use internally. This creates various problems. The values can't be shared between 
+        documents as the wrappers would be shared too. Having to maintain per-document CSSValuePools complicate 
+        the architecture and increase memory usage. This also blocks sharing style sheet data structures 
+        between documents.
+        
+        This patch adds a concept of CSSOM-safe CSSValue. Only the safe values can be wrapped for JS access. 
+        Values are unsafe by default. The CSSOM functions that return CSSValues create safe instances by
+        cloning the internal values.
+        
+        The use of APIs that return CSSValues is very rare (the currect CSSOM draft deprecates them) and
+        cloning is cheap in any case. Future patches will eliminate the per-document value pool in favor
+        of a global one for a memory win.
+        
+        In the future we want to replace internally used CSSValues with true internal types (StyleValues) and
+        use CSSValues exclusively as wrappers (similar to how CSSStyleRule wraps internal StyleRule).
+
+        * bindings/js/JSCSSValueCustom.cpp:
+        (WebCore::toJS):
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue):
+        (WebCore::CSSComputedStyleDeclaration::getPropertyCSSValueInternal):
+        * css/CSSImageSetValue.cpp:
+        (WebCore::CSSImageSetValue::CSSImageSetValue):
+        (WebCore):
+        (WebCore::CSSImageSetValue::cloneForCSSOM):
+        * css/CSSImageSetValue.h:
+        (CSSImageSetValue):
+        * css/CSSPrimitiveValue.cpp:
+        (WebCore::CSSPrimitiveValue::cleanup):
+        
+            Opportunistically fix a string leak for CSS_COUNTER_NAME values.
+            Add all cases, remove default.
+        
+        (WebCore::CSSPrimitiveValue::cloneForCSSOM):
+        (WebCore):
+        * css/CSSPrimitiveValue.h:
+        (CSSPrimitiveValue):
+        (WebCore::CSSPrimitiveValue::setCSSOMSafe):
+        * css/CSSValue.cpp:
+        (WebCore):
+        (TextCloneCSSValue):
+        (WebCore::TextCloneCSSValue::create):
+        (WebCore::TextCloneCSSValue::cssText):
+        (WebCore::TextCloneCSSValue::TextCloneCSSValue):
+        
+            Most non-primitive value types are not exposed in CSSOM. For those we create a dummy value
+            that contains only the data that is accessible though the base CSSValue interface.
+        
+        (WebCore::CSSValue::addSubresourceStyleURLs):
+        (WebCore::CSSValue::cssText):
+        (WebCore::CSSValue::destroy):
+        (WebCore::CSSValue::cloneForCSSOM):
+        * css/CSSValue.h:
+        (WebCore):
+        (CSSValue):
+        (WebCore::CSSValue::isCSSOMSafe):
+        (WebCore::CSSValue::isSubtypeExposedToCSSOM):
+        (WebCore::CSSValue::CSSValue):
+        * css/CSSValueList.cpp:
+        (WebCore::CSSValueList::CSSValueList):
+        (WebCore):
+        (WebCore::CSSValueList::cloneForCSSOM):
+        * css/CSSValueList.h:
+        (CSSValueList):
+        * css/Counter.h:
+        (Counter):
+        (WebCore::Counter::cloneForCSSOM):
+        * css/PropertySetCSSStyleDeclaration.cpp:
+        (WebCore::PropertySetCSSStyleDeclaration::setCssText):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValue):
+        (WebCore::PropertySetCSSStyleDeclaration::setProperty):
+        (WebCore::PropertySetCSSStyleDeclaration::removeProperty):
+        (WebCore::PropertySetCSSStyleDeclaration::getPropertyCSSValueInternal):
+        (WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal):
+        (WebCore::PropertySetCSSStyleDeclaration::didMutate):
+        (WebCore):
+        (WebCore::PropertySetCSSStyleDeclaration::cloneAndCacheForCSSOM):
+        
+            Maintain a map of safe CSSValues so we can maintain object identity.
+        
+        * css/PropertySetCSSStyleDeclaration.h:
+        (WebCore::PropertySetCSSStyleDeclaration::setNeedsStyleRecalc):
+        (PropertySetCSSStyleDeclaration):
+        * css/RGBColor.cpp:
+        (WebCore::RGBColor::red):
+        (WebCore::RGBColor::green):
+        (WebCore::RGBColor::blue):
+        (WebCore::RGBColor::alpha):
+        * css/Rect.h:
+        (WebCore::RectBase::RectBase):
+        (RectBase):
+        (Rect):
+        (WebCore::Rect::cloneForCSSOM):
+        (WebCore::Rect::Rect):
+        (Quad):
+        (WebCore::Quad::cloneForCSSOM):
+        (WebCore::Quad::Quad):
+        * css/WebKitCSSFilterValue.cpp:
+        (WebCore::WebKitCSSFilterValue::WebKitCSSFilterValue):
+        (WebCore):
+        (WebCore::WebKitCSSFilterValue::cloneForCSSOM):
+        * css/WebKitCSSFilterValue.h:
+        (WebKitCSSFilterValue):
+        * css/WebKitCSSTransformValue.cpp:
+        (WebCore::WebKitCSSTransformValue::WebKitCSSTransformValue):
+        (WebCore):
+        (WebCore::WebKitCSSTransformValue::cloneForCSSOM):
+        * css/WebKitCSSTransformValue.h:
+        (WebKitCSSTransformValue):
+        * svg/SVGColor.cpp:
+        (WebCore::SVGColor::SVGColor):
+        (WebCore):
+        (WebCore::SVGColor::cloneForCSSOM):
+        * svg/SVGColor.h:
+        (SVGColor):
+        * svg/SVGPaint.cpp:
+        (WebCore::SVGPaint::SVGPaint):
+        (WebCore):
+        (WebCore::SVGPaint::cloneForCSSOM):
+        * svg/SVGPaint.h:
+        (SVGPaint):
+        * svg/SVGStyledElement.cpp:
+        (WebCore::SVGStyledElement::getPresentationAttribute):
+
 2012-04-09  Pavel Feldman  <pfeldman@chromium.org>
 
         Web Inspector: get rid of WebInspector.Resource.category, use  WebInspector.Resource.type instead.
index 3771e49..ed804b0 100644 (file)
@@ -76,6 +76,9 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, CSSValue* value)
     if (!value)
         return jsNull();
 
+    // Scripts should only ever see cloned CSSValues, never the internal ones.
+    ASSERT(value->isCSSOMSafe());
+
     JSDOMWrapper* wrapper = getCachedWrapper(currentWorld(exec), value);
 
     if (wrapper)
index 6f1face..300a154 100644 (file)
@@ -2656,7 +2656,8 @@ PassRefPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValue(const Stri
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return 0;
-    return getPropertyCSSValue(propertyID);
+    RefPtr<CSSValue> value = getPropertyCSSValue(propertyID);
+    return value ? value->cloneForCSSOM() : 0;
 }
 
 String CSSComputedStyleDeclaration::getPropertyValue(const String &propertyName)
@@ -2696,7 +2697,8 @@ String CSSComputedStyleDeclaration::removeProperty(const String&, ExceptionCode&
     
 PassRefPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValueInternal(CSSPropertyID propertyID)
 {
-    return getPropertyCSSValue(propertyID);
+    RefPtr<CSSValue> value = getPropertyCSSValue(propertyID);
+    return value ? value->cloneForCSSOM() : 0;
 }
 
 String CSSComputedStyleDeclaration::getPropertyValueInternal(CSSPropertyID propertyID)
index 941f704..98fca5a 100644 (file)
@@ -139,6 +139,18 @@ String CSSImageSetValue::customCssText() const
     return "-webkit-image-set(" + CSSValueList::customCssText() + ")";
 }
 
+CSSImageSetValue::CSSImageSetValue(const CSSImageSetValue& cloneFrom)
+    : CSSValueList(cloneFrom)
+    , m_accessedBestFitImage(false)
+{
+    // Non-CSSValueList data is not accessible through CSS OM, no need to clone.
+}
+
+PassRefPtr<CSSImageSetValue> CSSImageSetValue::cloneForCSSOM() const
+{
+    return adoptRef(new CSSImageSetValue(*this));
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(CSS_IMAGE_SET)
index 8174469..404d3f9 100644 (file)
@@ -60,11 +60,14 @@ public:
         float scaleFactor;
     };
 
+    PassRefPtr<CSSImageSetValue> cloneForCSSOM() const;
+
 protected:
     ImageWithScale bestImageForScaleFactor();
 
 private:
     CSSImageSetValue();
+    CSSImageSetValue(const CSSImageSetValue& cloneFrom);
 
     void fillImageSet();
     static inline bool compareByScaleFactor(ImageWithScale first, ImageWithScale second) { return first.scaleFactor < second.scaleFactor; }
index 4f3b376..50cc88a 100644 (file)
@@ -332,41 +332,69 @@ CSSPrimitiveValue::~CSSPrimitiveValue()
 void CSSPrimitiveValue::cleanup()
 {
     switch (m_primitiveUnitType) {
-        case CSS_STRING:
-        case CSS_URI:
-        case CSS_ATTR:
-        case CSS_PARSER_HEXCOLOR:
-            if (m_value.string)
-                m_value.string->deref();
-            break;
-        case CSS_COUNTER:
-            m_value.counter->deref();
-            break;
-        case CSS_RECT:
-            m_value.rect->deref();
-            break;
-        case CSS_QUAD:
-            m_value.quad->deref();
-            break;
-        case CSS_PAIR:
-            m_value.pair->deref();
-            break;
+    case CSS_STRING:
+    case CSS_URI:
+    case CSS_ATTR:
+    case CSS_COUNTER_NAME:
+    case CSS_PARSER_HEXCOLOR:
+        if (m_value.string)
+            m_value.string->deref();
+        break;
+    case CSS_COUNTER:
+        m_value.counter->deref();
+        break;
+    case CSS_RECT:
+        m_value.rect->deref();
+        break;
+    case CSS_QUAD:
+        m_value.quad->deref();
+        break;
+    case CSS_PAIR:
+        m_value.pair->deref();
+        break;
 #if ENABLE(DASHBOARD_SUPPORT)
-        case CSS_DASHBOARD_REGION:
-            if (m_value.region)
-                m_value.region->deref();
-            break;
+    case CSS_DASHBOARD_REGION:
+        if (m_value.region)
+            m_value.region->deref();
+        break;
 #endif
-        case CSS_CALC:
-            m_value.calc->deref();
-            break;
-        case CSS_SHAPE:
-            m_value.shape->deref();
-            break;
-        default:
-            break;
+    case CSS_CALC:
+        m_value.calc->deref();
+        break;
+    case CSS_SHAPE:
+        m_value.shape->deref();
+        break;
+    case CSS_NUMBER:
+    case CSS_PARSER_INTEGER:
+    case CSS_PERCENTAGE:
+    case CSS_EMS:
+    case CSS_EXS:
+    case CSS_REMS:
+    case CSS_PX:
+    case CSS_CM:
+    case CSS_MM:
+    case CSS_IN:
+    case CSS_PT:
+    case CSS_PC:
+    case CSS_DEG:
+    case CSS_RAD:
+    case CSS_GRAD:
+    case CSS_MS:
+    case CSS_S:
+    case CSS_HZ:
+    case CSS_KHZ:
+    case CSS_TURN:
+    case CSS_VW:
+    case CSS_VH:
+    case CSS_VMIN:
+    case CSS_IDENT:
+    case CSS_RGBCOLOR:
+    case CSS_DIMENSION:
+    case CSS_UNKNOWN:
+    case CSS_PARSER_OPERATOR:
+    case CSS_PARSER_IDENTIFIER:
+        break;
     }
-
     m_primitiveUnitType = 0;
     if (m_hasCachedCSSText) {
         cssTextCache().remove(this);
@@ -1058,4 +1086,87 @@ Length CSSPrimitiveValue::viewportPercentageLength()
     return viewportLength;
 }
 
+PassRefPtr<CSSPrimitiveValue> CSSPrimitiveValue::cloneForCSSOM() const
+{
+    RefPtr<CSSPrimitiveValue> result;
+
+    switch (m_primitiveUnitType) {
+    case CSS_STRING:
+    case CSS_URI:
+    case CSS_ATTR:
+    case CSS_COUNTER_NAME:
+        result = CSSPrimitiveValue::create(m_value.string, static_cast<UnitTypes>(m_primitiveUnitType));
+        break;
+    case CSS_COUNTER:
+        result = CSSPrimitiveValue::create(m_value.counter->cloneForCSSOM());
+        break;
+    case CSS_RECT:
+        result = CSSPrimitiveValue::create(m_value.rect->cloneForCSSOM());
+        break;
+    case CSS_QUAD:
+        result = CSSPrimitiveValue::create(m_value.quad->cloneForCSSOM());
+        break;
+    case CSS_PAIR:
+        // Pair is not exposed to the CSSOM, no need for a deep clone.
+        result = CSSPrimitiveValue::create(m_value.pair);
+        break;
+#if ENABLE(DASHBOARD_SUPPORT)
+    case CSS_DASHBOARD_REGION:
+        // DashboardRegion is not exposed to the CSSOM, no need for a deep clone.
+        result = CSSPrimitiveValue::create(m_value.region);
+        break;
+#endif
+    case CSS_CALC:
+        // CSSCalcValue is not exposed to the CSSOM, no need for a deep clone.
+        result = CSSPrimitiveValue::create(m_value.calc);
+        break;
+    case CSS_SHAPE:
+        // CSSShapeValue is not exposed to the CSSOM, no need for a deep clone.
+        result = CSSPrimitiveValue::create(m_value.shape);
+        break;
+    case CSS_NUMBER:
+    case CSS_PARSER_INTEGER:
+    case CSS_PERCENTAGE:
+    case CSS_EMS:
+    case CSS_EXS:
+    case CSS_REMS:
+    case CSS_PX:
+    case CSS_CM:
+    case CSS_MM:
+    case CSS_IN:
+    case CSS_PT:
+    case CSS_PC:
+    case CSS_DEG:
+    case CSS_RAD:
+    case CSS_GRAD:
+    case CSS_MS:
+    case CSS_S:
+    case CSS_HZ:
+    case CSS_KHZ:
+    case CSS_TURN:
+    case CSS_VW:
+    case CSS_VH:
+    case CSS_VMIN:
+        result = CSSPrimitiveValue::create(m_value.num, static_cast<UnitTypes>(m_primitiveUnitType));
+        break;
+    case CSS_IDENT:
+        result = CSSPrimitiveValue::createIdentifier(m_value.ident);
+        break;
+    case CSS_RGBCOLOR:
+        result = CSSPrimitiveValue::createColor(m_value.rgbcolor);
+        break;
+    case CSS_DIMENSION:
+    case CSS_UNKNOWN:
+    case CSS_PARSER_OPERATOR:
+    case CSS_PARSER_IDENTIFIER:
+    case CSS_PARSER_HEXCOLOR:
+        ASSERT_NOT_REACHED();
+        break;
+    }
+    if (result)
+        result->setCSSOMSafe();
+
+    return result;
+}
+
 } // namespace WebCore
index 37453f7..d8499ea 100644 (file)
@@ -275,6 +275,9 @@ public:
     void addSubresourceStyleURLs(ListHashSet<KURL>&, const CSSStyleSheet*);
 
     Length viewportPercentageLength();
+    
+    PassRefPtr<CSSPrimitiveValue> cloneForCSSOM() const;
+    void setCSSOMSafe() { m_isCSSOMSafe = true; }
 
 private:
     // FIXME: int vs. unsigned overloading is too subtle to distinguish the color and identifier cases.
index dbb0563..602416a 100644 (file)
 namespace WebCore {
 
 class SameSizeAsCSSValue : public RefCounted<SameSizeAsCSSValue> {
-    unsigned char bitfields[2];
+    uint32_t bitfields;
 };
 
 COMPILE_ASSERT(sizeof(CSSValue) == sizeof(SameSizeAsCSSValue), CSS_value_should_stay_small);
+    
+class TextCloneCSSValue : public CSSValue {
+public:
+    static PassRefPtr<TextCloneCSSValue> create(ClassType classType, const String& text) { return adoptRef(new TextCloneCSSValue(classType, text)); }
+
+    String cssText() const { return m_cssText; }
+
+private:
+    TextCloneCSSValue(ClassType classType, const String& text) 
+        : CSSValue(classType, /*isCSSOMSafe*/ true)
+        , m_cssText(text)
+    {
+        m_isTextClone = true;
+    }
+
+    String m_cssText;
+};
 
 bool CSSValue::isImplicitInitialValue() const
 {
@@ -84,6 +101,9 @@ CSSValue::Type CSSValue::cssValueType() const
 
 void CSSValue::addSubresourceStyleURLs(ListHashSet<KURL>& urls, const CSSStyleSheet* styleSheet)
 {
+    // This should get called for internal instances only.
+    ASSERT(!isCSSOMSafe());
+
     if (isPrimitiveValue())
         static_cast<CSSPrimitiveValue*>(this)->addSubresourceStyleURLs(urls, styleSheet);
     else if (isValueList())
@@ -96,6 +116,12 @@ void CSSValue::addSubresourceStyleURLs(ListHashSet<KURL>& urls, const CSSStyleSh
 
 String CSSValue::cssText() const
 {
+    if (m_isTextClone) {
+         ASSERT(isCSSOMSafe());
+        return static_cast<const TextCloneCSSValue*>(this)->cssText();
+    }
+    ASSERT(!isCSSOMSafe() || isSubtypeExposedToCSSOM());
+
     switch (classType()) {
     case AspectRatioClass:
         return static_cast<const CSSAspectRatioValue*>(this)->customCssText();
@@ -172,6 +198,13 @@ String CSSValue::cssText() const
 
 void CSSValue::destroy()
 {
+    if (m_isTextClone) {
+        ASSERT(isCSSOMSafe());
+        delete static_cast<TextCloneCSSValue*>(this);
+        return;
+    }
+    ASSERT(!isCSSOMSafe() || isSubtypeExposedToCSSOM());
+
     switch (classType()) {
     case AspectRatioClass:
         delete static_cast<CSSAspectRatioValue*>(this);
@@ -275,4 +308,33 @@ void CSSValue::destroy()
     ASSERT_NOT_REACHED();
 }
 
+PassRefPtr<CSSValue> CSSValue::cloneForCSSOM() const
+{
+    switch (classType()) {
+    case PrimitiveClass:
+        return static_cast<const CSSPrimitiveValue*>(this)->cloneForCSSOM();
+    case ValueListClass:
+        return static_cast<const CSSValueList*>(this)->cloneForCSSOM();
+#if ENABLE(CSS_FILTERS)
+    case WebKitCSSFilterClass:
+        return static_cast<const WebKitCSSFilterValue*>(this)->cloneForCSSOM();
+#endif
+    case WebKitCSSTransformClass:
+        return static_cast<const WebKitCSSTransformValue*>(this)->cloneForCSSOM();
+#if ENABLE(CSS_IMAGE_SET)
+    case ImageSetClass:
+        return static_cast<const CSSImageSetValue*>(this)->cloneForCSSOM();
+#endif
+#if ENABLE(SVG)
+    case SVGColorClass:
+        return static_cast<const SVGColor*>(this)->cloneForCSSOM();
+    case SVGPaintClass:
+        return static_cast<const SVGPaint*>(this)->cloneForCSSOM();
+#endif
+    default:
+        ASSERT(!isSubtypeExposedToCSSOM());
+        return TextCloneCSSValue::create(classType(), cssText());
+    }
+}
+
 }
index 7135f54..6491131 100644 (file)
@@ -21,6 +21,7 @@
 #ifndef CSSValue_h
 #define CSSValue_h
 
+#include "ExceptionCode.h"
 #include "KURLHash.h"
 #include <wtf/ListHashSet.h>
 #include <wtf/RefCounted.h>
 namespace WebCore {
 
 class CSSStyleSheet;
+    
+// FIXME: The current CSSValue and subclasses should be turned into internal types (StyleValue).
+// The few subtypes that are actually exposed in CSSOM can be seen in the cloneForCSSOM() function.
+// They should be handled by separate wrapper classes.
 
-typedef int ExceptionCode;
-
+// Please don't expose more CSSValue types to the web.
 class CSSValue : public RefCounted<CSSValue> {
 public:
     enum Type {
@@ -90,6 +94,18 @@ public:
     bool isSVGColor() const { return m_classType == SVGColorClass || m_classType == SVGPaintClass; }
     bool isSVGPaint() const { return m_classType == SVGPaintClass; }
 #endif
+    
+    bool isCSSOMSafe() const { return m_isCSSOMSafe; }
+    bool isSubtypeExposedToCSSOM() const
+    { 
+        return isPrimitiveValue() 
+#if ENABLE(SVG)
+            || isSVGColor()
+#endif
+            || isValueList();
+    }
+
+    PassRefPtr<CSSValue> cloneForCSSOM() const;
 
     void addSubresourceStyleURLs(ListHashSet<KURL>&, const CSSStyleSheet*);
 
@@ -159,8 +175,10 @@ protected:
 
     ClassType classType() const { return static_cast<ClassType>(m_classType); }
 
-    explicit CSSValue(ClassType classType)
-        : m_primitiveUnitType(0)
+    explicit CSSValue(ClassType classType, bool isCSSOMSafe = false)
+        : m_isCSSOMSafe(isCSSOMSafe)
+        , m_isTextClone(false)
+        , m_primitiveUnitType(0)
         , m_hasCachedCSSText(false)
         , m_isQuirkValue(false)
         , m_valueListSeparator(SpaceSeparator)
@@ -177,18 +195,20 @@ private:
     void destroy();
 
 protected:
+    unsigned m_isCSSOMSafe : 1;
+    unsigned m_isTextClone : 1;
     // The bits in this section are only used by specific subclasses but kept here
     // to maximize struct packing.
 
     // CSSPrimitiveValue bits:
-    unsigned char m_primitiveUnitType : 7; // CSSPrimitiveValue::UnitTypes
-    mutable bool m_hasCachedCSSText : 1;
-    bool m_isQuirkValue : 1;
+    unsigned m_primitiveUnitType : 7; // CSSPrimitiveValue::UnitTypes
+    mutable unsigned m_hasCachedCSSText : 1;
+    unsigned m_isQuirkValue : 1;
 
-    unsigned char m_valueListSeparator : ValueListSeparatorBits;
+    unsigned m_valueListSeparator : ValueListSeparatorBits;
 
 private:
-    unsigned char m_classType : ClassTypeBits; // ClassType
+    unsigned m_classType : ClassTypeBits; // ClassType
 };
 
 } // namespace WebCore
index 07aa789..3b3e6cf 100644 (file)
@@ -143,4 +143,18 @@ void CSSValueList::addSubresourceStyleURLs(ListHashSet<KURL>& urls, const CSSSty
         m_values[i]->addSubresourceStyleURLs(urls, styleSheet);
 }
 
+CSSValueList::CSSValueList(const CSSValueList& cloneFrom)
+    : CSSValue(cloneFrom.classType(), /* isCSSOMSafe */ true)
+{
+    m_valueListSeparator = cloneFrom.m_valueListSeparator;
+    m_values.resize(cloneFrom.m_values.size());
+    for (unsigned i = 0; i < m_values.size(); ++i)
+        m_values[i] = cloneFrom.m_values[i]->cloneForCSSOM();
+}
+
+PassRefPtr<CSSValueList> CSSValueList::cloneForCSSOM() const
+{
+    return adoptRef(new CSSValueList(*this));
+}
+
 } // namespace WebCore
index 16d195b..8a6bbb9 100644 (file)
@@ -61,9 +61,12 @@ public:
     String customCssText() const;
 
     void addSubresourceStyleURLs(ListHashSet<KURL>&, const CSSStyleSheet*);
+    
+    PassRefPtr<CSSValueList> cloneForCSSOM() const;
 
 protected:
     CSSValueList(ClassType, ValueListSeparator);
+    CSSValueList(const CSSValueList& cloneFrom);
 
 private:
     explicit CSSValueList(ValueListSeparator);
index c5c8176..bd3a078 100644 (file)
@@ -42,6 +42,13 @@ public:
     void setIdentifier(PassRefPtr<CSSPrimitiveValue> identifier) { m_identifier = identifier; }
     void setListStyle(PassRefPtr<CSSPrimitiveValue> listStyle) { m_listStyle = listStyle; }
     void setSeparator(PassRefPtr<CSSPrimitiveValue> separator) { m_separator = separator; }
+    
+    PassRefPtr<Counter> cloneForCSSOM() const
+    {
+        return create(m_identifier ? m_identifier->cloneForCSSOM() : 0
+            , m_listStyle ? m_listStyle->cloneForCSSOM() : 0
+            , m_separator ? m_separator->cloneForCSSOM() : 0);
+    }
 
 private:
     Counter(PassRefPtr<CSSPrimitiveValue> identifier, PassRefPtr<CSSPrimitiveValue> listStyle, PassRefPtr<CSSPrimitiveValue> separator)
index e9b41a3..f7ed7c5 100644 (file)
@@ -155,7 +155,7 @@ void PropertySetCSSStyleDeclaration::setCssText(const String& text, ExceptionCod
     // FIXME: Detect syntax errors and set ec.
     m_propertySet->parseDeclaration(text, contextStyleSheet());
 
-    setNeedsStyleRecalc();
+    didMutate();
 #if ENABLE(MUTATION_OBSERVERS)
     mutationScope.enqueueMutationRecord();    
 #endif
@@ -166,7 +166,7 @@ PassRefPtr<CSSValue> PropertySetCSSStyleDeclaration::getPropertyCSSValue(const S
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return 0;
-    return m_propertySet->getPropertyCSSValue(propertyID);
+    return cloneAndCacheForCSSOM(m_propertySet->getPropertyCSSValue(propertyID).get());
 }
 
 String PropertySetCSSStyleDeclaration::getPropertyValue(const String &propertyName)
@@ -218,7 +218,7 @@ void PropertySetCSSStyleDeclaration::setProperty(const String& propertyName, con
     if (changed) {
         // CSS DOM requires raising SYNTAX_ERR of parsing failed, but this is too dangerous for compatibility,
         // see <http://bugs.webkit.org/show_bug.cgi?id=7296>.
-        setNeedsStyleRecalc();
+        didMutate();
 #if ENABLE(MUTATION_OBSERVERS)
         mutationScope.enqueueMutationRecord();
 #endif
@@ -237,7 +237,7 @@ String PropertySetCSSStyleDeclaration::removeProperty(const String& propertyName
     String result;
     bool changes = m_propertySet->removeProperty(propertyID, &result);
     if (changes) {
-        setNeedsStyleRecalc();
+        didMutate();
 #if ENABLE(MUTATION_OBSERVERS)
         mutationScope.enqueueMutationRecord();
 #endif
@@ -247,7 +247,7 @@ String PropertySetCSSStyleDeclaration::removeProperty(const String& propertyName
 
 PassRefPtr<CSSValue> PropertySetCSSStyleDeclaration::getPropertyCSSValueInternal(CSSPropertyID propertyID)
 { 
-    return m_propertySet->getPropertyCSSValue(propertyID); 
+    return cloneAndCacheForCSSOM(m_propertySet->getPropertyCSSValue(propertyID).get());
 }
 
 String PropertySetCSSStyleDeclaration::getPropertyValueInternal(CSSPropertyID propertyID)
@@ -263,13 +263,35 @@ void PropertySetCSSStyleDeclaration::setPropertyInternal(CSSPropertyID propertyI
     ec = 0;
     bool changed = m_propertySet->setProperty(propertyID, value, important, contextStyleSheet());
     if (changed) {
-        setNeedsStyleRecalc();
+        didMutate();
 #if ENABLE(MUTATION_OBSERVERS)
         mutationScope.enqueueMutationRecord();
 #endif
     }
 }
 
+void PropertySetCSSStyleDeclaration::didMutate()
+{
+    m_cssomCSSValueClones.clear();
+    setNeedsStyleRecalc();
+}
+
+CSSValue* PropertySetCSSStyleDeclaration::cloneAndCacheForCSSOM(CSSValue* internalValue)
+{
+    if (!internalValue)
+        return 0;
+
+    // The map is here to maintain the object identity of the CSSValues over multiple invocations.
+    // FIXME: It is likely that the identity is not important for web compatibility and this code should be removed.
+    if (!m_cssomCSSValueClones)
+        m_cssomCSSValueClones = adoptPtr(new HashMap<CSSValue*, RefPtr<CSSValue> >);
+    
+    RefPtr<CSSValue>& clonedValue = m_cssomCSSValueClones->add(internalValue, RefPtr<CSSValue>()).iterator->second;
+    if (!clonedValue)
+        clonedValue = internalValue->cloneForCSSOM();
+    return clonedValue.get();
+}
+
 CSSStyleSheet* PropertySetCSSStyleDeclaration::parentStyleSheet() const
 { 
     return contextStyleSheet(); 
index a5428b6..649bd76 100644 (file)
@@ -69,10 +69,14 @@ private:
     virtual CSSStyleSheet* parentStyleSheet() const OVERRIDE;
     virtual PassRefPtr<StylePropertySet> copy() const OVERRIDE;
     virtual PassRefPtr<StylePropertySet> makeMutable() OVERRIDE;
-    virtual void setNeedsStyleRecalc() { }    
+    virtual void setNeedsStyleRecalc() { }
+    
+    void didMutate();
+    CSSValue* cloneAndCacheForCSSOM(CSSValue*);
     
 protected:
     StylePropertySet* m_propertySet;
+    OwnPtr<HashMap<CSSValue*, RefPtr<CSSValue> > > m_cssomCSSValueClones;
 };
 
 class StyleRuleCSSStyleDeclaration : public PropertySetCSSStyleDeclaration
index 11ebfe4..eeb87c5 100644 (file)
@@ -38,25 +38,33 @@ PassRefPtr<RGBColor> RGBColor::create(unsigned rgbColor)
 PassRefPtr<CSSPrimitiveValue> RGBColor::red()
 {
     unsigned value = (m_rgbColor >> 16) & 0xFF;
-    return CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);
+    RefPtr<CSSPrimitiveValue> result = CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);
+    result->setCSSOMSafe();
+    return result.release();
 }
 
 PassRefPtr<CSSPrimitiveValue> RGBColor::green()
 {
     unsigned value = (m_rgbColor >> 8) & 0xFF;
-    return CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);
+    RefPtr<CSSPrimitiveValue> result = CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);
+    result->setCSSOMSafe();
+    return result.release();
 }
 
 PassRefPtr<CSSPrimitiveValue> RGBColor::blue()
 {
     unsigned value = m_rgbColor & 0xFF;
-    return CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);
+    RefPtr<CSSPrimitiveValue> result = CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);
+    result->setCSSOMSafe();
+    return result.release();
 }
 
 PassRefPtr<CSSPrimitiveValue> RGBColor::alpha()
 {
     float value = static_cast<float>((m_rgbColor >> 24) & 0xFF) / 0xFF;
-    return WebCore::CSSPrimitiveValue::create(value, WebCore::CSSPrimitiveValue::CSS_NUMBER);
+    RefPtr<CSSPrimitiveValue> result = CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);
+    result->setCSSOMSafe();
+    return result.release();
 }
 
 } // namespace WebCore
index b70c6c4..355d6fb 100644 (file)
@@ -40,6 +40,14 @@ public:
 
 protected:
     RectBase() { }
+    RectBase(const RectBase& cloneFrom)
+        : m_top(cloneFrom.m_top ? cloneFrom.m_top->cloneForCSSOM() : 0)
+        , m_right(cloneFrom.m_right ? cloneFrom.m_right->cloneForCSSOM() : 0)
+        , m_bottom(cloneFrom.m_bottom ? cloneFrom.m_bottom->cloneForCSSOM() : 0)
+        , m_left(cloneFrom.m_left ? cloneFrom.m_left->cloneForCSSOM() : 0)
+    {
+    }
+
     ~RectBase() { }
 
 private:
@@ -52,17 +60,23 @@ private:
 class Rect : public RectBase, public RefCounted<Rect> {
 public:
     static PassRefPtr<Rect> create() { return adoptRef(new Rect); }
+    
+    PassRefPtr<Rect> cloneForCSSOM() const { return adoptRef(new Rect(*this)); }
 
 private:
     Rect() { }
+    Rect(const Rect& cloneFrom) : RectBase(cloneFrom), RefCounted<Rect>() { }
 };
 
 class Quad : public RectBase, public RefCounted<Quad> {
 public:
     static PassRefPtr<Quad> create() { return adoptRef(new Quad); }
+    
+    PassRefPtr<Quad> cloneForCSSOM() const { return adoptRef(new Quad(*this)); }
 
 private:
     Quad() { }
+    Quad(const Quad& cloneFrom) : RectBase(cloneFrom), RefCounted<Quad>() { }
 };
 
 } // namespace WebCore
index 2cbbf9c..2916dc2 100644 (file)
@@ -99,6 +99,17 @@ String WebKitCSSFilterValue::customCssText() const
     return result + CSSValueList::customCssText() + ")";
 }
 
+WebKitCSSFilterValue::WebKitCSSFilterValue(const WebKitCSSFilterValue& cloneFrom)
+    : CSSValueList(cloneFrom)
+    , m_type(cloneFrom.m_type)
+{
+}
+
+PassRefPtr<WebKitCSSFilterValue> WebKitCSSFilterValue::cloneForCSSOM() const
+{
+    return adoptRef(new WebKitCSSFilterValue(*this));
+}
+
 }
 
 #endif // ENABLE(CSS_FILTERS)
index 640ea36..a34d574 100644 (file)
@@ -66,8 +66,11 @@ public:
 
     FilterOperationType operationType() const { return m_type; }
 
+    PassRefPtr<WebKitCSSFilterValue> cloneForCSSOM() const;
+
 private:
     WebKitCSSFilterValue(FilterOperationType);
+    WebKitCSSFilterValue(const WebKitCSSFilterValue& cloneFrom);
 
     FilterOperationType m_type;
 };
index 3e2f9e6..dfdd71e 100644 (file)
@@ -115,4 +115,15 @@ String WebKitCSSTransformValue::customCssText() const
     return result;
 }
 
+WebKitCSSTransformValue::WebKitCSSTransformValue(const WebKitCSSTransformValue& cloneFrom)
+    : CSSValueList(cloneFrom)
+    , m_type(cloneFrom.m_type)
+{
+}
+
+PassRefPtr<WebKitCSSTransformValue> WebKitCSSTransformValue::cloneForCSSOM() const
+{
+    return adoptRef(new WebKitCSSTransformValue(*this));
+}
+
 }
index 89e0141..647428b 100644 (file)
@@ -68,9 +68,12 @@ public:
     String customCssText() const;
 
     TransformOperationType operationType() const { return m_type; }
+    
+    PassRefPtr<WebKitCSSTransformValue> cloneForCSSOM() const;
 
 private:
     WebKitCSSTransformValue(TransformOperationType);
+    WebKitCSSTransformValue(const WebKitCSSTransformValue& cloneFrom);
 
     TransformOperationType m_type;
 };
index 8ab12c6..2572b7b 100644 (file)
@@ -92,6 +92,18 @@ String SVGColor::customCssText() const
     return String();
 }
 
+SVGColor::SVGColor(ClassType classType, const SVGColor& cloneFrom)
+    : CSSValue(classType, /*isCSSOMSafe*/ true)
+    , m_color(cloneFrom.m_color)
+    , m_colorType(cloneFrom.m_colorType)
+{
+}
+
+PassRefPtr<SVGColor> SVGColor::cloneForCSSOM() const
+{
+    return adoptRef(new SVGColor(SVGColorClass, *this));
+}
+
 }
 
 #endif // ENABLE(SVG)
index b8e3280..cffac53 100644 (file)
@@ -72,11 +72,14 @@ public:
     String customCssText() const;
 
     ~SVGColor() { }
+    
+    PassRefPtr<SVGColor> cloneForCSSOM() const;
 
 protected:
     friend class CSSComputedStyleDeclaration;
 
     SVGColor(ClassType, const SVGColorType&);
+    SVGColor(ClassType, const SVGColor& cloneFrom);
 
     void setColor(const Color& color) { m_color = color; }
     void setColorType(const SVGColorType& type) { m_colorType = type; }
index 2ec3cc9..8cf3d7e 100644 (file)
@@ -99,6 +99,18 @@ String SVGPaint::customCssText() const
     return String();
 }
 
+SVGPaint::SVGPaint(const SVGPaint& cloneFrom)
+    : SVGColor(SVGPaintClass, cloneFrom)
+    , m_paintType(cloneFrom.m_paintType)
+    , m_uri(cloneFrom.m_uri)
+{
+}
+
+PassRefPtr<SVGPaint> SVGPaint::cloneForCSSOM() const
+{
+    return adoptRef(new SVGPaint(*this));
+}
+
 }
 
 #endif // ENABLE(SVG)
index 0604c91..b053cbd 100644 (file)
@@ -93,6 +93,8 @@ public:
 
     String customCssText() const;
 
+    PassRefPtr<SVGPaint> cloneForCSSOM() const;
+
 private:
     friend class CSSComputedStyleDeclaration;
 
@@ -105,6 +107,7 @@ private:
 
 private:
     SVGPaint(const SVGPaintType&, const String& uri = String());
+    SVGPaint(const SVGPaint& cloneFrom);
 
     SVGPaintType m_paintType;
     String m_uri;
index 75a00ff..7f2c1c5 100644 (file)
@@ -423,7 +423,8 @@ PassRefPtr<CSSValue> SVGStyledElement::getPresentationAttribute(const String& na
     RefPtr<StylePropertySet> style = StylePropertySet::create(SVGAttributeMode);
     CSSPropertyID propertyID = SVGStyledElement::cssPropertyIdForSVGAttributeName(attr->name());
     style->setProperty(propertyID, attr->value());
-    return style->getPropertyCSSValue(propertyID);
+    RefPtr<CSSValue> cssValue = style->getPropertyCSSValue(propertyID);
+    return cssValue ? cssValue->cloneForCSSOM() : 0;
 }
 
 bool SVGStyledElement::instanceUpdatesBlocked() const