Implement ::cue() pseudo element property whitelist
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2013 18:22:46 +0000 (18:22 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2013 18:22:46 +0000 (18:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=105477

Patch by Dima Gorbik <dgorbik@apple.com> on 2013-01-18
Reviewed by Antti Koivisto.

Source/WebCore:

Only allowed by specs properties are applied to WebVTT nodes. We used whitelists before for the region
style rule so this refactors the code a little bit to pass bitfields that contain the information on which
whitelists should be used.

Test: media/track/track-css-property-whitelist.html

* css/RuleSet.cpp:
(WebCore::RuleData::RuleData): set a region bit when creating a RuleData.
(WebCore::RuleSet::addRule): set a cue bit when PseudoCue type is set for a selector.
* css/RuleSet.h: add the new bitfield as an ivar and setters/getters to access it.
(WebCore::RuleData::isInRegionRule):
(WebCore::RuleData::whitelistType):
(WebCore::RuleData::isInCueRule):
(WebCore::RuleData::setIsInCueRule):
* css/StyleResolver.cpp: refactor the code to pass a bitfield instead of single bits.
(WebCore::StyleResolver::addMatchedProperties):
(WebCore::StyleResolver::sortAndTransferMatchedRules):
(WebCore::StyleResolver::applyProperties):
(WebCore::StyleResolver::applyMatchedProperties):
(WebCore::StyleResolver::isValidCueStyleProperty):
* css/StyleResolver.h:

LayoutTests:

* media/track/captions-webvtt/whitelist.vtt: Added.
* media/track/track-css-property-whitelist-expected.txt: Added.
* media/track/track-css-property-whitelist.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/media/track/captions-webvtt/whitelist.vtt [new file with mode: 0644]
LayoutTests/media/track/track-css-property-whitelist-expected.txt [new file with mode: 0644]
LayoutTests/media/track/track-css-property-whitelist.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/RuleSet.cpp
Source/WebCore/css/RuleSet.h
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h

index 499f41d..c2342e3 100644 (file)
@@ -1,3 +1,14 @@
+2013-01-18  Dima Gorbik  <dgorbik@apple.com>
+
+        Implement ::cue() pseudo element property whitelist
+        https://bugs.webkit.org/show_bug.cgi?id=105477
+
+        Reviewed by Antti Koivisto.
+
+        * media/track/captions-webvtt/whitelist.vtt: Added.
+        * media/track/track-css-property-whitelist-expected.txt: Added.
+        * media/track/track-css-property-whitelist.html: Added.
+
 2013-01-18  Tony Chang  <tony@chromium.org>
 
         Incorrect scrollable height during simplified layout
diff --git a/LayoutTests/media/track/captions-webvtt/whitelist.vtt b/LayoutTests/media/track/captions-webvtt/whitelist.vtt
new file mode 100644 (file)
index 0000000..388571c
--- /dev/null
@@ -0,0 +1,5 @@
+WEBVTT
+
+1
+00:00.000 --> 00:00.200
+<c.red>One,</c> <c>two,</c> <c>three.</c>
diff --git a/LayoutTests/media/track/track-css-property-whitelist-expected.txt b/LayoutTests/media/track/track-css-property-whitelist-expected.txt
new file mode 100644 (file)
index 0000000..352cf74
--- /dev/null
@@ -0,0 +1,13 @@
+EVENT(canplaythrough)
+EVENT(seeked)
+
+
+Test that only allowed for the ::cue pseudo-element properties are applied to WebVTT node objects.
+EXPECTED (getComputedStyle(cueNode).color == 'rgb(255, 0, 0)') OK
+EXPECTED (getComputedStyle(cueNode).padding == '0px') OK
+EXPECTED (getComputedStyle(cueNode).color == 'rgb(255, 0, 0)') OK
+EXPECTED (getComputedStyle(cueNode).padding == '0px') OK
+EXPECTED (getComputedStyle(cueNode).color == 'rgb(255, 0, 0)') OK
+EXPECTED (getComputedStyle(cueNode).padding == '0px') OK
+END OF TEST
+
diff --git a/LayoutTests/media/track/track-css-property-whitelist.html b/LayoutTests/media/track/track-css-property-whitelist.html
new file mode 100644 (file)
index 0000000..de21e6b
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+
+        <script src=../media-file.js></script>
+        <script src=../video-test.js></script>
+        <script src=../media-controls.js></script>
+
+        <style>
+        ::cue(c) {padding-left: 10px; color: red;}
+        </style>
+
+        <script>
+
+        var cueNode;
+
+        function skipNonElements(root)
+        {
+            nextElementSibling = root;
+            while (nextElementSibling.nodeType != 1) {
+                nextElementSibling = nextElementSibling.nextSibling;
+            }
+            return nextElementSibling;
+        }
+
+        function seeked()
+        {
+            if (testEnded)
+                return;
+
+            consoleWrite("<br>");
+            consoleWrite("Test that only allowed for the ::cue pseudo-element properties are applied to WebVTT node objects.");
+
+            cueNode = skipNonElements(textTrackDisplayElement(video, 'all-nodes').firstChild);
+            skipNonElements(cueNode);
+            testExpected("getComputedStyle(cueNode).color", "rgb(255, 0, 0)");
+            testExpected("getComputedStyle(cueNode).padding", "0px");
+            cueNode = skipNonElements(cueNode.nextSibling);
+            testExpected("getComputedStyle(cueNode).color", "rgb(255, 0, 0)");
+            testExpected("getComputedStyle(cueNode).padding", "0px");
+            cueNode = skipNonElements(cueNode.nextSibling);
+            testExpected("getComputedStyle(cueNode).color", "rgb(255, 0, 0)");
+            testExpected("getComputedStyle(cueNode).padding", "0px");
+            endTest();
+        }
+
+        function loaded()
+        {
+            findMediaElement();
+            video.src = findMediaFile('video', '../content/test');
+            video.id = "testvideo";
+            waitForEvent('seeked', seeked);
+            waitForEvent('canplaythrough', function() { video.currentTime = 0.1; });
+        }
+
+        </script>
+    </head>
+    <body onload="loaded()">
+        <video controls >
+            <track src="captions-webvtt/whitelist.vtt" kind="captions" default>
+        </video>
+    </body>
+</html>
index ae7d63d..611b16d 100644 (file)
@@ -1,3 +1,32 @@
+2013-01-18  Dima Gorbik  <dgorbik@apple.com>
+
+        Implement ::cue() pseudo element property whitelist
+        https://bugs.webkit.org/show_bug.cgi?id=105477
+
+        Reviewed by Antti Koivisto.
+
+        Only allowed by specs properties are applied to WebVTT nodes. We used whitelists before for the region
+        style rule so this refactors the code a little bit to pass bitfields that contain the information on which 
+        whitelists should be used.
+
+        Test: media/track/track-css-property-whitelist.html
+
+        * css/RuleSet.cpp: 
+        (WebCore::RuleData::RuleData): set a region bit when creating a RuleData.
+        (WebCore::RuleSet::addRule): set a cue bit when PseudoCue type is set for a selector.
+        * css/RuleSet.h: add the new bitfield as an ivar and setters/getters to access it.
+        (WebCore::RuleData::isInRegionRule):
+        (WebCore::RuleData::whitelistType):
+        (WebCore::RuleData::isInCueRule):
+        (WebCore::RuleData::setIsInCueRule):
+        * css/StyleResolver.cpp: refactor the code to pass a bitfield instead of single bits.
+        (WebCore::StyleResolver::addMatchedProperties):
+        (WebCore::StyleResolver::sortAndTransferMatchedRules):
+        (WebCore::StyleResolver::applyProperties):
+        (WebCore::StyleResolver::applyMatchedProperties):
+        (WebCore::StyleResolver::isValidCueStyleProperty): 
+        * css/StyleResolver.h:
+
 2013-01-18  Tony Chang  <tony@chromium.org>
 
         Incorrect scrollable height during simplified layout
index a7c32df..c677151 100644 (file)
@@ -108,18 +108,29 @@ static inline bool containsUncommonAttributeSelector(const CSSSelector* selector
     return false;
 }
 
+static inline PropertyWhitelistType determinePropertyWhitelistType(const AddRuleFlags addRuleFlags, const CSSSelector* selector)
+{
+    if (addRuleFlags & RuleIsInRegionRule)
+        return PropertyWhitelistRegion;
+#if ENABLE(VIDEO_TRACK)
+    if (selector->pseudoType() == CSSSelector::PseudoCue)
+        return PropertyWhitelistCue;
+#endif
+    return PropertyWhitelistNone;
+}
+
 RuleData::RuleData(StyleRule* rule, unsigned selectorIndex, unsigned position, AddRuleFlags addRuleFlags)
     : m_rule(rule)
     , m_selectorIndex(selectorIndex)
     , m_position(position)
-    , m_specificity(selector()->specificity())
     , m_hasFastCheckableSelector((addRuleFlags & RuleCanUseFastCheckSelector) && SelectorChecker::isFastCheckableSelector(selector()))
+    , m_specificity(selector()->specificity())
     , m_hasMultipartSelector(!!selector()->tagHistory())
     , m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash(isSelectorMatchingHTMLBasedOnRuleHash(selector()))
     , m_containsUncommonAttributeSelector(WebCore::containsUncommonAttributeSelector(selector()))
     , m_linkMatchType(SelectorChecker::determineLinkMatchType(selector()))
     , m_hasDocumentSecurityOrigin(addRuleFlags & RuleHasDocumentSecurityOrigin)
-    , m_isInRegionRule(!!(addRuleFlags & RuleIsInRegionRule))
+    , m_propertyWhitelistType(determinePropertyWhitelistType(addRuleFlags, selector()))
 {
     ASSERT(m_position == position);
     ASSERT(m_selectorIndex == selectorIndex);
index edfefd3..03024be 100644 (file)
@@ -37,6 +37,14 @@ enum AddRuleFlags {
     RuleCanUseFastCheckSelector   = 1 << 1,
     RuleIsInRegionRule            = 1 << 2,
 };
+    
+enum PropertyWhitelistType {
+    PropertyWhitelistNone   = 0,
+    PropertyWhitelistRegion,
+#if ENABLE(VIDEO_TRACK)
+    PropertyWhitelistCue
+#endif
+};
 
 class CSSSelector;
 class ContainerNode;
@@ -61,8 +69,7 @@ public:
     unsigned specificity() const { return m_specificity; }
     unsigned linkMatchType() const { return m_linkMatchType; }
     bool hasDocumentSecurityOrigin() const { return m_hasDocumentSecurityOrigin; }
-    bool isInRegionRule() const { return m_isInRegionRule; }
-
+    PropertyWhitelistType propertyWhitelistType() const { return static_cast<PropertyWhitelistType>(m_propertyWhitelistType); }
     // Try to balance between memory usage (there can be lots of RuleData objects) and good filtering performance.
     static const unsigned maximumIdentifierCount = 4;
     const unsigned* descendantSelectorIdentifierHashes() const { return m_descendantSelectorIdentifierHashes; }
@@ -74,15 +81,15 @@ private:
     unsigned m_selectorIndex : 12;
     // This number was picked fairly arbitrarily. We can probably lower it if we need to.
     // Some simple testing showed <100,000 RuleData's on large sites.
-    unsigned m_position : 20;
-    unsigned m_specificity : 24;
+    unsigned m_position : 19;
     unsigned m_hasFastCheckableSelector : 1;
+    unsigned m_specificity : 24;
     unsigned m_hasMultipartSelector : 1;
     unsigned m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash : 1;
     unsigned m_containsUncommonAttributeSelector : 1;
     unsigned m_linkMatchType : 2; //  SelectorChecker::LinkMatchMask
     unsigned m_hasDocumentSecurityOrigin : 1;
-    unsigned m_isInRegionRule : 1;
+    unsigned m_propertyWhitelistType : 2;
     // Use plain array instead of a Vector to minimize memory overhead.
     unsigned m_descendantSelectorIdentifierHashes[maximumIdentifierCount];
 };
index 96710d6..dcca3e4 100644 (file)
@@ -593,13 +593,13 @@ static void ensureDefaultStyleSheetsForElement(Element* element)
     ASSERT(mathMLStyleSheet || defaultStyle->features().siblingRules.isEmpty());
 }
 
-void StyleResolver::addMatchedProperties(MatchResult& matchResult, const StylePropertySet* properties, StyleRule* rule, unsigned linkMatchType, bool inRegionRule)
+void StyleResolver::addMatchedProperties(MatchResult& matchResult, const StylePropertySet* properties, StyleRule* rule, unsigned linkMatchType, PropertyWhitelistType propertyWhitelistType)
 {
     matchResult.matchedProperties.grow(matchResult.matchedProperties.size() + 1);
     MatchedProperties& newProperties = matchResult.matchedProperties.last();
     newProperties.properties = const_cast<StylePropertySet*>(properties);
     newProperties.linkMatchType = linkMatchType;
-    newProperties.isInRegionRule = inRegionRule;
+    newProperties.whitelistType = propertyWhitelistType;
     matchResult.matchedRules.append(rule);
 }
 
@@ -726,7 +726,7 @@ void StyleResolver::sortAndTransferMatchedRules(MatchResult& result)
         unsigned linkMatchType = m_matchedRules[i]->linkMatchType();
         if (swapVisitedUnvisited && linkMatchType && linkMatchType != SelectorChecker::MatchAll)
             linkMatchType = (linkMatchType == SelectorChecker::MatchVisited) ? SelectorChecker::MatchLink : SelectorChecker::MatchVisited;
-        addMatchedProperties(result, m_matchedRules[i]->rule()->properties(), m_matchedRules[i]->rule(), linkMatchType, m_matchedRules[i]->isInRegionRule());
+        addMatchedProperties(result, m_matchedRules[i]->rule()->properties(), m_matchedRules[i]->rule(), linkMatchType, m_matchedRules[i]->propertyWhitelistType());
     }
 }
 
