git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 2892bbd)
Source/WebCore: Crash due to bad data in SVGDocumentExtensions m_pendingResources
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 10 Sep 2011 11:25:03 +0000 (11:25 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 10 Sep 2011 11:25:03 +0000 (11:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=67488

Patch by Ken Buchanan <kenrb@chromium.org> on 2011-09-10
Reviewed by Nikolas Zimmermann.

Resolving a crash condition caused by the deletion of
elements while pending resource entries for those elements are still
recorded.

* rendering/svg/RenderSVGResourceContainer.cpp:
(WebCore::RenderSVGResourceContainer::registerResource)
* svg/SVGDocumentExtensions.h:
(WebCore::SVGDocumentExtensions::isElementInPendingResources)
* svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::addPendingResource)
(WebCore::SVGDocumentExtensions::isElementInPendingResources)
(WebCore::SVGDocumentExtensions::removeElementFromPendingResources)
* svg/SVGStyledElement.h:
(WebCore::SVGStyledElement::clearHasPendingResourcesIfPossible)
* svg/SVGStyledElement.cpp:
(WebCore::SVGStyledElement::buildPendingResourcesIfNeeded)
(WebCore::SVGStyledElement::clearHasPendingResourcesIfPossible)
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::svgAttributeChanged)

LayoutTests: Crash due to bad data in SVGDocumentExtensions m_pendingResources.
https://bugs.webkit.org/show_bug.cgi?id=67488

Patch by Ken Buchanan <kenrb@chromium.org> on 2011-09-10
Reviewed by Nikolas Zimmermann.

Test added: validating that the crash referenced in the bug is not present.

* svg/dom/SVGStyledElement-pendingResource-crash.html: Added.
* svg/dom/SVGStyledElement-pendingResource-crash-expected.txt: Added.
* svg/dom/resources/SVGStyledElement-pendingResource-crash.svg: Added.

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

LayoutTests/ChangeLog [changed mode: 0644->0755] patch | blob | history
LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash-expected.txt [new file with mode: 0755] patch | blob
LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash.html [new file with mode: 0755] patch | blob
LayoutTests/svg/dom/resources/SVGStyledElement-pendingResource-crash.svg [new file with mode: 0755] patch | blob
Source/WebCore/ChangeLog [changed mode: 0644->0755] patch | blob | history
Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp patch | blob | history
Source/WebCore/svg/SVGDocumentExtensions.cpp [changed mode: 0644->0755] patch | blob | history
Source/WebCore/svg/SVGDocumentExtensions.h [changed mode: 0644->0755] patch | blob | history
Source/WebCore/svg/SVGStyledElement.cpp [changed mode: 0644->0755] patch | blob | history
Source/WebCore/svg/SVGStyledElement.h [changed mode: 0644->0755] patch | blob | history
Source/WebCore/svg/SVGUseElement.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
old mode 100644 (file)
new mode 100755 (executable)
index 6c66217..e79591d
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2011-09-10  Ken Buchanan <kenrb@chromium.org>
+
+        Crash due to bad data in SVGDocumentExtensions m_pendingResources.
+        https://bugs.webkit.org/show_bug.cgi?id=67488
+
+        Reviewed by Nikolas Zimmermann.
+
+        Test added: validating that the crash referenced in the bug is not present.
+
+        * svg/dom/SVGStyledElement-pendingResource-crash.html: Added.
+        * svg/dom/SVGStyledElement-pendingResource-crash-expected.txt: Added.
+        * svg/dom/resources/SVGStyledElement-pendingResource-crash.svg: Added.
+
 2011-09-09  Erik Arvidsson  <arv@chromium.org>
 
         Move Element.contains to Node
