Title: [154268] trunk/Source/WebCore
Revision
154268
Author
[email protected]
Date
2013-08-19 01:56:27 -0700 (Mon, 19 Aug 2013)

Log Message

<https://webkit.org/b/120001> Clean up StyleElement

Reviewed by Andreas Kling.

- Make it non-virtual so we don't use virtual multiple inheritance
- Improve names
- Improve code clarity

* dom/StyleElement.cpp:
(WebCore::StyleElement::StyleElement):
(WebCore::StyleElement::insertedIntoDocument):
(WebCore::StyleElement::clearDocumentData):
(WebCore::StyleElement::childrenChanged):
(WebCore::StyleElement::finishParsingChildren):
(WebCore::StyleElement::createSheetFromTextContents):
(WebCore::isValidCSSContentType):
(WebCore::StyleElement::createSheet):
(WebCore::StyleElement::isLoading):
* dom/StyleElement.h:
(WebCore::StyleElement::setStyleSheetContentType):
(WebCore::StyleElement::setStyleSheetMedia):
* html/HTMLStyleElement.cpp:
(WebCore::HTMLStyleElement::parseAttribute):
* html/HTMLStyleElement.h:
* svg/SVGStyleElement.cpp:
(WebCore::SVGStyleElement::isSupportedAttribute):
(WebCore::SVGStyleElement::parseAttribute):
* svg/SVGStyleElement.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (154267 => 154268)


--- trunk/Source/WebCore/ChangeLog	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/ChangeLog	2013-08-19 08:56:27 UTC (rev 154268)
@@ -1,3 +1,34 @@
+2013-08-19  Antti Koivisto  <[email protected]>
+
+        <https://webkit.org/b/120001> Clean up StyleElement
+
+        Reviewed by Andreas Kling.
+
+        - Make it non-virtual so we don't use virtual multiple inheritance
+        - Improve names
+        - Improve code clarity
+
+        * dom/StyleElement.cpp:
+        (WebCore::StyleElement::StyleElement):
+        (WebCore::StyleElement::insertedIntoDocument):
+        (WebCore::StyleElement::clearDocumentData):
+        (WebCore::StyleElement::childrenChanged):
+        (WebCore::StyleElement::finishParsingChildren):
+        (WebCore::StyleElement::createSheetFromTextContents):
+        (WebCore::isValidCSSContentType):
+        (WebCore::StyleElement::createSheet):
+        (WebCore::StyleElement::isLoading):
+        * dom/StyleElement.h:
+        (WebCore::StyleElement::setStyleSheetContentType):
+        (WebCore::StyleElement::setStyleSheetMedia):
+        * html/HTMLStyleElement.cpp:
+        (WebCore::HTMLStyleElement::parseAttribute):
+        * html/HTMLStyleElement.h:
+        * svg/SVGStyleElement.cpp:
+        (WebCore::SVGStyleElement::isSupportedAttribute):
+        (WebCore::SVGStyleElement::parseAttribute):
+        * svg/SVGStyleElement.h:
+
 2013-08-19  Gyuyoung Kim  <[email protected]>
 
         <https://webkit.org/b/119990> Add toSVGStopElement(Node* node) to clean-up a static_cast<SVGStopElement*>

Modified: trunk/Source/WebCore/css/MediaQueryEvaluator.cpp (154267 => 154268)


--- trunk/Source/WebCore/css/MediaQueryEvaluator.cpp	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/css/MediaQueryEvaluator.cpp	2013-08-19 08:56:27 UTC (rev 154268)
@@ -91,14 +91,6 @@
 {
 }
 
-MediaQueryEvaluator::MediaQueryEvaluator(const char* acceptedMediaType, bool mediaFeatureResult)
-    : m_mediaType(acceptedMediaType)
-    , m_frame(0)
-    , m_style(0)
-    , m_expResult(mediaFeatureResult)
-{
-}
-
 MediaQueryEvaluator::MediaQueryEvaluator(const String& acceptedMediaType, Frame* frame, RenderStyle* style)
     : m_mediaType(acceptedMediaType)
     , m_frame(frame)

Modified: trunk/Source/WebCore/css/MediaQueryEvaluator.h (154267 => 154268)


--- trunk/Source/WebCore/css/MediaQueryEvaluator.h	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/css/MediaQueryEvaluator.h	2013-08-19 08:56:27 UTC (rev 154268)
@@ -63,7 +63,6 @@
      *  for any media features
      */
     MediaQueryEvaluator(const String& acceptedMediaType, bool mediaFeatureResult = false);
-    MediaQueryEvaluator(const char* acceptedMediaType, bool mediaFeatureResult = false);
 
     /** Creates evaluator which evaluates full media queries
      */

Modified: trunk/Source/WebCore/dom/StyleElement.cpp (154267 => 154268)


--- trunk/Source/WebCore/dom/StyleElement.cpp	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/dom/StyleElement.cpp	2013-08-19 08:56:27 UTC (rev 154268)
@@ -36,13 +36,8 @@
 
 namespace WebCore {
 
-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_isParsingChildren(createdByParser)
     , m_loading(false)
     , m_startLineNumber(WTF::OrdinalNumber::beforeFirst())
 {
@@ -58,11 +53,11 @@
 {
     ASSERT(document);
     ASSERT(element);
-    document->styleSheetCollection()->addStyleSheetCandidateNode(element, m_createdByParser);
-    if (m_createdByParser)
+    document->styleSheetCollection()->addStyleSheetCandidateNode(element, m_isParsingChildren);
+
+    if (m_isParsingChildren)
         return;
-
-    process(element);
+    createSheetFromTextContents(element);
 }
 
 void StyleElement::removedFromDocument(Document* document, Element* element)
@@ -84,34 +79,32 @@
     if (m_sheet)
         m_sheet->clearOwnerNode();
 
-    if (element->inDocument())
-        document->styleSheetCollection()->removeStyleSheetCandidateNode(element);
+    if (!element->inDocument())
+        return;
+    document->styleSheetCollection()->removeStyleSheetCandidateNode(element);
 }
 
 void StyleElement::childrenChanged(Element* element)
 {
     ASSERT(element);
-    if (m_createdByParser)
+    if (m_isParsingChildren)
         return;
-
-    process(element);
+    if (!element->inDocument())
+        return;
+    createSheetFromTextContents(element);
 }
 
 void StyleElement::finishParsingChildren(Element* element)
 {
     ASSERT(element);
-    process(element);
-    m_createdByParser = false;
+    if (element->inDocument())
+        createSheetFromTextContents(element);
+    m_isParsingChildren = false;
 }
 
-void StyleElement::process(Element* element)
+void StyleElement::createSheetFromTextContents(Element* element)
 {
-    if (!element || !element->inDocument())
-        return;
-
-    String sheetText = TextNodeTraversal::contentsAsString(element);
-
-    createSheet(element, m_startLineNumber, sheetText);
+    createSheet(element, m_startLineNumber, TextNodeTraversal::contentsAsString(element));
 }
 
 void StyleElement::clearSheet()
@@ -120,41 +113,52 @@
     m_sheet.release()->clearOwnerNode();
 }
 
-void StyleElement::createSheet(Element* e, WTF::OrdinalNumber startLineNumber, const String& text)
+inline bool isValidCSSContentType(Element* element, const AtomicString& type)
 {
-    ASSERT(e);
-    ASSERT(e->inDocument());
-    Document* document = e->document();
+    DEFINE_STATIC_LOCAL(const AtomicString, cssContentType, ("text/css", AtomicString::ConstructFromLiteral));
+    if (type.isEmpty())
+        return true;
+    return element->isHTMLElement() ? equalIgnoringCase(type, cssContentType) : type == cssContentType;
+}
+
+void StyleElement::createSheet(Element* element, WTF::OrdinalNumber startLineNumber, const String& text)
+{
+    ASSERT(element);
+    ASSERT(element->inDocument());
+    Document* document = element->document();
     if (m_sheet) {
         if (m_sheet->isLoading())
             document->styleSheetCollection()->removePendingSheet();
         clearSheet();
     }
 
-    // If type is empty or CSS, this is a CSS style sheet.
-    const AtomicString& type = this->type();
-    if (document->contentSecurityPolicy()->allowInlineStyle(e->document()->url(), startLineNumber) && isCSS(e, type)) {
-        RefPtr<MediaQuerySet> mediaQueries;
-        if (e->isHTMLElement())
-            mediaQueries = MediaQuerySet::createAllowingDescriptionSyntax(media());
-        else
-            mediaQueries = MediaQuerySet::create(media());
+    if (!isValidCSSContentType(element, m_contentType))
+        return;
+    if (!document->contentSecurityPolicy()->allowInlineStyle(document->url(), startLineNumber))
+        return;
 
-        MediaQueryEvaluator screenEval("screen", true);
-        MediaQueryEvaluator printEval("print", true);
-        if (screenEval.eval(mediaQueries.get()) || printEval.eval(mediaQueries.get())) {
-            document->styleSheetCollection()->addPendingSheet();
-            m_loading = true;
+    RefPtr<MediaQuerySet> mediaQueries;
+    if (element->isHTMLElement())
+        mediaQueries = MediaQuerySet::createAllowingDescriptionSyntax(m_media);
+    else
+        mediaQueries = MediaQuerySet::create(m_media);
 
-            m_sheet = CSSStyleSheet::createInline(e, KURL(), document->inputEncoding());
-            m_sheet->setMediaQueries(mediaQueries.release());
-            m_sheet->setTitle(e->title());
-            m_sheet->contents()->parseStringAtLine(text, startLineNumber.zeroBasedInt(), m_createdByParser);
+    MediaQueryEvaluator screenEval(ASCIILiteral("screen"), true);
+    MediaQueryEvaluator printEval(ASCIILiteral("print"), true);
+    if (!screenEval.eval(mediaQueries.get()) && !printEval.eval(mediaQueries.get()))
+        return;
 
-            m_loading = false;
-        }
-    }
+    document->styleSheetCollection()->addPendingSheet();
 
+    m_loading = true;
+
+    m_sheet = CSSStyleSheet::createInline(element, KURL(), document->inputEncoding());
+    m_sheet->setMediaQueries(mediaQueries.release());
+    m_sheet->setTitle(element->title());
+    m_sheet->contents()->parseStringAtLine(text, startLineNumber.zeroBasedInt(), m_isParsingChildren);
+
+    m_loading = false;
+
     if (m_sheet)
         m_sheet->contents()->checkLoaded();
 }