@@ -2311,9 +2311,9 @@ Length StyleResolver::convertToFloatLength(CSSPrimitiveValue* primitiveValue, Re
 }
 
 template <StyleResolver::StyleApplicationPass pass>
-void StyleResolver::applyProperties(const StylePropertySet* properties, StyleRule* rule, bool isImportant, bool inheritedOnly, bool filterRegionProperties)
+void StyleResolver::applyProperties(const StylePropertySet* properties, StyleRule* rule, bool isImportant, bool inheritedOnly, PropertyWhitelistType propertyWhitelistType)
 {
-    ASSERT(!filterRegionProperties || m_regionForStyling);
+    ASSERT((propertyWhitelistType != PropertyWhitelistRegion) || m_regionForStyling);
     InspectorInstrumentationCookie cookie = InspectorInstrumentation::willProcessRule(document(), rule, this);
 
     unsigned propertyCount = properties->propertyCount();
@@ -2330,9 +2330,12 @@ void StyleResolver::applyProperties(const StylePropertySet* properties, StyleRul
         }
         CSSPropertyID property = current.id();
 
-        if (filterRegionProperties && !StyleResolver::isValidRegionStyleProperty(property))
+        if (propertyWhitelistType == PropertyWhitelistRegion && !StyleResolver::isValidRegionStyleProperty(property))
             continue;
-
+#if ENABLE(VIDEO_TRACK)
+        if (propertyWhitelistType == PropertyWhitelistCue && !StyleResolver::isValidCueStyleProperty(property))
+            continue;
+#endif
         switch (pass) {
 #if ENABLE(CSS_VARIABLES)
         case VariableDefinitions:
@@ -2378,7 +2381,7 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, bool
             m_applyPropertyToRegularStyle = linkMatchType & SelectorChecker::MatchLink;
             m_applyPropertyToVisitedLinkStyle = linkMatchType & SelectorChecker::MatchVisited;
 
-            applyProperties<pass>(matchedProperties.properties.get(), matchResult.matchedRules[i], isImportant, inheritedOnly, matchedProperties.isInRegionRule);
+            applyProperties<pass>(matchedProperties.properties.get(), matchResult.matchedRules[i], isImportant, inheritedOnly, static_cast<PropertyWhitelistType>(matchedProperties.whitelistType));
         }
         m_applyPropertyToRegularStyle = true;
         m_applyPropertyToVisitedLinkStyle = false;
@@ -2386,7 +2389,7 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, bool
     }
     for (int i = startIndex; i <= endIndex; ++i) {
         const MatchedProperties& matchedProperties = matchResult.matchedProperties[i];
-        applyProperties<pass>(matchedProperties.properties.get(), matchResult.matchedRules[i], isImportant, inheritedOnly, matchedProperties.isInRegionRule);
+        applyProperties<pass>(matchedProperties.properties.get(), matchResult.matchedRules[i], isImportant, inheritedOnly, static_cast<PropertyWhitelistType>(matchedProperties.whitelistType));
     }
 }
 
