Changeset 155536 in webkit for trunk/Source/WebCore/css/CSSParser.cpp

Timestamp:

Sep 11, 2013, 9:48:20 AM (12 years ago)

Author:

Darin Adler

Message:

Rework CSS parser, eliminating "floating" concept and using %destructor
​https://bugs.webkit.org/show_bug.cgi?id=121161

Reviewed by Antti Koivisto.

This is some basic improvement, but there is still room for a lot more
consistent approach and formatting in this file. There is a mix of code that
works by side effects in the CSSParser class and code that works with the
values that bison generates in the union that is more or less random. And
the data structures seem too costly, with too much heap allocation. And the
CSSParser class has grown massive, with a mix of both function for use by
code that wants to trigger parsing, and helper functions called by code in
the grammar file. All of that can benefit from more refinement in the future.

• css/CSSGrammar.y.in: Made some incremental improvements to the structure

of the grammar file, including:

• Breaking up the %union so types are declared next to their use
• Eliminating one shift/reduce conflict caused by two "maybe_space" in a row
• Breaking the conditional sections out into their own sections instead of scattering them in with the other code.
• Eliminating unused return values in productions such as charset, ignored_charset, namespace, margin_box, invalid_rule, save_block, invalid_at, and declarations_and_margins.
• Adding %destructor to productions that return values that need to be deleted or deref'd. This removes the need for CSSParser to separately track these as "floating" to clean up in case of errors.
• Removing unneeded productions such as media_feature, region_selector, attr_name, and medium.
• Removing explicit code blocks that just say "$$ = $1" or empty blocks when there is no return type, since those are default.
• Formatting many productions on single lines since I find them easier to read. Later I think we could make many more CSSParser functions and make even more of the production single lines in the grammar file.
• Using adoptPtr, adoptRef, delete, deref, leakPtr, and leakRef to put heap allocated values into and out of the union without storage leaks.

• css/CSSParser.cpp:

(WebCore::CSSParser::~CSSParser): Remove the now-unneeded deleteAllValues for
the various colections of floating things.
(WebCore::CSSParser::createFilterRule): Don't put the rule into m_parsedRules,
just return a PassRefPtr instead.
(WebCore::CSSParser::createImportRule): Ditto.
(WebCore::CSSParser::createMediaRule): Ditto.
(WebCore::CSSParser::createEmptyMediaRule): Ditto.
(WebCore::CSSParser::createSupportsRule): Ditto.
(WebCore::CSSParser::createKeyframesRule): Ditto.
(WebCore::CSSParser::createStyleRule): Ditto.
(WebCore::CSSParser::createFontFaceRule): Ditto.
(WebCore::CSSParser::createHostRule): Ditto.
(WebCore::CSSParser::rewriteSpecifiersWithNamespaceIfNeeded): Got rid of the
unused return value from this function and changed it to to take a reference
instead of a pointer.
(WebCore::CSSParser::rewriteSpecifiersWithElementName): Ditto.
(WebCore::CSSParser::rewriteSpecifiers): Changed this to take and return OwnPtr.
(WebCore::CSSParser::createPageRule): Don't put rule into m_parsedRules, return
PassRefPtr instead.
(WebCore::CSSParser::createSelectorVector): Added. Used to implement the
optimization where we recycle a single selector vector. Not sure we still need
this, or maybe we need more optimizations like it, but for now keep it.
(WebCore::CSSParser::recycleSelectorVector): Ditto.
(WebCore::CSSParser::createRegionRule): Don't put rule into m_parsedRules, return
PassRefPtr instead.
(WebCore::CSSParser::createMarginAtRule): Got rid of unused return value.
When this function is implemented for real, we might add a return value.
(WebCore::CSSParser::createKeyframe): Don't put keyframe into m_parsedKeyframes,
return PassRefPtr instead.
(WebCore::CSSParser::createViewportRule): Don't put rule into m_parsedRules, return
PassRefPtr instead.

• css/CSSParser.h: Removed many now-needed functions to manage floating items.