@@ -163,7 +167,7 @@
 {
     if (m_loading)
         return true;
-    return m_sheet ? m_sheet->isLoading() : false;
+    return m_sheet && m_sheet->isLoading();
 }
 
 bool StyleElement::sheetLoaded(Document* document)

Modified: trunk/Source/WebCore/dom/StyleElement.h (154267 => 154268)


--- trunk/Source/WebCore/dom/StyleElement.h	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/dom/StyleElement.h	2013-08-19 08:56:27 UTC (rev 154268)
@@ -32,11 +32,11 @@
 class StyleElement {
 public:
     StyleElement(Document*, bool createdByParser);
-    virtual ~StyleElement();
+    ~StyleElement();
 
 protected:
-    virtual const AtomicString& type() const = 0;
-    virtual const AtomicString& media() const = 0;
+    void setStyleSheetContentType(const AtomicString& contentType) { m_contentType = contentType; }
+    void setStyleSheetMedia(const AtomicString& media) { m_media = media; }
 
     CSSStyleSheet* sheet() const { return m_sheet.get(); }
 
@@ -50,16 +50,18 @@
     void childrenChanged(Element*);
     void finishParsingChildren(Element*);
 
-    RefPtr<CSSStyleSheet> m_sheet;
-
 private:
     void createSheet(Element*, WTF::OrdinalNumber startLineNumber, const String& text = String());
-    void process(Element*);
+    void createSheetFromTextContents(Element*);
     void clearSheet();
 
-    bool m_createdByParser;
+    bool m_isParsingChildren;
     bool m_loading;
     WTF::OrdinalNumber m_startLineNumber;
+    AtomicString m_contentType;
+    AtomicString m_media;
+    RefPtr<CSSStyleSheet> m_sheet;
+
 };
 
 }

Modified: trunk/Source/WebCore/html/HTMLStyleElement.cpp (154267 => 154268)


--- trunk/Source/WebCore/html/HTMLStyleElement.cpp	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/html/HTMLStyleElement.cpp	2013-08-19 08:56:27 UTC (rev 154268)
@@ -72,14 +72,17 @@
 
 void HTMLStyleElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
-    if (name == titleAttr && m_sheet)
-        m_sheet->setTitle(value);
+    if (name == titleAttr && sheet())
+        sheet()->setTitle(value);
     else if (name == scopedAttr && ContextFeatures::styleScopedEnabled(document()))
         scopedAttributeChanged(!value.isNull());
-    else if (name == mediaAttr && inDocument() && document()->renderer() && m_sheet) {
-        m_sheet->setMediaQueries(MediaQuerySet::createAllowingDescriptionSyntax(value));
+    else if (name == mediaAttr && inDocument() && document()->renderer() && sheet()) {
+        setStyleSheetMedia(value);
+        sheet()->setMediaQueries(MediaQuerySet::createAllowingDescriptionSyntax(value));
         document()->styleResolverChanged(RecalcStyleImmediately);
-    } else
+    } else if (name == typeAttr)
+        setStyleSheetContentType(value);
+    else
         HTMLElement::parseAttribute(name, value);
 }
 