@@ -2774,6 +2777,49 @@ inline bool StyleResolver::isValidRegionStyleProperty(CSSPropertyID id)
     return false;
 }
 
+#if ENABLE(VIDEO_TRACK)
+inline bool StyleResolver::isValidCueStyleProperty(CSSPropertyID id)
+{
+    switch (id) {
+    case CSSPropertyBackground:
+    case CSSPropertyBackgroundAttachment:
+    case CSSPropertyBackgroundClip:
+    case CSSPropertyBackgroundColor:
+    case CSSPropertyBackgroundImage:
+    case CSSPropertyBackgroundOrigin:
+    case CSSPropertyBackgroundPosition:
+    case CSSPropertyBackgroundPositionX:
+    case CSSPropertyBackgroundPositionY:
+    case CSSPropertyBackgroundRepeat:
+    case CSSPropertyBackgroundRepeatX:
+    case CSSPropertyBackgroundRepeatY:
+    case CSSPropertyBackgroundSize:
+    case CSSPropertyColor:
+    case CSSPropertyFont:
+    case CSSPropertyFontFamily:
+    case CSSPropertyFontSize:
+    case CSSPropertyFontStyle:
+    case CSSPropertyFontVariant:
+    case CSSPropertyFontWeight:
+    case CSSPropertyLineHeight:
+    case CSSPropertyOpacity:
+    case CSSPropertyOutline:
+    case CSSPropertyOutlineColor:
+    case CSSPropertyOutlineOffset:
+    case CSSPropertyOutlineStyle:
+    case CSSPropertyOutlineWidth:
+    case CSSPropertyVisibility:
+    case CSSPropertyWhiteSpace:
+    case CSSPropertyTextDecoration:
+    case CSSPropertyTextShadow:
+    case CSSPropertyBorderStyle:
+        return true;
+    default:
+        break;
+    }
+    return false;
+}
+#endif
 // SVG handles zooming in a different way compared to CSS. The whole document is scaled instead
 // of each individual length value in the render style / tree. CSSPrimitiveValue::computeLength*()
 // multiplies each resolved length with the zoom multiplier - so for SVG we need to disable that.