Changed rule creation functions to return PassRefPtr. Other changes as mentioned above.

• css/CSSParserValues.cpp:

(WebCore::destroy): Added.
(WebCore::CSSParserValueList::~CSSParserValueList): Updated to call destroy.

• css/CSSParserValues.h: Ditto.

File:

• trunk/Source/WebCore/css/CSSParser.cpp (modified) (22 diffs)

trunk/Source/WebCore/css/CSSParser.cpp

@@ -358 +358 @@
 {
     clearProperties();
-
-    deleteAllValues(m_floatingSelectors);
-    deleteAllValues(m_floatingSelectorVectors);
-    deleteAllValues(m_floatingValueLists);
-    deleteAllValues(m_floatingFunctions);
 }
 
@@ -523 +518 @@
 #if ENABLE(CSS3_TEXT)
     case CSSPropertyWebkitTextDecorationColor:
-#endif // CSS3_TEXT
+#endif
     case CSSPropertyWebkitTextEmphasisColor:
     case CSSPropertyWebkitTextFillColor:
@@ -9238 +9233 @@
 }
 
-StyleRuleBase* CSSParser::createFilterRule(const CSSParserString& filterName)
+PassRefPtr<StyleRuleBase> CSSParser::createFilterRule(const CSSParserString& filterName)
 {
     RefPtr<StyleRuleFilter> rule = StyleRuleFilter::create(filterName, createStylePropertySet());
     clearProperties();
-    StyleRuleFilter* result = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
-    return result;
+    return rule.release();
 }
 
@@ -11465 +11458 @@
 }
 