@@ -206,16 +209,6 @@
     StyleElement::childrenChanged(this);
 }
 
-const AtomicString& HTMLStyleElement::media() const
-{
-    return getAttribute(mediaAttr);
-}
-
-const AtomicString& HTMLStyleElement::type() const
-{
-    return getAttribute(typeAttr);
-}
-
 bool HTMLStyleElement::scoped() const
 {
     return fastHasAttribute(scopedAttr) && ContextFeatures::styleScopedEnabled(document());
@@ -274,10 +267,10 @@
 
 bool HTMLStyleElement::disabled() const
 {
-    if (!m_sheet)
+    if (!sheet())
         return false;
 
-    return m_sheet->disabled();
+    return sheet()->disabled();
 }
 
 void HTMLStyleElement::setDisabled(bool setDisabled)

Modified: trunk/Source/WebCore/html/HTMLStyleElement.h (154267 => 154268)


--- trunk/Source/WebCore/html/HTMLStyleElement.h	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/html/HTMLStyleElement.h	2013-08-19 08:56:27 UTC (rev 154268)
@@ -39,8 +39,6 @@
     static PassRefPtr<HTMLStyleElement> create(const QualifiedName&, Document*, bool createdByParser);
     virtual ~HTMLStyleElement();
 
-    void setType(const AtomicString&);
-
     bool scoped() const;
     void setScoped(bool);
     Element* scopingElement() const;
@@ -79,9 +77,6 @@
 
     virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
 
-    virtual const AtomicString& media() const;
-    virtual const AtomicString& type() const;
-
     void scopedAttributeChanged(bool);
     void registerWithScopingNode(bool);
     void unregisterWithScopingNode(ContainerNode*);

Modified: trunk/Source/WebCore/svg/SVGStyleElement.cpp (154267 => 154268)


--- trunk/Source/WebCore/svg/SVGStyleElement.cpp	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/svg/SVGStyleElement.cpp	2013-08-19 08:56:27 UTC (rev 154268)
@@ -54,10 +54,7 @@
 
 bool SVGStyleElement::disabled() const
 {
-    if (!m_sheet)
-        return false;
-    
-    return m_sheet->disabled();
+    return sheet() && sheet()->disabled();
 }
 
 void SVGStyleElement::setDisabled(bool setDisabled)
@@ -106,6 +103,8 @@
     if (supportedAttributes.isEmpty()) {
         SVGLangSpace::addSupportedAttributes(supportedAttributes);
         supportedAttributes.add(SVGNames::titleAttr);
+        supportedAttributes.add(SVGNames::mediaAttr);
+        supportedAttributes.add(SVGNames::typeAttr);
     }
     return supportedAttributes.contains<SVGAttributeHashTranslator>(attrName);
 }
@@ -116,13 +115,19 @@
         SVGElement::parseAttribute(name, value);
         return;
     }
-
     if (name == SVGNames::titleAttr) {
-        if (m_sheet)
-            m_sheet->setTitle(value);
+        if (sheet())
+            sheet()->setTitle(value);
         return;
     }
-
+    if (name == SVGNames::typeAttr) {
+        setStyleSheetContentType(value);
+        return;
+    }
+    if (name == SVGNames::mediaAttr) {
+        setStyleSheetMedia(value);
+        return;
+    }
     if (SVGLangSpace::parseAttribute(name, value))
         return;
 

Modified: trunk/Source/WebCore/svg/SVGStyleElement.h (154267 => 154268)


--- trunk/Source/WebCore/svg/SVGStyleElement.h	2013-08-19 08:44:56 UTC (rev 154267)
+++ trunk/Source/WebCore/svg/SVGStyleElement.h	2013-08-19 08:56:27 UTC (rev 154268)
@@ -27,8 +27,7 @@
 
 namespace WebCore {
 
-class SVGStyleElement FINAL : public SVGElement
-                            , public StyleElement {
+class SVGStyleElement FINAL : public SVGElement, private StyleElement {
 public:
     static PassRefPtr<SVGStyleElement> create(const QualifiedName&, Document*, bool createdByParser);
     virtual ~SVGStyleElement();
@@ -38,13 +37,13 @@
     bool disabled() const;
     void setDisabled(bool);
                           
-    virtual const AtomicString& type() const;
+    const AtomicString& type() const;
     void setType(const AtomicString&, ExceptionCode&);
 
-    virtual const AtomicString& media() const;
+    const AtomicString& media() const;
     void setMedia(const AtomicString&, ExceptionCode&);
 
-    virtual String title() const;
+    String title() const;
     void setTitle(const AtomicString&, ExceptionCode&);
 
 private:
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to