diff --git a/LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash-expected.txt b/LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash-expected.txt
new file mode 100755 (executable)
index 0000000..d007298
--- /dev/null
+++ b/LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash-expected.txt
@@ -0,0 +1 @@
+PASS, if DumpRenderTree doesn't crash, and no assertion in a Debug build.
diff --git a/LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash.html b/LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash.html
new file mode 100755 (executable)
index 0000000..816b4a2
--- /dev/null
+++ b/LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html >
+    <script>
+        function body_start() {
+            var q = document.getElementById('root').contentDocument;
+            q.getElementsByTagName('svg')[0].replaceChild(q.getElementById('refImage'), q.getElementById('d'));
+            q.getElementsByTagName('use')[0].setAttribute('xlink:href', '#testName');
+            if (window.layoutTestController) {
+                layoutTestController.dumpAsText();
+                layoutTestController.waitUntilDone();
+            }
+            setTimeout(function () {
+                    document.body.innerHTML = "PASS, if DumpRenderTree doesn't crash, and no assertion in a Debug build.";
+                    if (window.layoutTestController)
+                        layoutTestController.notifyDone();
+               }, 0);
+        }
+    </script>
+    <object data="resources/SVGStyledElement-pendingResource-crash.svg" id="root" onload="body_start();" type="image/svg+xml"/></object>
+</html>
diff --git a/LayoutTests/svg/dom/resources/SVGStyledElement-pendingResource-crash.svg b/LayoutTests/svg/dom/resources/SVGStyledElement-pendingResource-crash.svg
new file mode 100755 (executable)
index 0000000..fcd5b2c
--- /dev/null
+++ b/LayoutTests/svg/dom/resources/SVGStyledElement-pendingResource-crash.svg
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>\r
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">\r
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">\r
+       <image id="refImage"/>\r
+       <g>\r
+               <text id="testName">X</text>\r
+               <use xlink:href="#navigationGroup" />\r
+       </g>\r
+       <defs id="d">\r
+               <g id="navigationGroup" fill='url(#testName)'>\r
+                       <a stroke='url(#refImage)'><text id="ABC">A</text></a>\r
+        </g>\r
+    </defs>\r
+</svg>\r
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
old mode 100644 (file)
new mode 100755 (executable)
index 2acf1ea..1c29f4a
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2011-09-10  Ken Buchanan <kenrb@chromium.org>
+
+        Crash due to bad data in SVGDocumentExtensions m_pendingResources
+        https://bugs.webkit.org/show_bug.cgi?id=67488
+
+        Reviewed by Nikolas Zimmermann.
+
+        Resolving a crash condition caused by the deletion of
+        elements while pending resource entries for those elements are still
+        recorded.
+
+        * rendering/svg/RenderSVGResourceContainer.cpp:
+        (WebCore::RenderSVGResourceContainer::registerResource)
+        * svg/SVGDocumentExtensions.h:
+        (WebCore::SVGDocumentExtensions::isElementInPendingResources)
+        * svg/SVGDocumentExtensions.cpp:
+        (WebCore::SVGDocumentExtensions::addPendingResource)
+        (WebCore::SVGDocumentExtensions::isElementInPendingResources)
+        (WebCore::SVGDocumentExtensions::removeElementFromPendingResources)
+        * svg/SVGStyledElement.h:
+        (WebCore::SVGStyledElement::clearHasPendingResourcesIfPossible)
+        * svg/SVGStyledElement.cpp:
+        (WebCore::SVGStyledElement::buildPendingResourcesIfNeeded)
+        (WebCore::SVGStyledElement::clearHasPendingResourcesIfPossible)
+        * svg/SVGUseElement.cpp:
+        (WebCore::SVGUseElement::svgAttributeChanged)
+
 2011-09-10  Adam Barth  <abarth@webkit.org>
 
         Remove DocumentWriter::deprecatedFrameEncoding()
diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp b/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
index 74d5d69..55f77e5 100644 (file)
--- a/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
+++ b/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
@@ -168,7 +168,7 @@ void RenderSVGResourceContainer::registerResource()
     const SVGDocumentExtensions::SVGPendingElements::const_iterator end = clients->end();
     for (SVGDocumentExtensions::SVGPendingElements::const_iterator it = clients->begin(); it != end; ++it) {
         ASSERT((*it)->hasPendingResources());
-        (*it)->setHasPendingResources(false);
+        (*it)->clearHasPendingResourcesIfPossible();
         RenderObject* renderer = (*it)->renderer();
         if (!renderer)
             continue;
diff --git a/Source/WebCore/svg/SVGDocumentExtensions.cpp b/Source/WebCore/svg/SVGDocumentExtensions.cpp
old mode 100644 (file)
new mode 100755 (executable)
index 31fea3c..c697029
--- a/Source/WebCore/svg/SVGDocumentExtensions.cpp
+++ b/Source/WebCore/svg/SVGDocumentExtensions.cpp
@@ -227,7 +227,7 @@ void SVGDocumentExtensions::addPendingResource(const AtomicString& id, SVGStyled
         m_pendingResources.add(id, set);
     }
 
-    element->setHasPendingResources(true);
+    element->setHasPendingResources();
 }
 
 bool SVGDocumentExtensions::hasPendingResources(const AtomicString& id) const
@@ -238,6 +238,24 @@ bool SVGDocumentExtensions::hasPendingResources(const AtomicString& id) const
     return m_pendingResources.contains(id);
 }
 