-CSSParserSelector* CSSParser::createFloatingSelectorWithTagName(const QualifiedName& tagQName)
-{
-    CSSParserSelector* selector = new CSSParserSelector(tagQName);
-    m_floatingSelectors.add(selector);
-    return selector;
-}
-
-CSSParserSelector* CSSParser::createFloatingSelector()
-{
-    CSSParserSelector* selector = new CSSParserSelector;
-    m_floatingSelectors.add(selector);
-    return selector;
-}
-
-PassOwnPtr<CSSParserSelector> CSSParser::sinkFloatingSelector(CSSParserSelector* selector)
-{
-    if (selector) {
-        ASSERT(m_floatingSelectors.contains(selector));
-        m_floatingSelectors.remove(selector);
-    }
-    return adoptPtr(selector);
-}
-
-Vector<OwnPtr<CSSParserSelector> >* CSSParser::createFloatingSelectorVector()
-{
-    Vector<OwnPtr<CSSParserSelector> >* selectorVector = new Vector<OwnPtr<CSSParserSelector> >;
-    m_floatingSelectorVectors.add(selectorVector);
-    return selectorVector;
-}
-
-PassOwnPtr<Vector<OwnPtr<CSSParserSelector> > > CSSParser::sinkFloatingSelectorVector(Vector<OwnPtr<CSSParserSelector> >* selectorVector)
-{
-    if (selectorVector) {
-        ASSERT(m_floatingSelectorVectors.contains(selectorVector));
-        m_floatingSelectorVectors.remove(selectorVector);
-    }
-    return adoptPtr(selectorVector);
-}
-
-CSSParserValueList* CSSParser::createFloatingValueList()
-{
-    CSSParserValueList* list = new CSSParserValueList;
-    m_floatingValueLists.add(list);
-    return list;
-}
-
-PassOwnPtr<CSSParserValueList> CSSParser::sinkFloatingValueList(CSSParserValueList* list)
-{
-    if (list) {
-        ASSERT(m_floatingValueLists.contains(list));
-        m_floatingValueLists.remove(list);
-    }
-    return adoptPtr(list);
-}
-
-CSSParserFunction* CSSParser::createFloatingFunction()
-{
-    CSSParserFunction* function = new CSSParserFunction;
-    m_floatingFunctions.add(function);
-    return function;
-}
-
-PassOwnPtr<CSSParserFunction> CSSParser::sinkFloatingFunction(CSSParserFunction* function)
-{
-    if (function) {
-        ASSERT(m_floatingFunctions.contains(function));
-        m_floatingFunctions.remove(function);
-    }
-    return adoptPtr(function);
-}
-
-CSSParserValue& CSSParser::sinkFloatingValue(CSSParserValue& value)
-{
-    if (value.unit == CSSParserValue::Function) {
-        ASSERT(m_floatingFunctions.contains(value.function));
-        m_floatingFunctions.remove(value.function);
-    }
-    return value;
-}
-
-MediaQueryExp* CSSParser::createFloatingMediaQueryExp(const AtomicString& mediaFeature, CSSParserValueList* values)
-{
-    m_floatingMediaQueryExp = MediaQueryExp::create(mediaFeature, values);
-    return m_floatingMediaQueryExp.get();
-}
-
-PassOwnPtr<MediaQueryExp> CSSParser::sinkFloatingMediaQueryExp(MediaQueryExp* expression)
-{
-    ASSERT_UNUSED(expression, expression == m_floatingMediaQueryExp);
-    return m_floatingMediaQueryExp.release();
-}
-
-Vector<OwnPtr<MediaQueryExp> >* CSSParser::createFloatingMediaQueryExpList()
-{
-    m_floatingMediaQueryExpList = adoptPtr(new Vector<OwnPtr<MediaQueryExp> >);
-    return m_floatingMediaQueryExpList.get();
-}
-
-PassOwnPtr<Vector<OwnPtr<MediaQueryExp> > > CSSParser::sinkFloatingMediaQueryExpList(Vector<OwnPtr<MediaQueryExp> >* list)
-{
-    ASSERT_UNUSED(list, list == m_floatingMediaQueryExpList);
-    return m_floatingMediaQueryExpList.release();
-}
-
-MediaQuery* CSSParser::createFloatingMediaQuery(MediaQuery::Restrictor restrictor, const String& mediaType, PassOwnPtr<Vector<OwnPtr<MediaQueryExp> > > expressions)
-{
-    m_floatingMediaQuery = adoptPtr(new MediaQuery(restrictor, mediaType, expressions));
-    return m_floatingMediaQuery.get();
-}
-
-MediaQuery* CSSParser::createFloatingMediaQuery(PassOwnPtr<Vector<OwnPtr<MediaQueryExp> > > expressions)
-{
-    return createFloatingMediaQuery(MediaQuery::None, "all", expressions);
-}
-
-PassOwnPtr<MediaQuery> CSSParser::sinkFloatingMediaQuery(MediaQuery* query)
-{
-    ASSERT_UNUSED(query, query == m_floatingMediaQuery);
-    return m_floatingMediaQuery.release();
-}
-
-Vector<RefPtr<StyleKeyframe> >* CSSParser::createFloatingKeyframeVector()
-{
-    m_floatingKeyframeVector = adoptPtr(new Vector<RefPtr<StyleKeyframe> >());
-    return m_floatingKeyframeVector.get();
-}
-
-PassOwnPtr<Vector<RefPtr<StyleKeyframe> > > CSSParser::sinkFloatingKeyframeVector(Vector<RefPtr<StyleKeyframe> >* keyframeVector)
-{
-    ASSERT_UNUSED(keyframeVector, m_floatingKeyframeVector == keyframeVector);
-    return m_floatingKeyframeVector.release();
-}
-
-MediaQuerySet* CSSParser::createMediaQuerySet()
-{
-    RefPtr<MediaQuerySet> queries = MediaQuerySet::create();
-    MediaQuerySet* result = queries.get();
-    m_parsedMediaQuerySets.append(queries.release());
-    return result;
-}
-
-StyleRuleBase* CSSParser::createImportRule(const CSSParserString& url, MediaQuerySet* media)
+PassRefPtr<StyleRuleBase> CSSParser::createImportRule(const CSSParserString& url, PassRefPtr<MediaQuerySet> media)
 {
     if (!media || !m_allowImportRules) {
@@ -11613 +11465 @@
     }
     RefPtr<StyleRuleImport> rule = StyleRuleImport::create(url, media);
-    StyleRuleImport* result = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
-    return result;
-}
-
-StyleRuleBase* CSSParser::createMediaRule(MediaQuerySet* media, RuleList* rules)
+    return rule.release();
+}
+
+PassRefPtr<StyleRuleBase> CSSParser::createMediaRule(PassRefPtr<MediaQuerySet> media, RuleList* rules)
 {
     m_allowImportRules = m_allowNamespaceDeclarations = false;
@@ -11630 +11480 @@
     } else
         rule = StyleRuleMedia::create(media, rules ? *rules : emptyRules);
