Skip to content

GitLab

T
tangled
Commit 62e3c1aa authored by adamk@chromium.org's avatar adamk@chromium.org
Browse files
Options

Move inline stylesheet cache decision making from StyleSheetContents to StyleEngine 

This separates the decisions about caching StyleSheetContents for <style> elements
from those related to <link> elements. This makes the code clearer as there are
fewer conditions that could cause uncacheability (for example, the StyleSheetContents
for <style> elements could never be for an @import rule).

And since the <style> cache is per-StyleEngine (and thus per-Document), the
<style> case can now safely cache stylesheets that include @media rules.
This speeds up the included performance test from ~90ms on my Z620 to ~50ms,
and improves the biggest block of chromestatus.com load performance by ~20%.

This patch also cleans up StyleSheetContents a bit, collapsing the maybeCacheable()
method into isCacheable() and making appropriate accessors const.

BUG=342507

Review URL: https://codereview.chromium.org/196133034

git-svn-id: svn://svn.chromium.org/blink/trunk@169610 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 83b6d615
Hide whitespace changes
Inline Side-by-side
Showing with 67 additions and 17 deletions
third_party/WebKit/PerformanceTests/ShadowDOM/shadow-style-share-media-query.html 0 → 100644
View file @ 62e3c1aa
<!doctype html>
<script src="../resources/runner.js"></script>
<script>
var listSize = 1000;
window.onload = function() {
PerfTestRunner.measureTime({
run: function() {
var list = document.querySelector('#list');
var tmpl = document.querySelector("#tmpl");
list.innerHTML = '';
var start = PerfTestRunner.now();
var i = 0;
do {
var host = document.createElement('div');
var root = host.createShadowRoot();
root.appendChild(tmpl.content.cloneNode(true));
var light = document.createElement('div');
list.appendChild(host);
} while (++i < listSize);
document.body.offsetHeight;
return PerfTestRunner.now() - start;
}
});
}
</script>
<template id="tmpl">
<style>
@media (max-width: 600px) {
div { color: red; }
}
.foo { color: black; }
.bar { color: blue; }
.baz { color: green; }
.bat { color: orange; }
</style>
<div>item</div>
</template>
<section id="list"></section>
third_party/WebKit/Source/core/css/StyleSheetContents.cpp
View file @ 62e3c1aa
...@@ -113,8 +113,11 @@ void StyleSheetContents::setHasSyntacticallyValidCSSHeader(bool isValidCss) ...@@ -113,8 +113,11 @@ void StyleSheetContents::setHasSyntacticallyValidCSSHeader(bool isValidCss)
m_hasSyntacticallyValidCSSHeader = isValidCss; m_hasSyntacticallyValidCSSHeader = isValidCss;
} }
bool StyleSheetContents::maybeCacheable() const bool StyleSheetContents::isCacheable() const
{ {
// This would require dealing with multiple clients for load callbacks.
if (!loadCompleted())
return false;
// FIXME: StyleSheets with media queries can't be cached because their RuleSet // FIXME: StyleSheets with media queries can't be cached because their RuleSet
// is processed differently based off the media queries, which might resolve // is processed differently based off the media queries, which might resolve
// differently depending on the context of the parent CSSStyleSheet (e.g. // differently depending on the context of the parent CSSStyleSheet (e.g.
...@@ -140,14 +143,6 @@ bool StyleSheetContents::maybeCacheable() const ...@@ -140,14 +143,6 @@ bool StyleSheetContents::maybeCacheable() const
return true; return true;
} }
bool StyleSheetContents::isCacheable() const
{
// This would require dealing with multiple clients for load callbacks.
if (!loadCompleted())
return false;
return maybeCacheable();
}
void StyleSheetContents::parserAppendRule(PassRefPtrWillBeRawPtr<StyleRuleBase> rule) void StyleSheetContents::parserAppendRule(PassRefPtrWillBeRawPtr<StyleRuleBase> rule)
{ {
ASSERT(!rule->isCharsetRule()); ASSERT(!rule->isCharsetRule());
...@@ -589,8 +584,6 @@ void StyleSheetContents::clientLoadStarted(CSSStyleSheet* sheet) ...@@ -589,8 +584,6 @@ void StyleSheetContents::clientLoadStarted(CSSStyleSheet* sheet)
void StyleSheetContents::removeSheetFromCache(Document* document) void StyleSheetContents::removeSheetFromCache(Document* document)
{ {
ASSERT(document); ASSERT(document);
if (!maybeCacheable())
return;
document->styleEngine()->removeSheet(this); document->styleEngine()->removeSheet(this);
} }
......
third_party/WebKit/Source/core/css/StyleSheetContents.h
View file @ 62e3c1aa
...@@ -71,7 +71,6 @@ public: ...@@ -71,7 +71,6 @@ public:
bool parseStringAtPosition(const String&, const TextPosition&, bool); bool parseStringAtPosition(const String&, const TextPosition&, bool);
bool isCacheable() const; bool isCacheable() const;
bool maybeCacheable() const;
bool isLoading() const; bool isLoading() const;
...@@ -154,7 +153,9 @@ public: ...@@ -154,7 +153,9 @@ public:
void removedFromMemoryCache(); void removedFromMemoryCache();
void setHasMediaQueries(); void setHasMediaQueries();
bool hasMediaQueries() { return m_hasMediaQueries; } bool hasMediaQueries() const { return m_hasMediaQueries; }
bool didLoadErrorOccur() const { return m_didLoadErrorOccur; }
void shrinkToFit(); void shrinkToFit();
RuleSet& ruleSet() { ASSERT(m_ruleSet); return *m_ruleSet.get(); } RuleSet& ruleSet() { ASSERT(m_ruleSet); return *m_ruleSet.get(); }
......
third_party/WebKit/Source/core/dom/StyleEngine.cpp
View file @ 62e3c1aa
...@@ -562,6 +562,21 @@ void StyleEngine::markDocumentDirty() ...@@ -562,6 +562,21 @@ void StyleEngine::markDocumentDirty()
m_document.import()->master()->styleEngine()->markDocumentDirty(); m_document.import()->master()->styleEngine()->markDocumentDirty();
} }
static bool isCacheableForStyleElement(const StyleSheetContents& contents)
{
// FIXME: Support copying import rules.
if (!contents.importRules().isEmpty())
return false;
// Until import rules are supported in cached sheets it's not possible for loading to fail.
ASSERT(!contents.didLoadErrorOccur());
// It is not the original sheet anymore.
if (contents.isMutable())
return false;
if (!contents.hasSyntacticallyValidCSSHeader())
return false;
return true;
}
PassRefPtrWillBeRawPtr<CSSStyleSheet> StyleEngine::createSheet(Element* e, const String& text, TextPosition startPosition, bool createdByParser) PassRefPtrWillBeRawPtr<CSSStyleSheet> StyleEngine::createSheet(Element* e, const String& text, TextPosition startPosition, bool createdByParser)
{ {
RefPtrWillBeRawPtr<CSSStyleSheet> styleSheet; RefPtrWillBeRawPtr<CSSStyleSheet> styleSheet;
...@@ -574,14 +589,16 @@ PassRefPtrWillBeRawPtr<CSSStyleSheet> StyleEngine::createSheet(Element* e, const ...@@ -574,14 +589,16 @@ PassRefPtrWillBeRawPtr<CSSStyleSheet> StyleEngine::createSheet(Element* e, const
HashMap<AtomicString, StyleSheetContents*>::AddResult result = m_textToSheetCache.add(textContent, 0); HashMap<AtomicString, StyleSheetContents*>::AddResult result = m_textToSheetCache.add(textContent, 0);
if (result.isNewEntry || !result.storedValue->value) { if (result.isNewEntry || !result.storedValue->value) {
styleSheet = StyleEngine::parseSheet(e, text, startPosition, createdByParser); styleSheet = StyleEngine::parseSheet(e, text, startPosition, createdByParser);
if (result.isNewEntry && styleSheet->contents()->maybeCacheable()) { if (result.isNewEntry && isCacheableForStyleElement(*styleSheet->contents())) {
result.storedValue->value = styleSheet->contents(); result.storedValue->value = styleSheet->contents();
m_sheetToTextCache.add(styleSheet->contents(), textContent); m_sheetToTextCache.add(styleSheet->contents(), textContent);
} }
} else { } else {
ASSERT(result.storedValue->value->maybeCacheable()); StyleSheetContents* contents = result.storedValue->value;
ASSERT(result.storedValue->value->singleOwnerDocument() == e->document()); ASSERT(contents);
styleSheet = CSSStyleSheet::createInline(result.storedValue->value, e, startPosition); ASSERT(isCacheableForStyleElement(*contents));
ASSERT(contents->singleOwnerDocument() == e->document());
styleSheet = CSSStyleSheet::createInline(contents, e, startPosition);
} }
} else { } else {
// FIXME: currently we don't cache StyleSheetContents inQuirksMode. // FIXME: currently we don't cache StyleSheetContents inQuirksMode.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment