Don't use StyleSheetList internally.
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2012 04:25:35 +0000 (04:25 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2012 04:25:35 +0000 (04:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97504

Reviewed by Ryosuke Niwa.

StyleSheetList is a DOM type and should not be used internally. Use plain Vector instead and construct StyleSheetList on DOM access only.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::StyleResolver):
(WebCore::StyleResolver::addStylesheetsFromSeamlessParents):
(WebCore::StyleResolver::collectMatchingRulesForList):
* css/StyleSheetList.cpp:
(WebCore::StyleSheetList::StyleSheetList):
(WebCore::StyleSheetList::styleSheets):
(WebCore):
(WebCore::StyleSheetList::detachFromDocument):

    Use live stylesheet vector of the documents stylesheet collection as long as we are attached to a document.
    When detached copy the stylesheet vector to a member field and use that instead.

(WebCore::StyleSheetList::length):
(WebCore::StyleSheetList::item):
(WebCore::StyleSheetList::getNamedItem):
* css/StyleSheetList.h:

    Removed StyleSheetVector typedef as Vector<RefPtr<StyleSheet> > is less opaque and not much longer.

(WebCore):
(WebCore::StyleSheetList::create):
(StyleSheetList):
(WebCore::StyleSheetList::document):
* dom/Document.cpp:
(WebCore::Document::~Document):
(WebCore::Document::setCompatibilityMode):
(WebCore::Document::styleSheets):
* dom/Document.h:
(Document):
* dom/DocumentStyleSheetCollection.cpp:
(WebCore::DocumentStyleSheetCollection::DocumentStyleSheetCollection):
(WebCore::DocumentStyleSheetCollection::~DocumentStyleSheetCollection):
(WebCore::DocumentStyleSheetCollection::analyzeStyleSheetChange):
(WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets):
* dom/DocumentStyleSheetCollection.h:
(WebCore::DocumentStyleSheetCollec

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@129452 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleSheetList.cpp
Source/WebCore/css/StyleSheetList.h
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/DocumentStyleSheetCollection.cpp
Source/WebCore/dom/DocumentStyleSheetCollection.h

index 0124ebd..2245147 100644 (file)
@@ -1,3 +1,50 @@
+2012-09-24  Antti Koivisto  <antti@apple.com>
+
+        Don't use StyleSheetList internally.
+        https://bugs.webkit.org/show_bug.cgi?id=97504
+
+        Reviewed by Ryosuke Niwa.
+
+        StyleSheetList is a DOM type and should not be used internally. Use plain Vector instead and construct StyleSheetList on DOM access only.
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::StyleResolver):
+        (WebCore::StyleResolver::addStylesheetsFromSeamlessParents):
+        (WebCore::StyleResolver::collectMatchingRulesForList):
+        * css/StyleSheetList.cpp:
+        (WebCore::StyleSheetList::StyleSheetList):
+        (WebCore::StyleSheetList::styleSheets):
+        (WebCore):
+        (WebCore::StyleSheetList::detachFromDocument):
+        
+            Use live stylesheet vector of the documents stylesheet collection as long as we are attached to a document. 
+            When detached copy the stylesheet vector to a member field and use that instead.
+
+        (WebCore::StyleSheetList::length):
+        (WebCore::StyleSheetList::item):
+        (WebCore::StyleSheetList::getNamedItem):
+        * css/StyleSheetList.h:
+        
+            Removed StyleSheetVector typedef as Vector<RefPtr<StyleSheet> > is less opaque and not much longer.
+    
+        (WebCore):
+        (WebCore::StyleSheetList::create):
+        (StyleSheetList):
+        (WebCore::StyleSheetList::document):
+        * dom/Document.cpp:
+        (WebCore::Document::~Document):
+        (WebCore::Document::setCompatibilityMode):
+        (WebCore::Document::styleSheets):
+        * dom/Document.h:
+        (Document):
+        * dom/DocumentStyleSheetCollection.cpp:
+        (WebCore::DocumentStyleSheetCollection::DocumentStyleSheetCollection):
+        (WebCore::DocumentStyleSheetCollection::~DocumentStyleSheetCollection):
+        (WebCore::DocumentStyleSheetCollection::analyzeStyleSheetChange):
+        (WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets):
+        * dom/DocumentStyleSheetCollection.h:
+        (WebCore::DocumentStyleSheetCollec
+
 2012-09-24  Laszlo Gombos  <l.gombos@samsung.com>
 
         [GTK][EFL] Remove cairo prefix from include statements 
index 11e549d..c164606 100644 (file)
@@ -449,7 +449,7 @@ StyleResolver::StyleResolver(Document* document, bool matchAuthorAndUserStyles)
 #endif
 
     addStylesheetsFromSeamlessParents();
-    appendAuthorStylesheets(0, styleSheetCollection->authorStyleSheets()->vector());
+    appendAuthorStylesheets(0, styleSheetCollection->authorStyleSheets());
 }
 
 void StyleResolver::addStylesheetsFromSeamlessParents()
@@ -457,14 +457,14 @@ void StyleResolver::addStylesheetsFromSeamlessParents()
     // Build a list of stylesheet lists from our ancestors, and walk that
     // list in reverse order so that the root-most sheets are appended first.
     Document* childDocument = document();
-    Vector<StyleSheetList*> ancestorSheets;
+    Vector<const Vector<RefPtr<StyleSheet> >* > ancestorSheets;
     while (HTMLIFrameElement* parentIFrame = childDocument->seamlessParentIFrame()) {
         Document* parentDocument = parentIFrame->document();
-        ancestorSheets.append(parentDocument->styleSheets());
+        ancestorSheets.append(&parentDocument->styleSheetCollection()->authorStyleSheets());
         childDocument = parentDocument;
     }
     for (int i = ancestorSheets.size() - 1; i >= 0; i--)
-        appendAuthorStylesheets(0, ancestorSheets.at(i)->vector());
+        appendAuthorStylesheets(0, *ancestorSheets[i]);
 }
 
 void StyleResolver::addAuthorRulesAndCollectUserRulesFromSheets(const Vector<RefPtr<CSSStyleSheet> >* userSheets, RuleSet& userStyle)