-    StyleRuleMedia* result = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
-    return result;
-}
-
-StyleRuleBase* CSSParser::createEmptyMediaRule(RuleList* rules)
-{
-    return createMediaRule(MediaQuerySet::create().get(), rules);
+    return rule.release();
+}
+
+PassRefPtr<StyleRuleBase> CSSParser::createEmptyMediaRule(RuleList* rules)
+{
+    return createMediaRule(MediaQuerySet::create(), rules);
 }
 
 #if ENABLE(CSS3_CONDITIONAL_RULES)
-StyleRuleBase* CSSParser::createSupportsRule(bool conditionIsSupported, RuleList* rules)
+PassRefPtr<StyleRuleBase> CSSParser::createSupportsRule(bool conditionIsSupported, RuleList* rules)
 {
     m_allowImportRules = m_allowNamespaceDeclarations = false;
@@ -11664 +11512 @@
     }
 
-    StyleRuleSupports* result = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
 
-    return result;
+    return rule.release();
 }
 
@@ -11700 +11546 @@
 
 #endif
-
-CSSParser::RuleList* CSSParser::createRuleList()
-{
-    OwnPtr<RuleList> list = adoptPtr(new RuleList);
-    RuleList* listPtr = list.get();
-
-    m_parsedRuleLists.append(list.release());
-    return listPtr;
-}
 
 void CSSParser::processAndAddNewRuleToSourceTreeIfNeeded()
@@ -11774 +11611 @@
 void CSSParser::logError(const String& message, int lineNumber)
 {
-    // FIXME: <http://webkit.org/b/114313> CSS Parser ConsoleMessage errors should include column numbers
+    // FIXME: <http://webkit.org/b/114313> CSS parser console message errors should include column numbers.
     PageConsole& console = m_styleSheet->singleOwnerDocument()->page()->console();
     console.addMessage(CSSMessageSource, WarningMessageLevel, message, m_styleSheet->baseURL().string(), lineNumber + 1, 0);
 }
 
-StyleRuleKeyframes* CSSParser::createKeyframesRule(const String& name, PassOwnPtr<Vector<RefPtr<StyleKeyframe> > > popKeyframes)
-{
-    OwnPtr<Vector<RefPtr<StyleKeyframe> > > keyframes = popKeyframes;
+PassRefPtr<StyleRuleKeyframes> CSSParser::createKeyframesRule(const String& name, PassOwnPtr<Vector<RefPtr<StyleKeyframe>>> popKeyframes)
+{
+    OwnPtr<Vector<RefPtr<StyleKeyframe>>> keyframes = popKeyframes;
     m_allowImportRules = m_allowNamespaceDeclarations = false;
     RefPtr<StyleRuleKeyframes> rule = StyleRuleKeyframes::create();
@@ -11787 +11624 @@
         rule->parserAppendKeyframe(keyframes->at(i));
     rule->setName(name);
-    StyleRuleKeyframes* rulePtr = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
-    return rulePtr;
-}
-
-StyleRuleBase* CSSParser::createStyleRule(Vector<OwnPtr<CSSParserSelector> >* selectors)
-{
-    StyleRule* result = 0;
+    return rule.release();
+}
+
+PassRefPtr<StyleRuleBase> CSSParser::createStyleRule(Vector<OwnPtr<CSSParserSelector>>* selectors)
+{
+    RefPtr<StyleRule> rule;
     if (selectors) {
         m_allowImportRules = false;
@@ -11801 +11636 @@
         if (m_hasFontFaceOnlyValues)
             deleteFontFaceOnlyValues();
-        RefPtr<StyleRule> rule = StyleRule::create(m_lastSelectorLineNumber, createStylePropertySet());
+        rule = StyleRule::create(m_lastSelectorLineNumber, createStylePropertySet());
         rule->parserAdoptSelectorVector(*selectors);
-        result = rule.get();
-        m_parsedRules.append(rule.release());
         processAndAddNewRuleToSourceTreeIfNeeded();
     } else
         popRuleData();
     clearProperties();
-    return result;
-}
-
-StyleRuleBase* CSSParser::createFontFaceRule()
+    return rule.release();
+}
+
+PassRefPtr<StyleRuleBase> CSSParser::createFontFaceRule()
 {
     m_allowImportRules = m_allowNamespaceDeclarations = false;
@@ -11831 +11664 @@
     RefPtr<StyleRuleFontFace> rule = StyleRuleFontFace::create(createStylePropertySet());
     clearProperties();
-    StyleRuleFontFace* result = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
-    return result;
+    return rule.release();
 }
 
 #if ENABLE(SHADOW_DOM)
