- Revision
- 112382
- Author
- [email protected]
- Date
- 2012-03-28 03:41:18 -0700 (Wed, 28 Mar 2012)
Log Message
Separate @import rules from other rules in CSSStyleSheet
https://bugs.webkit.org/show_bug.cgi?id=82384
Reviewed by Andreas Kling.
Import rules always come before all other rules (except @charset). They currently live
in the generic child rule vector. They can be moved to a vector of their own for stronger
typing and more focused traversal. This will also make future refactoring easier.
- @import rules go to m_importRules
- the rest go to m_childRules
* css/CSSStyleSelector.cpp:
(WebCore::CSSStyleSelector::collectMatchingRulesForList):
* css/CSSStyleSheet.cpp:
(WebCore::CSSStyleSheet::parserAppendRule):
(WebCore::CSSStyleSheet::length):
(WebCore::CSSStyleSheet::item):
(WebCore::CSSStyleSheet::clearRules):
(WebCore::CSSStyleSheet::rules):
(WebCore::CSSStyleSheet::insertRule):
(WebCore::CSSStyleSheet::deleteRule):
(WebCore::CSSStyleSheet::isLoading):
(WebCore::CSSStyleSheet::addSubresourceStyleURLs):
* css/CSSStyleSheet.h:
(WebCore):
(CSSStyleSheet):
(WebCore::CSSStyleSheet::childRules):
(WebCore::CSSStyleSheet::importRules):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (112381 => 112382)
--- trunk/Source/WebCore/ChangeLog 2012-03-28 10:10:14 UTC (rev 112381)
+++ trunk/Source/WebCore/ChangeLog 2012-03-28 10:41:18 UTC (rev 112382)
@@ -1,3 +1,35 @@
+2012-03-27 Antti Koivisto <[email protected]>
+
+ Separate @import rules from other rules in CSSStyleSheet
+ https://bugs.webkit.org/show_bug.cgi?id=82384
+
+ Reviewed by Andreas Kling.
+
+ Import rules always come before all other rules (except @charset). They currently live
+ in the generic child rule vector. They can be moved to a vector of their own for stronger
+ typing and more focused traversal. This will also make future refactoring easier.
+
+ - @import rules go to m_importRules
+ - the rest go to m_childRules
+
+ * css/CSSStyleSelector.cpp:
+ (WebCore::CSSStyleSelector::collectMatchingRulesForList):
+ * css/CSSStyleSheet.cpp:
+ (WebCore::CSSStyleSheet::parserAppendRule):
+ (WebCore::CSSStyleSheet::length):
+ (WebCore::CSSStyleSheet::item):
+ (WebCore::CSSStyleSheet::clearRules):
+ (WebCore::CSSStyleSheet::rules):
+ (WebCore::CSSStyleSheet::insertRule):
+ (WebCore::CSSStyleSheet::deleteRule):
+ (WebCore::CSSStyleSheet::isLoading):
+ (WebCore::CSSStyleSheet::addSubresourceStyleURLs):
+ * css/CSSStyleSheet.h:
+ (WebCore):
+ (CSSStyleSheet):
+ (WebCore::CSSStyleSheet::childRules):
+ (WebCore::CSSStyleSheet::importRules):
+
2012-03-28 Pavel Feldman <[email protected]>
Web Inspector: REGRESSION: Stack overflow on the page with > 100kloc
Modified: trunk/Source/WebCore/css/CSSStyleSelector.cpp (112381 => 112382)
--- trunk/Source/WebCore/css/CSSStyleSelector.cpp 2012-03-28 10:10:14 UTC (rev 112381)
+++ trunk/Source/WebCore/css/CSSStyleSelector.cpp 2012-03-28 10:41:18 UTC (rev 112382)
@@ -2225,7 +2225,15 @@
{
ASSERT(!stylesheet->isLoading());
- const Vector<RefPtr<CSSRule> >& rules = stylesheet->ruleVector();
+ const Vector<RefPtr<CSSImportRule> >& importRules = stylesheet->importRules();
+ for (unsigned i = 0; i < importRules.size(); ++i) {
+ if (!importRules[i]->styleSheet())
+ continue;
+ if (!determineStylesheetSelectorScopes(importRules[i]->styleSheet(), idScopes, classScopes))
+ return false;
+ }
+
+ const Vector<RefPtr<CSSRule> >& rules = stylesheet->childRules();
for (unsigned i = 0; i < rules.size(); i++) {
CSSRule* rule = rules[i].get();
if (rule->isStyleRule()) {
@@ -2234,14 +2242,6 @@
return false;
continue;
}
- if (rule->isImportRule()) {
- CSSImportRule* importRule = static_cast<CSSImportRule*>(rule);
- if (importRule->styleSheet()) {
- if (!determineStylesheetSelectorScopes(importRule->styleSheet(), idScopes, classScopes))
- return false;
- }
- continue;
- }
// FIXME: Media rules and maybe some others could be allowed.
return false;
}
@@ -2453,19 +2453,23 @@
// contain our current medium
if (sheet->mediaQueries() && !medium.eval(sheet->mediaQueries(), styleSelector))
return; // the style sheet doesn't apply
+
+ const Vector<RefPtr<CSSImportRule> >& importRules = sheet->importRules();
+ for (unsigned i = 0; i < importRules.size(); ++i) {
+ CSSImportRule* importRule = importRules[i].get();
+ if (importRule->styleSheet() && (!importRule->mediaQueries() || medium.eval(importRule->mediaQueries(), styleSelector)))
+ addRulesFromSheet(importRule->styleSheet(), medium, styleSelector, scope);
+ }
- const Vector<RefPtr<CSSRule> >& rules = sheet->ruleVector();
+ const Vector<RefPtr<CSSRule> >& rules = sheet->childRules();
for (unsigned i = 0; i < rules.size(); ++i) {
CSSRule* rule = rules[i].get();
+
+ ASSERT(!rule->isImportRule());
if (rule->isStyleRule())
addStyleRule(static_cast<CSSStyleRule*>(rule)->styleRule(), !scope);
else if (rule->isPageRule())
addPageRule(static_cast<CSSPageRule*>(rule));
- else if (rule->isImportRule()) {
- CSSImportRule* import = static_cast<CSSImportRule*>(rule);
- if (import->styleSheet() && (!import->mediaQueries() || medium.eval(import->mediaQueries(), styleSelector)))
- addRulesFromSheet(import->styleSheet(), medium, styleSelector, scope);
- }
else if (rule->isMediaRule()) {
CSSMediaRule* mediaRule = static_cast<CSSMediaRule*>(rule);
Modified: trunk/Source/WebCore/css/CSSStyleSheet.cpp (112381 => 112382)
--- trunk/Source/WebCore/css/CSSStyleSheet.cpp 2012-03-28 10:10:14 UTC (rev 112381)
+++ trunk/Source/WebCore/css/CSSStyleSheet.cpp 2012-03-28 10:41:18 UTC (rev 112382)
@@ -102,13 +102,17 @@
clearRules();
}
-void CSSStyleSheet::parserAppendRule(PassRefPtr<CSSRule> child)
+void CSSStyleSheet::parserAppendRule(PassRefPtr<CSSRule> rule)
{
- ASSERT(!child->isCharsetRule());
- CSSRule* c = child.get();
- m_children.append(child);
- if (c->isImportRule())
- static_cast<CSSImportRule*>(c)->requestStyleSheet();
+ ASSERT(!rule->isCharsetRule());
+ if (rule->isImportRule()) {
+ // Parser enforces that @import rules come before anything else except @charset.
+ ASSERT(m_childRules.isEmpty());
+ m_importRules.append(static_cast<CSSImportRule*>(rule.get()));
+ m_importRules.last()->requestStyleSheet();
+ return;
+ }
+ m_childRules.append(rule);
}
CSSCharsetRule* CSSStyleSheet::ensureCharsetRule()
@@ -123,7 +127,8 @@
{
unsigned result = 0;
result += hasCharsetRule() ? 1 : 0;
- result += m_children.size();
+ result += m_importRules.size();
+ result += m_childRules.size();
return result;
}
@@ -135,7 +140,12 @@
return ensureCharsetRule();
--childVectorIndex;
}
- return childVectorIndex < m_children.size() ? m_children[childVectorIndex].get() : 0;
+ if (childVectorIndex < m_importRules.size())
+ return m_importRules[childVectorIndex].get();
+
+ childVectorIndex -= m_importRules.size();
+
+ return childVectorIndex < m_childRules.size() ? m_childRules[childVectorIndex].get() : 0;
}
void CSSStyleSheet::clearCharsetRule()
@@ -152,11 +162,18 @@
// For style rules outside the document, .parentStyleSheet can become null even if the style rule
// is still observable from _javascript_. This matches the behavior of .parentNode for nodes, but
// it's not ideal because it makes the CSSOM's behavior depend on the timing of garbage collection.
- for (unsigned i = 0; i < m_children.size(); ++i) {
- ASSERT(m_children.at(i)->parentStyleSheet() == this);
- m_children.at(i)->setParentStyleSheet(0);
+ for (unsigned i = 0; i < m_childRules.size(); ++i) {
+ ASSERT(m_childRules[i]->parentStyleSheet() == this);
+ m_childRules[i]->setParentStyleSheet(0);
}
- m_children.clear();
+ m_childRules.clear();
+
+ for (unsigned i = 0; i < m_importRules.size(); ++i) {
+ ASSERT(m_importRules.at(i)->parentStyleSheet() == this);
+ m_importRules[i]->setParentStyleSheet(0);
+ }
+ m_importRules.clear();
+
clearCharsetRule();
}
@@ -175,11 +192,12 @@
return 0;
// IE behavior.
RefPtr<StaticCSSRuleList> nonCharsetRules = StaticCSSRuleList::create();
- nonCharsetRules->rules().append(m_children);
+ nonCharsetRules->rules().append(m_importRules.data(), m_importRules.size());
+ nonCharsetRules->rules().append(m_childRules.data(), m_childRules.size());
return nonCharsetRules.release();
}
-unsigned CSSStyleSheet::insertRule(const String& rule, unsigned index, ExceptionCode& ec)
+unsigned CSSStyleSheet::insertRule(const String& ruleString, unsigned index, ExceptionCode& ec)
{
ec = 0;
if (index > length()) {
@@ -187,17 +205,17 @@
return 0;
}
CSSParser p(useStrictParsing());
- RefPtr<CSSRule> r = p.parseRule(this, rule);
+ RefPtr<CSSRule> rule = p.parseRule(this, ruleString);
- if (!r) {
+ if (!rule) {
ec = SYNTAX_ERR;
return 0;
}
// Parser::parseRule doesn't currently allow @charset so we don't need to deal with it.
- ASSERT(!r->isCharsetRule());
+ ASSERT(!rule->isCharsetRule());
unsigned childVectorIndex = index;
- // m_children does not contain @charset which is always in index 0 if it exists.
+ // m_childRules does not contain @charset which is always in index 0 if it exists.
if (hasCharsetRule()) {
if (index == 0) {
// Nothing can be inserted before @charset.
@@ -206,26 +224,30 @@
}
--childVectorIndex;
}
+
+ if (childVectorIndex < m_importRules.size() || (childVectorIndex == m_importRules.size() && rule->isImportRule())) {
+ // Inserting non-import rule before @import is not allowed.
+ if (!rule->isImportRule()) {
+ ec = HIERARCHY_REQUEST_ERR;
+ return 0;
+ }
+ m_importRules.insert(childVectorIndex, static_cast<CSSImportRule*>(rule.get()));
+ m_importRules[childVectorIndex]->requestStyleSheet();
- // Throw a HIERARCHY_REQUEST_ERR exception if the rule cannot be inserted at the specified index. The best
- // example of this is an @import rule inserted after regular rules.
- if (r->isImportRule()) {
- // Check all the rules that come before this one to make sure they are only @import rules.
- for (unsigned i = 0; i < childVectorIndex; ++i) {
- if (!m_children.at(i)->isImportRule()) {
- ec = HIERARCHY_REQUEST_ERR;
- return 0;
- }
- }
- }
+ // FIXME: Stylesheet doesn't actually change meaningfully before the imported sheets are loaded.
+ styleSheetChanged();
+ return index;
+ }
+ // Inserting @import rule after a non-import rule is not allowed.
+ if (rule->isImportRule()) {
+ ec = HIERARCHY_REQUEST_ERR;
+ return 0;
+ }
+ childVectorIndex -= m_importRules.size();
- CSSRule* c = r.get();
- m_children.insert(childVectorIndex, r.release());
- if (c->isImportRule())
- static_cast<CSSImportRule*>(c)->requestStyleSheet();
+ m_childRules.insert(childVectorIndex, rule.release());
styleSheetChanged();
-
return index;
}
@@ -270,9 +292,16 @@
}
--childVectorIndex;
}
+ if (childVectorIndex < m_importRules.size()) {
+ m_importRules[childVectorIndex]->setParentStyleSheet(0);
+ m_importRules.remove(childVectorIndex);
+ styleSheetChanged();
+ return;
+ }
+ childVectorIndex -= m_importRules.size();
- m_children.at(childVectorIndex)->setParentStyleSheet(0);
- m_children.remove(childVectorIndex);
+ m_childRules[childVectorIndex]->setParentStyleSheet(0);
+ m_childRules.remove(childVectorIndex);
styleSheetChanged();
}
@@ -317,9 +346,8 @@
bool CSSStyleSheet::isLoading()
{
- for (unsigned i = 0; i < m_children.size(); ++i) {
- CSSRule* rule = m_children.at(i).get();
- if (rule->isImportRule() && static_cast<CSSImportRule*>(rule)->isLoading())
+ for (unsigned i = 0; i < m_importRules.size(); ++i) {
+ if (m_importRules[i]->isLoading())
return true;
}
return false;
@@ -421,14 +449,17 @@
while (!styleSheetQueue.isEmpty()) {
CSSStyleSheet* styleSheet = styleSheetQueue.takeFirst();
+
+ for (unsigned i = 0; i < styleSheet->m_importRules.size(); ++i) {
+ CSSImportRule* importRule = styleSheet->m_importRules[i].get();
+ if (importRule->styleSheet())
+ styleSheetQueue.append(importRule->styleSheet());
+ importRule->addSubresourceStyleURLs(urls);
+ }
- for (unsigned i = 0; i < styleSheet->m_children.size(); ++i) {
- CSSRule* rule = styleSheet->m_children.at(i).get();
- if (rule->isImportRule()) {
- if (CSSStyleSheet* ruleStyleSheet = static_cast<CSSImportRule*>(rule)->styleSheet())
- styleSheetQueue.append(ruleStyleSheet);
- static_cast<CSSImportRule*>(rule)->addSubresourceStyleURLs(urls);
- } else if (rule->isFontFaceRule())
+ for (unsigned i = 0; i < styleSheet->m_childRules.size(); ++i) {
+ CSSRule* rule = styleSheet->m_childRules[i].get();
+ if (rule->isFontFaceRule())
static_cast<CSSFontFaceRule*>(rule)->addSubresourceStyleURLs(urls);
else if (rule->isStyleRule() || rule->isPageRule())
static_cast<CSSStyleRule*>(rule)->styleRule()->addSubresourceStyleURLs(urls, this);
Modified: trunk/Source/WebCore/css/CSSStyleSheet.h (112381 => 112382)
--- trunk/Source/WebCore/css/CSSStyleSheet.h 2012-03-28 10:10:14 UTC (rev 112381)
+++ trunk/Source/WebCore/css/CSSStyleSheet.h 2012-03-28 10:41:18 UTC (rev 112382)
@@ -28,6 +28,7 @@
struct CSSNamespace;
class CSSParser;
class CSSCharsetRule;
+class CSSImportRule;
class CSSRule;
class CSSRuleList;
class CachedCSSStyleSheet;
@@ -119,8 +120,9 @@
unsigned length() const;
CSSRule* item(unsigned index);
- // Does not contain @charset rule.
- const Vector<RefPtr<CSSRule> >& ruleVector() const { return m_children; }
+ // Rules other than @charset and @import.
+ const Vector<RefPtr<CSSRule> >& childRules() const { return m_childRules; }
+ const Vector<RefPtr<CSSImportRule> >& importRules() const { return m_importRules; }
virtual MediaList* media() const OVERRIDE;
@@ -142,7 +144,8 @@
String m_encodingFromCharsetRule;
RefPtr<CSSCharsetRule> m_charsetRuleCSSOMWrapper;
- Vector<RefPtr<CSSRule> > m_children;
+ Vector<RefPtr<CSSImportRule> > m_importRules;
+ Vector<RefPtr<CSSRule> > m_childRules;
OwnPtr<CSSNamespace> m_namespaces;
String m_charset;
RefPtr<MediaQuerySet> m_mediaQueries;
