Diff
Modified: trunk/LayoutTests/ChangeLog (124484 => 124485)
--- trunk/LayoutTests/ChangeLog 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/LayoutTests/ChangeLog 2012-08-02 19:36:04 UTC (rev 124485)
@@ -1,3 +1,13 @@
+2012-08-02 Philip Rogers <[email protected]>
+
+ Do not dispatch modification events in SVG attribute synchronization
+ https://bugs.webkit.org/show_bug.cgi?id=92604
+
+ Reviewed by Ryosuke Niwa.
+
+ * svg/custom/path-domsubtreemodified-crash-expected.txt: Added.
+ * svg/custom/path-domsubtreemodified-crash.html: Added.
+
2012-08-02 Konrad Piascik <[email protected]>
Web Inspector: Override the DeviceOrientation
Added: trunk/LayoutTests/svg/custom/path-domsubtreemodified-crash-expected.txt (0 => 124485)
--- trunk/LayoutTests/svg/custom/path-domsubtreemodified-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/path-domsubtreemodified-crash-expected.txt 2012-08-02 19:36:04 UTC (rev 124485)
@@ -0,0 +1 @@
+This test passes if no crash occurs and PASS is printed. PASS
Added: trunk/LayoutTests/svg/custom/path-domsubtreemodified-crash.html (0 => 124485)
--- trunk/LayoutTests/svg/custom/path-domsubtreemodified-crash.html (rev 0)
+++ trunk/LayoutTests/svg/custom/path-domsubtreemodified-crash.html 2012-08-02 19:36:04 UTC (rev 124485)
@@ -0,0 +1,38 @@
+<!DOCTYPE HTML>
+<html>
+<!-- Test for WK92604 - Passes if no crash occurs and "PASS" is printed. -->
+<body id="body">
+<svg xmlns="http://www.w3.org/2000/svg">
+ <line stroke="green" vector-effect="non-scaling-stroke" y2="1"/>
+ <defs id="defs"></defs>
+</svg>
+<script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ var body = document.getElementById("body");
+ var defs = document.getElementById("defs");
+ document.addEventListener("DOMContentLoaded", doCrash, false);
+
+ body.addEventListener("DOMSubtreeModified", function() {
+ // Prevent infinite loop of modification events.
+ this.removeEventListener("DOMSubtreeModified", arguments.callee);
+
+ document.write("This test passes if no crash occurs and PASS is printed. PASS");
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, false);
+
+ function doCrash() {
+ var farthestViewportElement = defs["farthestViewportElement"];
+ var externalResourcesRequired = farthestViewportElement["externalResourcesRequired"];
+ externalResourcesRequired["baseVal"] = undefined;
+ body.offsetWidth;
+ body.innerHTML = "FAIL";
+ }
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (124484 => 124485)
--- trunk/Source/WebCore/ChangeLog 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/Source/WebCore/ChangeLog 2012-08-02 19:36:04 UTC (rev 124485)
@@ -1,3 +1,41 @@
+2012-08-02 Philip Rogers <[email protected]>
+
+ Do not dispatch modification events in SVG attribute synchronization
+ https://bugs.webkit.org/show_bug.cgi?id=92604
+
+ Reviewed by Ryosuke Niwa.
+
+ Previously, calling hasAttribute() during layout could hit a
+ layout-during-layout bug because calling hasAttribute() could dispatch a
+ subtree modification event which could synchronously force a layout. hasAttribute()
+ exhibits this behavior because property synchronization is done lazily.
+
+ This patch skips dispatching subtree modification events during attribute
+ synchronization.
+
+ Additionally, this patch contains a refactoring of lazy attribute setting. We
+ now have a single place where lazy attributes are set (setSynchronizedLazyAttribute)
+ and lazy attribute flags have been moved to just Element and ElementAttributeData.
+
+ Test: svg/custom/path-domsubtreemodified-crash.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::setAttribute):
+ (WebCore::Element::setSynchronizedLazyAttribute):
+ (WebCore):
+ (WebCore::Element::setAttributeInternal):
+ * dom/Element.h:
+ (Element):
+ * dom/ElementAttributeData.cpp:
+ (WebCore::ElementAttributeData::addAttribute):
+ (WebCore::ElementAttributeData::removeAttribute):
+ * dom/ElementAttributeData.h:
+ (ElementAttributeData):
+ * dom/StyledElement.cpp:
+ (WebCore::StyledElement::updateStyleAttribute):
+ * svg/properties/SVGAnimatedPropertyMacros.h:
+ (WebCore::SVGSynchronizableAnimatedProperty::synchronize):
+
2012-08-02 Konrad Piascik <[email protected]>
Web Inspector: Override the DeviceOrientation
Modified: trunk/Source/WebCore/dom/Element.cpp (124484 => 124485)
--- trunk/Source/WebCore/dom/Element.cpp 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-08-02 19:36:04 UTC (rev 124485)
@@ -656,31 +656,36 @@
size_t index = ensureUpdatedAttributeData()->getAttributeItemIndex(localName, false);
const QualifiedName& qName = index != notFound ? attributeItem(index)->name() : QualifiedName(nullAtom, localName, nullAtom);
- setAttributeInternal(index, qName, value, NotInUpdateStyleAttribute);
+ setAttributeInternal(index, qName, value, NotInSynchronizationOfLazyAttribute);
}
-void Element::setAttribute(const QualifiedName& name, const AtomicString& value, EInUpdateStyleAttribute inUpdateStyleAttribute)
+void Element::setAttribute(const QualifiedName& name, const AtomicString& value)
{
- setAttributeInternal(ensureUpdatedAttributeData()->getAttributeItemIndex(name), name, value, inUpdateStyleAttribute);
+ setAttributeInternal(ensureUpdatedAttributeData()->getAttributeItemIndex(name), name, value, NotInSynchronizationOfLazyAttribute);
}
-inline void Element::setAttributeInternal(size_t index, const QualifiedName& name, const AtomicString& value, EInUpdateStyleAttribute inUpdateStyleAttribute)
+void Element::setSynchronizedLazyAttribute(const QualifiedName& name, const AtomicString& value)
{
+ setAttributeInternal(mutableAttributeData()->getAttributeItemIndex(name), name, value, InSynchronizationOfLazyAttribute);
+}
+
+inline void Element::setAttributeInternal(size_t index, const QualifiedName& name, const AtomicString& value, SynchronizationOfLazyAttribute inSynchronizationOfLazyAttribute)
+{
ElementAttributeData* attributeData = mutableAttributeData();
Attribute* old = index != notFound ? attributeData->attributeItem(index) : 0;
if (value.isNull()) {
if (old)
- attributeData->removeAttribute(index, this, inUpdateStyleAttribute);
+ attributeData->removeAttribute(index, this, inSynchronizationOfLazyAttribute);
return;
}
if (!old) {
- attributeData->addAttribute(Attribute(name, value), this, inUpdateStyleAttribute);
+ attributeData->addAttribute(Attribute(name, value), this, inSynchronizationOfLazyAttribute);
return;
}
- if (inUpdateStyleAttribute == NotInUpdateStyleAttribute)
+ if (inSynchronizationOfLazyAttribute == NotInSynchronizationOfLazyAttribute)
willModifyAttribute(name, old->value(), value);
if (RefPtr<Attr> attrNode = attrIfExists(name))
@@ -688,7 +693,7 @@
else
old->setValue(value);
- if (inUpdateStyleAttribute == NotInUpdateStyleAttribute)
+ if (inSynchronizationOfLazyAttribute == NotInSynchronizationOfLazyAttribute)
didModifyAttribute(Attribute(old->name(), old->value()));
}
Modified: trunk/Source/WebCore/dom/Element.h (124484 => 124485)
--- trunk/Source/WebCore/dom/Element.h 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/Source/WebCore/dom/Element.h 2012-08-02 19:36:04 UTC (rev 124485)
@@ -113,7 +113,8 @@
bool hasAttribute(const QualifiedName&) const;
const AtomicString& getAttribute(const QualifiedName&) const;
- void setAttribute(const QualifiedName&, const AtomicString& value, EInUpdateStyleAttribute = NotInUpdateStyleAttribute);
+ void setAttribute(const QualifiedName&, const AtomicString& value);
+ void setSynchronizedLazyAttribute(const QualifiedName&, const AtomicString& value);
void removeAttribute(const QualifiedName&);
void removeAttribute(size_t index);
@@ -479,7 +480,7 @@
virtual NodeType nodeType() const;
virtual bool childTypeAllowed(NodeType) const;
- void setAttributeInternal(size_t index, const QualifiedName&, const AtomicString& value, EInUpdateStyleAttribute);
+ void setAttributeInternal(size_t index, const QualifiedName&, const AtomicString& value, SynchronizationOfLazyAttribute);
#ifndef NDEBUG
virtual void formatForDebugger(char* buffer, unsigned length) const;
Modified: trunk/Source/WebCore/dom/ElementAttributeData.cpp (124484 => 124485)
--- trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-08-02 19:36:04 UTC (rev 124485)
@@ -218,20 +218,20 @@
m_inlineStyleDecl = 0;
}
-void ElementAttributeData::addAttribute(const Attribute& attribute, Element* element, EInUpdateStyleAttribute inUpdateStyleAttribute)
+void ElementAttributeData::addAttribute(const Attribute& attribute, Element* element, SynchronizationOfLazyAttribute inSynchronizationOfLazyAttribute)
{
ASSERT(isMutable());
- if (element && inUpdateStyleAttribute == NotInUpdateStyleAttribute)
+ if (element && inSynchronizationOfLazyAttribute == NotInSynchronizationOfLazyAttribute)
element->willModifyAttribute(attribute.name(), nullAtom, attribute.value());
m_mutableAttributeVector->append(attribute);
- if (element && inUpdateStyleAttribute == NotInUpdateStyleAttribute)
+ if (element && inSynchronizationOfLazyAttribute == NotInSynchronizationOfLazyAttribute)
element->didAddAttribute(attribute);
}
-void ElementAttributeData::removeAttribute(size_t index, Element* element, EInUpdateStyleAttribute inUpdateStyleAttribute)
+void ElementAttributeData::removeAttribute(size_t index, Element* element, SynchronizationOfLazyAttribute inSynchronizationOfLazyAttribute)
{
ASSERT(isMutable());
ASSERT(index < length());
@@ -239,7 +239,7 @@
Attribute& attribute = m_mutableAttributeVector->at(index);
QualifiedName name = attribute.name();
- if (element && inUpdateStyleAttribute == NotInUpdateStyleAttribute)
+ if (element && inSynchronizationOfLazyAttribute == NotInSynchronizationOfLazyAttribute)
element->willRemoveAttribute(name, attribute.value());
if (RefPtr<Attr> attr = attrIfExists(element, name))
@@ -247,7 +247,7 @@
m_mutableAttributeVector->remove(index);
- if (element && inUpdateStyleAttribute == NotInUpdateStyleAttribute)
+ if (element && inSynchronizationOfLazyAttribute == NotInSynchronizationOfLazyAttribute)
element->didRemoveAttribute(name);
}
Modified: trunk/Source/WebCore/dom/ElementAttributeData.h (124484 => 124485)
--- trunk/Source/WebCore/dom/ElementAttributeData.h 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/Source/WebCore/dom/ElementAttributeData.h 2012-08-02 19:36:04 UTC (rev 124485)
@@ -37,7 +37,7 @@
class Element;
class MemoryObjectInfo;
-enum EInUpdateStyleAttribute { NotInUpdateStyleAttribute, InUpdateStyleAttribute };
+enum SynchronizationOfLazyAttribute { NotInSynchronizationOfLazyAttribute, InSynchronizationOfLazyAttribute };
class ElementAttributeData {
WTF_MAKE_FAST_ALLOCATED;
@@ -78,9 +78,9 @@
size_t getAttributeItemIndex(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
// These functions do no error checking.
- void addAttribute(const Attribute&, Element*, EInUpdateStyleAttribute = NotInUpdateStyleAttribute);
+ void addAttribute(const Attribute&, Element*, SynchronizationOfLazyAttribute = NotInSynchronizationOfLazyAttribute);
void removeAttribute(const QualifiedName&, Element*);
- void removeAttribute(size_t index, Element*, EInUpdateStyleAttribute = NotInUpdateStyleAttribute);
+ void removeAttribute(size_t index, Element*, SynchronizationOfLazyAttribute = NotInSynchronizationOfLazyAttribute);
PassRefPtr<Attr> takeAttribute(size_t index, Element*);
bool hasID() const { return !m_idForStyleResolution.isNull(); }
Modified: trunk/Source/WebCore/dom/StyledElement.cpp (124484 => 124485)
--- trunk/Source/WebCore/dom/StyledElement.cpp 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/Source/WebCore/dom/StyledElement.cpp 2012-08-02 19:36:04 UTC (rev 124485)
@@ -127,7 +127,7 @@
ASSERT(!isStyleAttributeValid());
setIsStyleAttributeValid();
if (const StylePropertySet* inlineStyle = this->inlineStyle())
- const_cast<StyledElement*>(this)->setAttribute(styleAttr, inlineStyle->asText(), InUpdateStyleAttribute);
+ const_cast<StyledElement*>(this)->setSynchronizedLazyAttribute(styleAttr, inlineStyle->asText());
}
StyledElement::StyledElement(const QualifiedName& name, Document* document, ConstructionType type)
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h (124484 => 124485)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h 2012-08-02 19:29:26 UTC (rev 124484)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h 2012-08-02 19:36:04 UTC (rev 124485)
@@ -55,18 +55,7 @@
void synchronize(SVGElement* ownerElement, const QualifiedName& attrName, const AtomicString& value)
{
- // If the attribute already exists on the element, we change the
- // Attribute directly to avoid a call to Element::attributeChanged
- // that could cause the SVGElement to erroneously reset its properties.
- // svg/dom/SVGStringList-basics.xhtml exercises this behavior.
- ElementAttributeData* attributeData = ownerElement->mutableAttributeData();
- Attribute* old = attributeData->getAttributeItem(attrName);
- if (old && value.isNull())
- attributeData->removeAttribute(old->name(), ownerElement);
- else if (!old && !value.isNull())
- attributeData->addAttribute(Attribute(attrName, value), ownerElement);
- else if (old && !value.isNull())
- old->setValue(value);
+ ownerElement->setSynchronizedLazyAttribute(attrName, value);
}
PropertyType value;
