Diff
Modified: trunk/Source/WebCore/ChangeLog (135020 => 135021)
--- trunk/Source/WebCore/ChangeLog 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/ChangeLog 2012-11-17 01:34:36 UTC (rev 135021)
@@ -1,3 +1,58 @@
+2012-11-16 Andreas Kling <akl...@apple.com>
+
+ Exploit shared attribute data to avoid parsing identical "style" attributes.
+ <http://webkit.org/b/101163>
+
+ Reviewed by Antti Koivisto.
+
+ Track the "inline style dirty" state on ElementAttributeData instead of in a Node flag.
+ This allows us to avoid duplicate work for ElementAttributeData that are shared between multiple elements,
+ since the state is no longer per-Element.
+
+ * css/StyleResolver.cpp:
+ (WebCore::isCacheableInMatchedPropertiesCache):
+
+ Disable the matched properties cache for styles with non-standard writing-mode.
+ This is necessary because some CSS properties have different meaning depending on context -
+ properties handled by CSSProperty::resolveDirectionAwareProperty().
+
+ Now that multiple elements may have identical inlineStyle() pointers, this is necessary to
+ avoid mapping StylePropertySets with direction-aware properties to RenderStyles with differing
+ writing-modes in the matched properties cache.
+
+ * dom/Node.h:
+ * dom/ElementAttributeData.cpp:
+ (WebCore::ElementAttributeData::ElementAttributeData):
+ * dom/ElementAttributeData.h:
+ (WebCore::ElementAttributeData::ElementAttributeData):
+ (ElementAttributeData):
+ * dom/Element.h:
+ (WebCore::Element::updateInvalidAttributes):
+ * dom/Element.cpp:
+ (WebCore::Element::getAttribute):
+ (WebCore::Element::removeAttribute):
+ * dom/StyledElement.h:
+ (WebCore::StyledElement::invalidateStyleAttribute):
+ * dom/StyledElement.cpp:
+ (WebCore::StyledElement::updateStyleAttribute):
+
+ Move "style attribute dirty" flag to ElementAttributeData.
+
+ (WebCore::Element::cloneAttributesFromElement):
+
+ Remove ugly optimization to avoid reparsing inline style when cloning elements. This now happens
+ automagically since cloning nodes just refs the original attribute data.
+
+ * dom/StyledElement.cpp:
+ (WebCore::StyledElement::updateStyleAttribute):
+ (WebCore::StyledElement::setInlineStyleFromString):
+ (WebCore::StyledElement::styleAttributeChanged):
+ (WebCore::StyledElement::inlineStyleChanged):
+
+ Avoid reparsing the inline style if the element's attribute data is immutable and already has
+ a parsed inlineStyle(). Split the set-inline-style-from-string code out of styleAttributeChanged()
+ to make the code more understandable.
+
2012-11-16 Simon Fraser <simon.fra...@apple.com>
Don't update layer positions on scrolling if we're in the middle of layout
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (135020 => 135021)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2012-11-17 01:34:36 UTC (rev 135021)
@@ -2404,6 +2404,8 @@
return false;
if (style->zoom() != RenderStyle::initialZoom())
return false;
+ if (style->writingMode() != RenderStyle::initialWritingMode())
+ return false;
// The cache assumes static knowledge about which properties are inherited.
if (parentStyle->hasExplicitlyInheritedProperties())
return false;
Modified: trunk/Source/WebCore/dom/Element.cpp (135020 => 135021)
--- trunk/Source/WebCore/dom/Element.cpp 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-11-17 01:34:36 UTC (rev 135021)
@@ -304,7 +304,7 @@
const AtomicString& Element::getAttribute(const QualifiedName& name) const
{
- if (UNLIKELY(name == styleAttr) && !isStyleAttributeValid())
+ if (UNLIKELY(name == styleAttr) && attributeData() && attributeData()->m_styleAttributeIsDirty)
updateStyleAttribute();
#if ENABLE(SVG)
@@ -662,7 +662,7 @@
bool ignoreCase = shouldIgnoreAttributeCase(this);
// Update the 'style' attribute if it's invalid and being requested:
- if (!isStyleAttributeValid() && equalPossiblyIgnoringCase(name, styleAttr.localName(), ignoreCase))
+ if (attributeData() && attributeData()->m_styleAttributeIsDirty && equalPossiblyIgnoringCase(name, styleAttr.localName(), ignoreCase))
updateStyleAttribute();
#if ENABLE(SVG)
@@ -1652,7 +1652,7 @@
AtomicString localName = shouldIgnoreAttributeCase(this) ? name.lower() : name;
size_t index = attributeData()->getAttributeItemIndex(localName, false);
if (index == notFound) {
- if (UNLIKELY(localName == styleAttr) && !isStyleAttributeValid() && isStyledElement())
+ if (UNLIKELY(localName == styleAttr) && attributeData()->m_styleAttributeIsDirty && isStyledElement())
static_cast<StyledElement*>(this)->removeAllInlineStyleProperties();
return;
}
@@ -2444,8 +2444,6 @@
if (hasSyntheticAttrChildNodes())
detachAllAttrNodesFromElement();
- setIsStyleAttributeValid(other.isStyleAttributeValid());
-
other.updateInvalidAttributes();
if (!other.m_attributeData) {
m_attributeData.clear();
@@ -2476,11 +2474,6 @@
for (unsigned i = 0; i < m_attributeData->length(); ++i) {
const Attribute* attribute = const_cast<const ElementAttributeData*>(m_attributeData.get())->attributeItem(i);
- // This optimization isn't very nicely factored, but a huge win for cloning elements with inline style.
- if (isStyledElement() && attribute->name() == HTMLNames::styleAttr) {
- static_cast<StyledElement*>(this)->styleAttributeChanged(attribute->value(), StyledElement::DoNotReparseStyleAttribute);
- continue;
- }
attributeChanged(attribute->name(), attribute->value());
}
}
Modified: trunk/Source/WebCore/dom/Element.h (135020 => 135021)
--- trunk/Source/WebCore/dom/Element.h 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/dom/Element.h 2012-11-17 01:34:36 UTC (rev 135021)
@@ -732,7 +732,7 @@
inline void Element::updateInvalidAttributes() const
{
- if (!isStyleAttributeValid())
+ if (attributeData() && attributeData()->m_styleAttributeIsDirty)
updateStyleAttribute();
#if ENABLE(SVG)
Modified: trunk/Source/WebCore/dom/ElementAttributeData.cpp (135020 => 135021)
--- trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-11-17 01:34:36 UTC (rev 135021)
@@ -83,6 +83,7 @@
: m_isMutable(isMutable)
, m_arraySize(isMutable ? 0 : other.length())
, m_presentationAttributeStyleIsDirty(other.m_presentationAttributeStyleIsDirty)
+ , m_styleAttributeIsDirty(other.m_styleAttributeIsDirty)
, m_presentationAttributeStyle(other.m_presentationAttributeStyle)
, m_classNames(other.m_classNames)
, m_idForStyleResolution(other.m_idForStyleResolution)
Modified: trunk/Source/WebCore/dom/ElementAttributeData.h (135020 => 135021)
--- trunk/Source/WebCore/dom/ElementAttributeData.h 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/dom/ElementAttributeData.h 2012-11-17 01:34:36 UTC (rev 135021)
@@ -89,19 +89,22 @@
: m_isMutable(true)
, m_arraySize(0)
, m_presentationAttributeStyleIsDirty(false)
+ , m_styleAttributeIsDirty(false)
{ }
ElementAttributeData(unsigned arraySize)
: m_isMutable(false)
, m_arraySize(arraySize)
, m_presentationAttributeStyleIsDirty(false)
+ , m_styleAttributeIsDirty(false)
{ }
ElementAttributeData(const ElementAttributeData&, bool isMutable);
unsigned m_isMutable : 1;
- unsigned m_arraySize : 30;
+ unsigned m_arraySize : 29;
mutable unsigned m_presentationAttributeStyleIsDirty : 1;
+ mutable unsigned m_styleAttributeIsDirty : 1;
mutable RefPtr<StylePropertySet> m_inlineStyle;
mutable RefPtr<StylePropertySet> m_presentationAttributeStyle;
Modified: trunk/Source/WebCore/dom/Node.h (135020 => 135021)
--- trunk/Source/WebCore/dom/Node.h 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/dom/Node.h 2012-11-17 01:34:36 UTC (rev 135021)
@@ -98,7 +98,7 @@
typedef int ExceptionCode;
-const int nodeStyleChangeShift = 20;
+const int nodeStyleChangeShift = 19;
// SyntheticStyleChange means that we need to go through the entire style change logic even though
// no style property has actually changed. It is used to restructure the tree when, for instance,
@@ -710,34 +710,33 @@
// These bits are used by derived classes, pulled up here so they can
// be stored in the same memory word as the Node bits above.
IsParsingChildrenFinishedFlag = 1 << 15, // Element
- IsStyleAttributeValidFlag = 1 << 16, // StyledElement
#if ENABLE(SVG)
- AreSVGAttributesValidFlag = 1 << 17, // Element
- IsSynchronizingSVGAttributesFlag = 1 << 18, // SVGElement
- HasSVGRareDataFlag = 1 << 19, // SVGElement
+ AreSVGAttributesValidFlag = 1 << 16, // Element
+ IsSynchronizingSVGAttributesFlag = 1 << 17, // SVGElement
+ HasSVGRareDataFlag = 1 << 18, // SVGElement
#endif
StyleChangeMask = 1 << nodeStyleChangeShift | 1 << (nodeStyleChangeShift + 1),
- SelfOrAncestorHasDirAutoFlag = 1 << 22,
+ SelfOrAncestorHasDirAutoFlag = 1 << 21,
- HasNameFlag = 1 << 23,
+ HasNameFlag = 1 << 22,
- InNamedFlowFlag = 1 << 24,
- HasSyntheticAttrChildNodesFlag = 1 << 25,
- HasCustomCallbacksFlag = 1 << 26,
- HasScopedHTMLStyleChildFlag = 1 << 27,
- HasEventTargetDataFlag = 1 << 28,
- InEdenFlag = 1 << 29,
+ InNamedFlowFlag = 1 << 23,
+ HasSyntheticAttrChildNodesFlag = 1 << 24,
+ HasCustomCallbacksFlag = 1 << 25,
+ HasScopedHTMLStyleChildFlag = 1 << 26,
+ HasEventTargetDataFlag = 1 << 27,
+ InEdenFlag = 1 << 28,
#if ENABLE(SVG)
- DefaultNodeFlags = IsParsingChildrenFinishedFlag | IsStyleAttributeValidFlag | AreSVGAttributesValidFlag,
+ DefaultNodeFlags = IsParsingChildrenFinishedFlag | AreSVGAttributesValidFlag,
#else
- DefaultNodeFlags = IsParsingChildrenFinishedFlag | IsStyleAttributeValidFlag,
+ DefaultNodeFlags = IsParsingChildrenFinishedFlag,
#endif
};
- // 2 bits remaining
+ // 3 bits remaining
bool getFlag(NodeFlags mask) const { return m_nodeFlags & mask; }
void setFlag(bool f, NodeFlags mask) const { m_nodeFlags = (m_nodeFlags & ~mask) | (-(int32_t)f & mask); }
@@ -838,12 +837,6 @@
NodeRareDataBase* m_rareData;
} m_data;
-public:
- bool isStyleAttributeValid() const { return getFlag(IsStyleAttributeValidFlag); }
- void setIsStyleAttributeValid(bool f) { setFlag(f, IsStyleAttributeValidFlag); }
- void setIsStyleAttributeValid() const { setFlag(IsStyleAttributeValidFlag); }
- void clearIsStyleAttributeValid() { clearFlag(IsStyleAttributeValidFlag); }
-
protected:
bool isParsingChildrenFinished() const { return getFlag(IsParsingChildrenFinishedFlag); }
void setIsParsingChildrenFinished() { setFlag(IsParsingChildrenFinishedFlag); }
Modified: trunk/Source/WebCore/dom/StyledElement.cpp (135020 => 135021)
--- trunk/Source/WebCore/dom/StyledElement.cpp 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/dom/StyledElement.cpp 2012-11-17 01:34:36 UTC (rev 135021)
@@ -128,8 +128,9 @@
void StyledElement::updateStyleAttribute() const
{
- ASSERT(!isStyleAttributeValid());
- setIsStyleAttributeValid();
+ ASSERT(attributeData());
+ ASSERT(attributeData()->m_styleAttributeIsDirty);
+ attributeData()->m_styleAttributeIsDirty = false;
if (const StylePropertySet* inlineStyle = this->inlineStyle())
const_cast<StyledElement*>(this)->setSynchronizedLazyAttribute(styleAttr, inlineStyle->asText());
}
@@ -181,30 +182,41 @@
return cssomWrapper;
}
-void StyledElement::styleAttributeChanged(const AtomicString& newStyleString, ShouldReparseStyleAttribute shouldReparse)
+inline void StyledElement::setInlineStyleFromString(const AtomicString& newStyleString)
{
- if (shouldReparse) {
- WTF::OrdinalNumber startLineNumber = WTF::OrdinalNumber::beforeFirst();
- if (document() && document()->scriptableDocumentParser() && !document()->isInDocumentWrite())
- startLineNumber = document()->scriptableDocumentParser()->lineNumber();
+ RefPtr<StylePropertySet>& inlineStyle = attributeData()->m_inlineStyle;
- if (newStyleString.isNull()) {
- if (PropertySetCSSStyleDeclaration* cssomWrapper = inlineStyleCSSOMWrapper())
- cssomWrapper->clearParentElement();
- mutableAttributeData()->m_inlineStyle.clear();
- } else if (document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber)) {
- // We reconstruct the property set instead of mutating if there is no CSSOM wrapper.
- // This makes wrapperless property sets immutable and so cacheable.
- RefPtr<StylePropertySet>& inlineStyle = attributeData()->m_inlineStyle;
- if (inlineStyle && !inlineStyle->isMutable())
- inlineStyle.clear();
- if (!inlineStyle)
- inlineStyle = CSSParser::parseInlineStyleDeclaration(newStyleString, this);
- else
- inlineStyle->parseDeclaration(newStyleString, document()->elementSheet()->contents());
- }
- setIsStyleAttributeValid();
- }
+ // Avoid redundant work if we're using shared attribute data with already parsed inline style.
+ if (inlineStyle && !attributeData()->isMutable())
+ return;
+
+ // We reconstruct the property set instead of mutating if there is no CSSOM wrapper.
+ // This makes wrapperless property sets immutable and so cacheable.
+ if (inlineStyle && !inlineStyle->isMutable())
+ inlineStyle.clear();
+
+ if (!inlineStyle)
+ inlineStyle = CSSParser::parseInlineStyleDeclaration(newStyleString, this);
+ else
+ inlineStyle->parseDeclaration(newStyleString, document()->elementSheet()->contents());
+}
+
+void StyledElement::styleAttributeChanged(const AtomicString& newStyleString)
+{
+ WTF::OrdinalNumber startLineNumber = WTF::OrdinalNumber::beforeFirst();
+ if (document() && document()->scriptableDocumentParser() && !document()->isInDocumentWrite())
+ startLineNumber = document()->scriptableDocumentParser()->lineNumber();
+
+ if (newStyleString.isNull()) {
+ if (PropertySetCSSStyleDeclaration* cssomWrapper = inlineStyleCSSOMWrapper())
+ cssomWrapper->clearParentElement();
+ mutableAttributeData()->m_inlineStyle.clear();
+ } else if (document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber))
+ setInlineStyleFromString(newStyleString);
+
+ attributeData()->m_styleAttributeIsDirty = false;
+
+ // FIXME: Why aren't we using setNeedsStyleRecalc(InlineStyleChange) here?
setNeedsStyleRecalc();
InspectorInstrumentation::didInvalidateStyleAttr(document(), this);
}
@@ -220,7 +232,8 @@
void StyledElement::inlineStyleChanged()
{
setNeedsStyleRecalc(InlineStyleChange);
- setIsStyleAttributeValid(false);
+ ASSERT(attributeData());
+ attributeData()->m_styleAttributeIsDirty = true;
InspectorInstrumentation::didInvalidateStyleAttr(document(), this);
}
Modified: trunk/Source/WebCore/dom/StyledElement.h (135020 => 135021)
--- trunk/Source/WebCore/dom/StyledElement.h 2012-11-17 01:34:20 UTC (rev 135020)
+++ trunk/Source/WebCore/dom/StyledElement.h 2012-11-17 01:34:36 UTC (rev 135021)
@@ -56,10 +56,6 @@
virtual void collectStyleForPresentationAttribute(const Attribute&, StylePropertySet*) { }
- // May be called by ElementAttributeData::cloneDataFrom().
- enum ShouldReparseStyleAttribute { DoNotReparseStyleAttribute = 0, ReparseStyleAttribute = 1 };
- void styleAttributeChanged(const AtomicString& newStyleString, ShouldReparseStyleAttribute = ReparseStyleAttribute);
-
protected:
StyledElement(const QualifiedName&, Document*, ConstructionType);
@@ -75,9 +71,12 @@
virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
private:
+ void styleAttributeChanged(const AtomicString& newStyleString);
+
virtual void updateStyleAttribute() const;
void inlineStyleChanged();
PropertySetCSSStyleDeclaration* inlineStyleCSSOMWrapper();
+ void setInlineStyleFromString(const AtomicString&);
void makePresentationAttributeCacheKey(PresentationAttributeCacheKey&) const;
void rebuildPresentationAttributeStyle();
@@ -85,7 +84,8 @@
inline void StyledElement::invalidateStyleAttribute()
{
- clearIsStyleAttributeValid();
+ ASSERT(attributeData());
+ attributeData()->m_styleAttributeIsDirty = true;
}
inline const StylePropertySet* StyledElement::presentationAttributeStyle()
