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: