Avoid parsing css text if there are identical inline style blocks.

Source/core/dom/StyleElement.cpp

 /*
  * Copyright (C) 2006, 2007 Rob Buis
  * Copyright (C) 2008 Apple, Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 17 matching lines @@
 #include "core/dom/Element.h"
 #include "core/dom/ScriptableDocumentParser.h"
 #include "core/dom/StyleEngine.h"
 #include "core/html/HTMLStyleElement.h"
 #include "core/frame/ContentSecurityPolicy.h"
 #include "wtf/text/StringBuilder.h"
 #include "wtf/text/TextPosition.h"
 
 namespace WebCore {
 
+static const unsigned cacheableTextContentSizeLimit = (unsigned)-1; // UNLIMITED

[esprehn, 2013/10/30 21:33:50]

Not used, remove this constant.

+
 static bool isCSS(Element* element, const AtomicString& type)
 {
     return type.isEmpty() || (element->isHTMLElement() ? equalIgnoringCase(type, "text/css") : (type == "text/css"));
 }
 
 StyleElement::StyleElement(Document* document, bool createdByParser)
     : m_createdByParser(createdByParser)
     , m_loading(false)
     , m_startPosition(TextPosition::belowRangePosition())
 {
@@ skipping 60 matching lines @@
 void StyleElement::process(Element* element)
 {
     if (!element || !element->inDocument())
         return;
     createSheet(element, element->textFromChildren());
 }
 
 void StyleElement::clearSheet()
 {
     ASSERT(m_sheet);
+    // If the sheet was cached one(=!m_sheetText.isEmpty()), the cached sheet has an owner document and
+    // document is not destroyed (not in ~Document), we need to deref the cached item.
+    if (!m_sheetText.isEmpty() && m_sheet->ownerDocument() && m_sheet->ownerDocument()->isActive())
+        m_sheet->ownerDocument()->styleEngine()->styleSheetContentsCache().remove(m_sheetText);

[esprehn, 2013/10/30 21:33:50]

I don't think you should need this.

     m_sheet.release()->clearOwnerNode();
+    m_sheetText = AtomicString();

[esprehn, 2013/10/30 21:33:50]

You shouldn't need to store the string like this, the cache can be handled by
hooks inside StyleSheetContents

 }
 
 void StyleElement::createSheet(Element* e, const String& text)
 {
     ASSERT(e);
     ASSERT(e->inDocument());
     Document& document = e->document();
     if (m_sheet) {
         if (m_sheet->isLoading())
             document.styleEngine()->removePendingSheet(e);
         clearSheet();
     }
 
+    ASSERT(!isHTMLStyleElement(e) || m_sheetText.isEmpty());
+    AtomicString sheetText = isHTMLStyleElement(e) ? AtomicString(text.impl()) : AtomicString();
+
     // 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);
     if (isCSS(e, type) && passesContentSecurityPolicyChecks) {
         RefPtr<MediaQuerySet> mediaQueries = MediaQuerySet::create(media());
 
         MediaQueryEvaluator screenEval("screen", true);
         MediaQueryEvaluator printEval("print", true);
         if (screenEval.eval(mediaQueries.get()) || printEval.eval(mediaQueries.get())) {
             document.styleEngine()->addPendingSheet();
             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()->styleSheetContentsCache().find(sheetText)) {

[esprehn, 2013/10/30 21:33:50]

You should just always go through the cache and encapsulate the logic in there.

m_sheet = document.styleEngine().createSheet(text);

Internally it should do the hash table lookup or parse if needed.

+                // 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->copy(), e, startPosition);

[esprehn, 2013/10/30 21:33:50]

Why are you creating a copy? That's bad.

[tasak, 2013/11/06 10:18:22]

If we don't copy, 1 stylesheetcontents has multiple owner nodes.

So in StyleSheetContents::checkLoaded(), we cannot know what style element needs
"sheetLoaded".

+                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 (!sheetText.isEmpty() && m_sheet->contents()->isCacheable()) {
+            m_sheetText = sheetText;
+            document.styleEngine()->styleSheetContentsCache().add(m_sheetText, m_sheet->contents());
+        }

[esprehn, 2013/10/30 21:33:50]

Remove this, it should all be handled inside StyleEngine:createSheet(text)

[tasak, 2013/11/06 10:18:22]

However, we only knows whether a given stylesheetcontents is cacheable or not
after checkLoaded().

I think, it would be better to invoke the checkLoaded after updating
StyleElement::m_sheet and StyleElement::m_loading.

+    }
 }
 
 bool StyleElement::isLoading() const
 {
     if (m_loading)
         return true;
     return m_sheet ? m_sheet->isLoading() : false;
 }
 
 bool StyleElement::sheetLoaded(Document& document)
 {
     if (isLoading())
         return false;
 
     document.styleEngine()->removePendingSheet(m_sheet->ownerNode());
     return true;
 }
 
 void StyleElement::startLoadingDynamicSheet(Document& document)
 {
     document.styleEngine()->addPendingSheet();
 }
 
 }