index dbeb6ea..3c434f2 100644 (file)
@@ -29,6 +29,7 @@
 #include "MediaQueryExp.h"
 #include "RenderStyle.h"
 #include "RuleFeature.h"
+#include "RuleSet.h"
 #include "RuntimeEnabledFeatures.h"
 #include "SelectorChecker.h"
 #include "SelectorFilter.h"
@@ -327,7 +328,7 @@ private:
         union {
             struct {
                 unsigned linkMatchType : 2;
-                unsigned isInRegionRule : 1;
+                unsigned whitelistType : 2;
             };
             // Used to make sure all memory is zero-initialized since we compute the hash over the bytes of this object.
             void* possiblyPaddedMember;
@@ -352,7 +353,7 @@ private:
         const ContainerNode* scope;
     };
 
-    static void addMatchedProperties(MatchResult&, const StylePropertySet* properties, StyleRule* = 0, unsigned linkMatchType = SelectorChecker::MatchAll, bool inRegionRule = false);
+    static void addMatchedProperties(MatchResult&, const StylePropertySet* properties, StyleRule* = 0, unsigned linkMatchType = SelectorChecker::MatchAll, PropertyWhitelistType = PropertyWhitelistNone);
     void addElementStyleProperties(MatchResult&, const StylePropertySet*, bool isCacheable = true);
 
     void matchAllRules(MatchResult&, bool includeSMILProperties);