+bool SVGDocumentExtensions::isElementInPendingResources(SVGStyledElement* element) const
+{
+    ASSERT(element);
+
+    if (m_pendingResources.isEmpty())
+        return false;
+
+    HashMap<AtomicString, SVGPendingElements*>::const_iterator end = m_pendingResources.end();
+    for (HashMap<AtomicString, SVGPendingElements*>::const_iterator it = m_pendingResources.begin(); it != end; ++it) {
+        SVGPendingElements* elements = it->second;
+        ASSERT(elements);
+
+        if (elements->contains(element))
+            return true;
+    }
+    return false;
+}
+
 void SVGDocumentExtensions::removeElementFromPendingResources(SVGStyledElement* element)
 {
     ASSERT(element);
@@ -245,8 +263,6 @@ void SVGDocumentExtensions::removeElementFromPendingResources(SVGStyledElement*
     if (m_pendingResources.isEmpty() || !element->hasPendingResources())
         return;
 
-    element->setHasPendingResources(false);
-
     Vector<AtomicString> toBeRemoved;
     HashMap<AtomicString, SVGPendingElements*>::iterator end = m_pendingResources.end();
     for (HashMap<AtomicString, SVGPendingElements*>::iterator it = m_pendingResources.begin(); it != end; ++it) {
@@ -259,6 +275,8 @@ void SVGDocumentExtensions::removeElementFromPendingResources(SVGStyledElement*
             toBeRemoved.append(it->first);
     }
 
+    element->clearHasPendingResourcesIfPossible();
+
     if (toBeRemoved.isEmpty())
         return;
 
diff --git a/Source/WebCore/svg/SVGDocumentExtensions.h b/Source/WebCore/svg/SVGDocumentExtensions.h
old mode 100644 (file)
new mode 100755 (executable)
index a66b066..8ad44f8
--- a/Source/WebCore/svg/SVGDocumentExtensions.h
+++ b/Source/WebCore/svg/SVGDocumentExtensions.h
@@ -81,6 +81,7 @@ public:
     // For instance, dynamically build gradients / patterns / clippers...
     void addPendingResource(const AtomicString& id, SVGStyledElement*);
     bool hasPendingResources(const AtomicString& id) const;
+    bool isElementInPendingResources(SVGStyledElement*) const;
     void removeElementFromPendingResources(SVGStyledElement*);
     PassOwnPtr<SVGPendingElements> removePendingResource(const AtomicString& id);
 };
diff --git a/Source/WebCore/svg/SVGStyledElement.cpp b/Source/WebCore/svg/SVGStyledElement.cpp
old mode 100644 (file)
new mode 100755 (executable)
index d80dbe0..9880b18
--- a/Source/WebCore/svg/SVGStyledElement.cpp
+++ b/Source/WebCore/svg/SVGStyledElement.cpp
@@ -389,7 +389,7 @@ void SVGStyledElement::buildPendingResourcesIfNeeded()
     for (SVGDocumentExtensions::SVGPendingElements::const_iterator it = clients->begin(); it != end; ++it) {
         ASSERT((*it)->hasPendingResources());
         (*it)->buildPendingResource();
-        (*it)->setHasPendingResources(false);
+        (*it)->clearHasPendingResourcesIfPossible();
     }
 }
 
@@ -456,9 +456,15 @@ bool SVGStyledElement::hasPendingResources() const
     return hasRareSVGData() && rareSVGData()->hasPendingResources();
 }
 
-void SVGStyledElement::setHasPendingResources(bool value)
+void SVGStyledElement::setHasPendingResources()
 {
-    ensureRareSVGData()->setHasPendingResources(value);
+    ensureRareSVGData()->setHasPendingResources(true);
+}
+
+void SVGStyledElement::clearHasPendingResourcesIfPossible()
+{
+    if (!document()->accessSVGExtensions()->isElementInPendingResources(this))
+        ensureRareSVGData()->setHasPendingResources(false);
 }
 
 AffineTransform SVGStyledElement::localCoordinateSpaceTransform(SVGLocatable::CTMScope) const
diff --git a/Source/WebCore/svg/SVGStyledElement.h b/Source/WebCore/svg/SVGStyledElement.h
old mode 100644 (file)
new mode 100755 (executable)
index a4df4c0..8f3d7ff
--- a/Source/WebCore/svg/SVGStyledElement.h
+++ b/Source/WebCore/svg/SVGStyledElement.h
@@ -50,7 +50,8 @@ public:
     void setInstanceUpdatesBlocked(bool);
 
     bool hasPendingResources() const;
-    void setHasPendingResources(bool);
+    void setHasPendingResources();
+    void clearHasPendingResourcesIfPossible();
 
     virtual void animatedPropertyTypeForAttribute(const QualifiedName&, Vector<AnimatedPropertyType>&);
     static bool isAnimatableCSSProperty(const QualifiedName&);
diff --git a/Source/WebCore/svg/SVGUseElement.cpp b/Source/WebCore/svg/SVGUseElement.cpp
index 1892724..1c9b14e 100644 (file)
--- a/Source/WebCore/svg/SVGUseElement.cpp
+++ b/Source/WebCore/svg/SVGUseElement.cpp
@@ -204,11 +204,11 @@ void SVGUseElement::svgAttributeChanged(const QualifiedName& attrName)
             const SVGDocumentExtensions::SVGPendingElements::const_iterator end = clients->end();
             for (SVGDocumentExtensions::SVGPendingElements::const_iterator it = clients->begin(); it != end; ++it) {
                 ASSERT((*it)->hasPendingResources());
-                (*it)->setHasPendingResources(false);
+                (*it)->clearHasPendingResourcesIfPossible();
             }
 
             m_resourceId = String();
-            setHasPendingResources(false);
+            clearHasPendingResourcesIfPossible();
         }
 
         m_targetElementInstance = 0;
Mirror of the WebKit open source project from svn.webkit.org.