@@ -3178,7 +3178,7 @@ static void collectCSSOMWrappers(HashMap<StyleRule*, RefPtr<CSSStyleRule> >& wra
 
 static void collectCSSOMWrappers(HashMap<StyleRule*, RefPtr<CSSStyleRule> >& wrapperMap, DocumentStyleSheetCollection* styleSheetCollection)
 {
-    const Vector<RefPtr<StyleSheet> >& styleSheets = styleSheetCollection->authorStyleSheets()->vector();
+    const Vector<RefPtr<StyleSheet> >& styleSheets = styleSheetCollection->authorStyleSheets();
     for (unsigned i = 0; i < styleSheets.size(); ++i) {
         StyleSheet* styleSheet = styleSheets[i].get();
         if (!styleSheet->isCSSStyleSheet())
index f1ab52d..0f8fc25 100644 (file)
@@ -23,6 +23,7 @@
 
 #include "CSSStyleSheet.h"
 #include "Document.h"
+#include "DocumentStyleSheetCollection.h"
 #include "HTMLNames.h"
 #include "HTMLStyleElement.h"
 #include <wtf/text/WTFString.h>
@@ -31,8 +32,8 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-StyleSheetList::StyleSheetList(Document* doc)
-    : m_doc(doc)
+StyleSheetList::StyleSheetList(Document* document)
+    : m_document(document)
 {
 }
 
@@ -40,24 +41,33 @@ StyleSheetList::~StyleSheetList()
 {
 }
 
-void StyleSheetList::documentDestroyed()
+inline const Vector<RefPtr<StyleSheet> >& StyleSheetList::styleSheets() const
 {
-    m_doc = 0;
+    if (!m_document)
+        return m_detachedStyleSheets;
+    return m_document->styleSheetCollection()->authorStyleSheets();
+}
+
+void StyleSheetList::detachFromDocument()
+{
+    m_detachedStyleSheets = m_document->styleSheetCollection()->authorStyleSheets();
+    m_document = 0;
 }
 
 unsigned StyleSheetList::length() const
 {
-    return m_sheets.size();
+    return styleSheets().size();
 }
 
 StyleSheet* StyleSheetList::item(unsigned index)
 {
-    return index < length() ? m_sheets[index].get() : 0;
+    const Vector<RefPtr<StyleSheet> >& sheets = styleSheets();
+    return index < sheets.size() ? sheets[index].get() : 0;
 }
 
 HTMLStyleElement* StyleSheetList::getNamedItem(const String& name) const
 {
-    if (!m_doc)
+    if (!m_document)
         return 0;
 
     // IE also supports retrieving a stylesheet by name, using the name/id of the <style> tag
@@ -65,8 +75,7 @@ HTMLStyleElement* StyleSheetList::getNamedItem(const String& name) const
     // ### Bad implementation because returns a single element (are IDs always unique?)
     // and doesn't look for name attribute.
     // But unicity of stylesheet ids is good practice anyway ;)
-
-    Element* element = m_doc->getElementById(name);
+    Element* element = m_document->getElementById(name);
     if (element && element->hasTagName(styleTag))
         return static_cast<HTMLStyleElement*>(element);
     return 0;
index 3f81163..c052841 100644 (file)
@@ -32,40 +32,26 @@ class Document;
 class HTMLStyleElement;
 class StyleSheet;
 
-typedef Vector<RefPtr<StyleSheet> > StyleSheetVector;
-
 class StyleSheetList : public RefCounted<StyleSheetList> {
 public:
-    static PassRefPtr<StyleSheetList> create(Document* doc) { return adoptRef(new StyleSheetList(doc)); }
+    static PassRefPtr<StyleSheetList> create(Document* document) { return adoptRef(new StyleSheetList(document)); }
     ~StyleSheetList();
 
-    void documentDestroyed();
-
     unsigned length() const;
     StyleSheet* item(unsigned index);
 
     HTMLStyleElement* getNamedItem(const String&) const;
 
-    const StyleSheetVector& vector() const
-    {
-        return m_sheets;
-    }
-
-    void swap(StyleSheetVector& sheets)
-    {
-        m_sheets.swap(sheets);
-    }
+    Document* document() { return m_document; }
 
-    Document* document()
-    {
-        return m_doc;
-    }
+    void detachFromDocument();
 
 private:
     StyleSheetList(Document*);
+    const Vector<RefPtr<StyleSheet> >& styleSheets() const;
 
-    Document* m_doc;
-    StyleSheetVector m_sheets;
+    Document* m_document;
+    Vector<RefPtr<StyleSheet> > m_detachedStyleSheets;
 };
 
 } // namespace WebCore
index dc8df42..2d35776 100644 (file)
@@ -630,6 +630,9 @@ Document::~Document()
 
     m_decoder = 0;
 
+    if (m_styleSheetList)
+        m_styleSheetList->detachFromDocument();
+
     m_styleSheetCollection.clear();
 
     if (m_namedFlows)
@@ -778,7 +781,7 @@ void Document::setCompatibilityMode(CompatibilityMode mode)
 {
     if (m_compatibilityModeLocked || mode == m_compatibilityMode)
         return;
-    ASSERT(!m_styleSheetCollection->authorStyleSheets()->length());
+    ASSERT(m_styleSheetCollection->authorStyleSheets().isEmpty());
     bool wasInQuirksMode = inQuirksMode();
     m_compatibilityMode = mode;
     selectorQueryCache()->invalidate();
@@ -3193,7 +3196,9 @@ PassRefPtr<Node> Document::cloneNode(bool /*deep*/)
 
 StyleSheetList* Document::styleSheets()
 {
-    return m_styleSheetCollection->authorStyleSheets();
+    if (!m_styleSheetList)
+        m_styleSheetList = StyleSheetList::create(this);
+    return m_styleSheetList.get();
 }
 
 String Document::preferredStylesheetSet() const
index 8397367..f6d700a 100644 (file)
@@ -1319,6 +1319,7 @@ private:
 
 
     OwnPtr<DocumentStyleSheetCollection> m_styleSheetCollection;
+    RefPtr<StyleSheetList> m_styleSheetList;
 
     OwnPtr<FormController> m_formController;
 
index 4ce1c53..4be9613 100644 (file)
@@ -53,7 +53,6 @@ using namespace HTMLNames;
 
 DocumentStyleSheetCollection::DocumentStyleSheetCollection(Document* document)
     : m_document(document)
-    , m_authorStyleSheets(StyleSheetList::create(document))
     , m_pendingStylesheets(0)
     , m_pageGroupUserSheetCacheValid(false)
     , m_hadActiveLoadingStylesheet(false)
@@ -70,8 +69,6 @@ DocumentStyleSheetCollection::DocumentStyleSheetCollection(Document* document)
 
 DocumentStyleSheetCollection::~DocumentStyleSheetCollection()
 {
-    m_authorStyleSheets->documentDestroyed();
-
     if (m_pageUserSheet)
         m_pageUserSheet->clearOwnerNode();
     if (m_pageGroupUserSheets) {
@@ -400,11 +397,11 @@ void DocumentStyleSheetCollection::analyzeStyleSheetChange(UpdateFlag updateFlag
         return;
 
     // See if we are just adding stylesheets.
-    unsigned oldStylesheetCount = m_authorStyleSheets->length();
+    unsigned oldStylesheetCount = m_authorStyleSheets.size();
     if (newStylesheetCount < oldStylesheetCount)
         return;
     for (unsigned i = 0; i < oldStylesheetCount; ++i) {
-        if (m_authorStyleSheets->item(i) != newStylesheets[i])
+        if (m_authorStyleSheets[i] != newStylesheets[i])
             return;
     }
     requiresStyleResolverReset = false;
@@ -448,7 +445,7 @@ bool DocumentStyleSheetCollection::updateActiveStyleSheets(UpdateFlag updateFlag
     if (!m_document->renderer() || !m_document->attached())
         return false;
 
-    StyleSheetVector newStylesheets;
+    Vector<RefPtr<StyleSheet> > newStylesheets;
     collectActiveStyleSheets(newStylesheets);
 
     bool requiresStyleResolverReset;
@@ -458,12 +455,12 @@ bool DocumentStyleSheetCollection::updateActiveStyleSheets(UpdateFlag updateFlag
     if (requiresStyleResolverReset)
         m_document->clearStyleResolver();
     else {
-        m_document->styleResolver()->appendAuthorStylesheets(m_authorStyleSheets->length(), newStylesheets);
+        m_document->styleResolver()->appendAuthorStylesheets(m_authorStyleSheets.size(), newStylesheets);
         resetCSSFeatureFlags();
     }
-    m_authorStyleSheets->swap(newStylesheets);
+    m_authorStyleSheets.swap(newStylesheets);
 
-    m_usesRemUnits = styleSheetsUseRemUnits(m_authorStyleSheets->vector());
+    m_usesRemUnits = styleSheetsUseRemUnits(m_authorStyleSheets);
     m_needsUpdateActiveStylesheetsOnStyleRecalc = false;
 
     m_document->notifySeamlessChildDocumentsOfStylesheetUpdate();
index a7cea9e..a8104df 100644 (file)
@@ -49,7 +49,7 @@ public:
     DocumentStyleSheetCollection(Document*);
     ~DocumentStyleSheetCollection();
 
-    StyleSheetList* authorStyleSheets() { return m_authorStyleSheets.get(); }
+    const Vector<RefPtr<StyleSheet> >& authorStyleSheets() { return m_authorStyleSheets; }
 
     CSSStyleSheet* pageUserSheet();
     const Vector<RefPtr<CSSStyleSheet> >* pageGroupUserSheets() const;
@@ -106,7 +106,7 @@ private:
 
     Document* m_document;
 
-    RefPtr<StyleSheetList> m_authorStyleSheets;
+    Vector<RefPtr<StyleSheet> > m_authorStyleSheets;
 
     // Track the number of currently loading top-level stylesheets needed for rendering.
     // Sheets loaded using the @import directive are not included in this count.