@@ -384,12 +385,14 @@ private:
     template <StyleApplicationPass pass>
     void applyMatchedProperties(const MatchResult&, bool important, int startIndex, int endIndex, bool inheritedOnly);
     template <StyleApplicationPass pass>
-    void applyProperties(const StylePropertySet* properties, StyleRule*, bool isImportant, bool inheritedOnly, bool filterRegionProperties);
+    void applyProperties(const StylePropertySet* properties, StyleRule*, bool isImportant, bool inheritedOnly, PropertyWhitelistType = PropertyWhitelistNone);
 #if ENABLE(CSS_VARIABLES)
     void resolveVariables(CSSPropertyID, CSSValue*, Vector<std::pair<CSSPropertyID, String> >& knownExpressions);
 #endif
     static bool isValidRegionStyleProperty(CSSPropertyID);
-
+#if ENABLE(VIDEO_TRACK)
+    static bool isValidCueStyleProperty(CSSPropertyID);
+#endif
     void matchPageRules(MatchResult&, RuleSet*, bool isLeftPage, bool isFirstPage, const String& pageName);
     void matchPageRulesForList(Vector<StyleRulePage*>& matchedRules, const Vector<StyleRulePage*>&, bool isLeftPage, bool isFirstPage, const String& pageName);
     Settings* documentSettings() { return m_document->settings(); }