-StyleRuleBase* CSSParser::createHostRule(RuleList* rules)
+PassRefPtr<StyleRuleBase> CSSParser::createHostRule(RuleList* rules)
 {
     m_allowImportRules = m_allowNamespaceDeclarations = false;
@@ -11848 +11679 @@
         rule = StyleRuleHost::create(emptyRules);
     }
-    StyleRuleHost* result = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
-    return result;
+    return rule.release();
 }
 #endif
@@ -11872 +11701 @@
 }
 
-CSSParserSelector* CSSParser::rewriteSpecifiersWithNamespaceIfNeeded(CSSParserSelector* specifiers)
-{
-    if (m_defaultNamespace != starAtom || specifiers->isCustomPseudoElement())
-        return rewriteSpecifiersWithElementName(nullAtom, starAtom, specifiers, /*tagIsForNamespaceRule*/true);
-    return specifiers;
-}
-
-CSSParserSelector* CSSParser::rewriteSpecifiersWithElementName(const AtomicString& namespacePrefix, const AtomicString& elementName, CSSParserSelector* specifiers, bool tagIsForNamespaceRule)
+void CSSParser::rewriteSpecifiersWithNamespaceIfNeeded(CSSParserSelector& specifiers)
+{
+    if (m_defaultNamespace != starAtom || specifiers.isCustomPseudoElement())
+        rewriteSpecifiersWithElementName(nullAtom, starAtom, specifiers, /*tagIsForNamespaceRule*/true);
+}
+
+void CSSParser::rewriteSpecifiersWithElementName(const AtomicString& namespacePrefix, const AtomicString& elementName, CSSParserSelector& specifiers, bool tagIsForNamespaceRule)
 {
     AtomicString determinedNamespace = namespacePrefix != nullAtom && m_styleSheet ? m_styleSheet->determineNamespace(namespacePrefix) : m_defaultNamespace;
     QualifiedName tag(namespacePrefix, elementName, determinedNamespace);
 
-    if (!specifiers->isCustomPseudoElement()) {
+    if (!specifiers.isCustomPseudoElement()) {
         if (tag == anyQName())
-            return specifiers;
+            return;
 #if ENABLE(VIDEO_TRACK)
-        if (!(specifiers->pseudoType() == CSSSelector::PseudoCue))
+        if (specifiers.pseudoType() != CSSSelector::PseudoCue)
 #endif
-            specifiers->prependTagSelector(tag, tagIsForNamespaceRule);
-        return specifiers;
-    }
-
-    CSSParserSelector* lastShadowDescendant = specifiers;
-    CSSParserSelector* history = specifiers;
+            specifiers.prependTagSelector(tag, tagIsForNamespaceRule);
+        return;
+    }
+
+    CSSParserSelector* lastShadowDescendant = &specifiers;
+    CSSParserSelector* history = &specifiers;
     while (history->tagHistory()) {
         history = history->tagHistory();
@@ -11905 +11733 @@
         if (tag != anyQName())
             lastShadowDescendant->tagHistory()->prependTagSelector(tag, tagIsForNamespaceRule);
-        return specifiers;
+        return;
     }
 
     // For shadow-ID pseudo-elements to be correctly matched, the ShadowDescendant combinator has to be used.
     // We therefore create a new Selector with that combinator here in any case, even if matching any (host) element in any namespace (i.e. '*').
-    OwnPtr<CSSParserSelector> elementNameSelector = adoptPtr(new CSSParserSelector(tag));
-    lastShadowDescendant->setTagHistory(elementNameSelector.release());
+    lastShadowDescendant->setTagHistory(adoptPtr(new CSSParserSelector(tag)));
     lastShadowDescendant->setRelation(CSSSelector::ShadowDescendant);
-    return specifiers;
-}
-
-CSSParserSelector* CSSParser::rewriteSpecifiers(CSSParserSelector* specifiers, CSSParserSelector* newSpecifier)
+}
+
+OwnPtr<CSSParserSelector> CSSParser::rewriteSpecifiers(OwnPtr<CSSParserSelector> specifiers, OwnPtr<CSSParserSelector> newSpecifier)
 {
 #if ENABLE(VIDEO_TRACK)
@@ -11924 +11750 @@
 #endif
         // Unknown pseudo element always goes at the top of selector chain.
-        newSpecifier->appendTagHistory(CSSSelector::ShadowDescendant, sinkFloatingSelector(specifiers));
+        newSpecifier->appendTagHistory(CSSSelector::ShadowDescendant, specifiers.release());
         return newSpecifier;
     }
     if (specifiers->isCustomPseudoElement()) {
         // Specifiers for unknown pseudo element go right behind it in the chain.
-        specifiers->insertTagHistory(CSSSelector::SubSelector, sinkFloatingSelector(newSpecifier), CSSSelector::ShadowDescendant);
+        specifiers->insertTagHistory(CSSSelector::SubSelector, newSpecifier.release(), CSSSelector::ShadowDescendant);
         return specifiers;
     }
