Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(682)

Unified Diff: Source/core/dom/StyleElement.cpp

Issue 28553005: Avoid parsing css text if there are identical inline style blocks. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/dom/StyleElement.cpp
diff --git a/Source/core/dom/StyleElement.cpp b/Source/core/dom/StyleElement.cpp
index d50689a3bba2ba8cf52a76629959044db6fbaebf..4afe8edfb3454d6ece9be095c4c4c4a3e4530a53 100644
--- a/Source/core/dom/StyleElement.cpp
+++ b/Source/core/dom/StyleElement.cpp
@@ -35,6 +35,8 @@
namespace WebCore {
+static const unsigned cacheableTextContentSizeLimit = 256;
dglazkov 2013/10/18 16:56:19 How did you arrive at this number?
tasak 2013/10/21 12:27:37 I have no strong reason. So I removed this magic n
+
static bool isCSS(Element* element, const AtomicString& type)
{
return type.isEmpty() || (element->isHTMLElement() ? equalIgnoringCase(type, "text/css") : (type == "text/css"));
@@ -115,6 +117,8 @@ void StyleElement::process(Element* element)
void StyleElement::clearSheet()
{
ASSERT(m_sheet);
+ if (!m_sheetText.isEmpty() && m_sheet->ownerDocument() && m_sheet->ownerDocument()->isActive())
dglazkov 2013/10/18 16:56:19 It might be a clearer (though more complex) factor
tasak 2013/10/21 12:27:37 I added a comments which describes the condition.
+ m_sheet->ownerDocument()->styleEngine()->unregisterStyleSheetContents(m_sheetText);
m_sheet.release()->clearOwnerNode();
}
@@ -129,6 +133,8 @@ void StyleElement::createSheet(Element* e, const String& text)
clearSheet();
}
+ m_sheetText = isHTMLStyleElement(e) && text.length() < cacheableTextContentSizeLimit ? text : String();
+
// If type is empty or CSS, this is a CSS style sheet.
const AtomicString& type = this->type();
bool passesContentSecurityPolicyChecks = document.contentSecurityPolicy()->allowStyleNonce(e->fastGetAttribute(HTMLNames::nonceAttr)) || document.contentSecurityPolicy()->allowInlineStyle(e->document().url(), m_startPosition.m_line);
@@ -142,17 +148,28 @@ void StyleElement::createSheet(Element* e, const String& text)
m_loading = true;
TextPosition startPosition = m_startPosition == TextPosition::belowRangePosition() ? TextPosition::minimumPosition() : m_startPosition;
- m_sheet = CSSStyleSheet::createInline(e, KURL(), startPosition, document.inputEncoding());
- m_sheet->setMediaQueries(mediaQueries.release());
- m_sheet->setTitle(e->title());
- m_sheet->contents()->parseStringAtPosition(text, startPosition, m_createdByParser);
-
+ if (StyleSheetContents* contents = document.styleEngine()->findStyleSheetContents(m_sheetText)) {
+ // FIXME: should we use registerClient to avoid allocating a new StyleSheetContents?
+ // In this case, StyleSheetContents::singleOwnerNode() doesn't work. So need to modify
+ // codes depending on singleOwnerNode().
+ m_sheet = CSSStyleSheet::createInline(contents, e, startPosition);
+ m_sheet->setMediaQueries(mediaQueries.release());
+ m_sheet->setTitle(e->title());
+ } else {
+ m_sheet = CSSStyleSheet::createInline(e, KURL(), startPosition, document.inputEncoding());
+ m_sheet->setMediaQueries(mediaQueries.release());
+ m_sheet->setTitle(e->title());
+ m_sheet->contents()->parseStringAtPosition(text, startPosition, m_createdByParser);
+ }
m_loading = false;
}
}
- if (m_sheet)
+ if (m_sheet) {
m_sheet->contents()->checkLoaded();
+ if (!m_sheetText.isEmpty() && m_sheet->contents()->isCacheable())
+ document.styleEngine()->registerStyleSheetContents(m_sheetText, m_sheet->contents());
+ }
}
bool StyleElement::isLoading() const

Powered by Google App Engine
This is Rietveld 408576698