-    specifiers->appendTagHistory(CSSSelector::SubSelector, sinkFloatingSelector(newSpecifier));
+    specifiers->appendTagHistory(CSSSelector::SubSelector, newSpecifier.release());
     return specifiers;
 }
 
-StyleRuleBase* CSSParser::createPageRule(PassOwnPtr<CSSParserSelector> pageSelector)
+PassRefPtr<StyleRuleBase> CSSParser::createPageRule(PassOwnPtr<CSSParserSelector> pageSelector)
 {
     // FIXME: Margin at-rules are ignored.
     m_allowImportRules = m_allowNamespaceDeclarations = false;
-    StyleRulePage* pageRule = 0;
+    RefPtr<StyleRulePage> rule;
     if (pageSelector) {
-        RefPtr<StyleRulePage> rule = StyleRulePage::create(createStylePropertySet());
-        Vector<OwnPtr<CSSParserSelector> > selectorVector;
+        rule = StyleRulePage::create(createStylePropertySet());
+        Vector<OwnPtr<CSSParserSelector>> selectorVector;
         selectorVector.append(pageSelector);
         rule->parserAdoptSelectorVector(selectorVector);
-        pageRule = rule.get();
-        m_parsedRules.append(rule.release());
         processAndAddNewRuleToSourceTreeIfNeeded();
     } else
         popRuleData();
     clearProperties();
-    return pageRule;
-}
-
-void CSSParser::setReusableRegionSelectorVector(Vector<OwnPtr<CSSParserSelector> >* selectors)
-{
-    if (selectors)
-        m_reusableRegionSelectorVector.swap(*selectors);
-}
-
-StyleRuleBase* CSSParser::createRegionRule(Vector<OwnPtr<CSSParserSelector> >* regionSelector, RuleList* rules)
+    return rule.release();
+}
+
+OwnPtr<Vector<OwnPtr<CSSParserSelector>>> CSSParser::createSelectorVector()
+{
+    if (m_recycledSelectorVector) {
+        m_recycledSelectorVector->shrink(0);
+        return std::move(m_recycledSelectorVector);
+    }
+    return adoptPtr(new Vector<OwnPtr<CSSParserSelector>>);
+}
+
+void CSSParser::recycleSelectorVector(OwnPtr<Vector<OwnPtr<CSSParserSelector>>> vector)
+{
+    if (vector && !m_recycledSelectorVector)
+        m_recycledSelectorVector = std::move(vector);
+}
+
+PassRefPtr<StyleRuleBase> CSSParser::createRegionRule(Vector<OwnPtr<CSSParserSelector>>* regionSelector, RuleList* rules)
 {
     if (!cssRegionsEnabled() || !regionSelector || !rules) {
@@ -11972 +11805 @@
     RefPtr<StyleRuleRegion> regionRule = StyleRuleRegion::create(regionSelector, *rules);
 
-    StyleRuleRegion* result = regionRule.get();
-    m_parsedRules.append(regionRule.release());
     if (isExtractingSourceData())
         addNewRuleToSourceTree(CSSRuleSourceData::createUnknown());
 
-    return result;
-}
-
-StyleRuleBase* CSSParser::createMarginAtRule(CSSSelector::MarginBoxType /* marginBox */)
+    return regionRule.release();
+}
+
+void CSSParser::createMarginAtRule(CSSSelector::MarginBoxType /* marginBox */)
 {
     // FIXME: Implement margin at-rule here, using:
@@ -11988 +11819 @@
 
     endDeclarationsForMarginBox();
-    return 0; // until this method is implemented.
 }
 
@@ -12015 +11845 @@
 }
 
-StyleKeyframe* CSSParser::createKeyframe(CSSParserValueList* keys)
+PassRefPtr<StyleKeyframe> CSSParser::createKeyframe(CSSParserValueList& keys)
 {
     // Create a key string from the passed keys
     StringBuilder keyString;
-    for (unsigned i = 0; i < keys->size(); ++i) {
+    for (unsigned i = 0; i < keys.size(); ++i) {
         // Just as per the comment below, we ignore keyframes with
         // invalid key values (plain numbers or unknown identifiers)
         // marked as CSSPrimitiveValue::CSS_UNKNOWN during parsing.
-        if (keys->valueAt(i)->unit == CSSPrimitiveValue::CSS_UNKNOWN) {
+        if (keys.valueAt(i)->unit == CSSPrimitiveValue::CSS_UNKNOWN) {
             clearProperties();
             return 0;
         }
 
-        ASSERT(keys->valueAt(i)->unit == CSSPrimitiveValue::CSS_NUMBER);
-        float key = static_cast<float>(keys->valueAt(i)->fValue);
+        ASSERT(keys.valueAt(i)->unit == CSSPrimitiveValue::CSS_NUMBER);
+        float key = static_cast<float>(keys.valueAt(i)->fValue);
         if (key < 0 || key > 100) {
             // As per http://www.w3.org/TR/css3-animations/#keyframes,
@@ -12048 +11878 @@
     clearProperties();
 
-    StyleKeyframe* keyframePtr = keyframe.get();
-    m_parsedKeyframes.append(keyframe.release());
-    return keyframePtr;
+    return keyframe.release();
 }
 
@@ -12258 +12086 @@
 
 #if ENABLE(CSS_DEVICE_ADAPTATION)
-StyleRuleBase* CSSParser::createViewportRule()
+PassRefPtr<StyleRuleBase> CSSParser::createViewportRule()
 {
     m_allowImportRules = m_allowNamespaceDeclarations = false;
@@ -12265 +12093 @@
     clearProperties();
 
-    StyleRuleViewport* result = rule.get();
-    m_parsedRules.append(rule.release());
     processAndAddNewRuleToSourceTreeIfNeeded();
 
-    return result;
+    return rule.release();
 }