[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

zimmermann at webkit.org zimmermann at webkit.org
Wed Dec 22 15:26:27 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit f3c00cf171ad3eb2a1da66434440101650f8dc03
Author: zimmermann at webkit.org <zimmermann at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Wed Nov 3 15:44:16 2010 +0000

    2010-11-03  Nikolas Zimmermann  <nzimmermann at rim.com>
    
            Reviewed by Andreas Kling.
    
            chrome.dll!WebCore::SVGLength::SVGLength ...
            https://bugs.webkit.org/show_bug.cgi?id=48831
    
            Test: svg/dom/baseVal-animVal-crash.html
    
            * svg/properties/SVGListPropertyTearOff.h:
            (WebCore::SVGListPropertyTearOff::initialize): Renamed removeItemFromListIfNeeded to processIncomingListItem, to reflect its new job.
            (WebCore::SVGListPropertyTearOff::insertItemBefore): Ditto.
            (WebCore::SVGListPropertyTearOff::replaceItem): Ditto.
            (WebCore::SVGListPropertyTearOff::appendItem): Ditto.
            (WebCore::SVGListPropertyTearOff::processIncomingListItem): Copy incoming item, if necessary, see inline comments.
            * svg/properties/SVGPropertyTearOff.h:
            (WebCore::SVGPropertyTearOff::detachWrapper): Remove association with SVGAnimatedProperty, when wrapper is detached.
    
    2010-11-03  Nikolas Zimmermann  <nzimmermann at rim.com>
    
            Reviewed by Andreas Kling.
    
            chrome.dll!WebCore::SVGLength::SVGLength ...
            https://bugs.webkit.org/show_bug.cgi?id=48831
    
            * svg/dom/baseVal-animVal-crash-expected.txt: Added.
            * svg/dom/baseVal-animVal-crash.html: Added.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71240 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 881e623..747198a 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,5 +1,15 @@
 2010-11-03  Nikolas Zimmermann  <nzimmermann at rim.com>
 
+        Reviewed by Andreas Kling.
+
+        chrome.dll!WebCore::SVGLength::SVGLength ...
+        https://bugs.webkit.org/show_bug.cgi?id=48831
+
+        * svg/dom/baseVal-animVal-crash-expected.txt: Added.
+        * svg/dom/baseVal-animVal-crash.html: Added.
+
+2010-11-03  Nikolas Zimmermann  <nzimmermann at rim.com>
+
         Reviewed by Dirk Schulze.
 
         chrome.dll!WebCore::SVGListPropertyTearOff<...>::getItem ReadAV at NULL (578c0f7f21ca517ba29a4eafb7099c1b)
diff --git a/LayoutTests/fast/frames/javascript-url-for-deleted-frame-expected.txt b/LayoutTests/svg/dom/baseVal-animVal-crash-expected.txt
similarity index 100%
copy from LayoutTests/fast/frames/javascript-url-for-deleted-frame-expected.txt
copy to LayoutTests/svg/dom/baseVal-animVal-crash-expected.txt
diff --git a/LayoutTests/svg/dom/baseVal-animVal-crash.html b/LayoutTests/svg/dom/baseVal-animVal-crash.html
new file mode 100644
index 0000000..1699996
--- /dev/null
+++ b/LayoutTests/svg/dom/baseVal-animVal-crash.html
@@ -0,0 +1,26 @@
+<html>
+<head>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function go() {
+    var oSVGAltGlyphElement = document.createElementNS("http://www.w3.org/2000/svg", "altGlyph");
+    var oSvgMaskElement = document.createElementNS("http://www.w3.org/2000/svg", "mask");
+    var oSvgLengthList = oSVGAltGlyphElement.dy.baseVal;
+    oSvgLengthList.appendItem(oSvgMaskElement.width.animVal);
+    gc();
+    oSvgLengthList.appendItem(oSvgMaskElement.width.animVal);
+    gc();
+    oSvgLengthList.removeItem(0);
+    gc();
+    oSvgLengthList.appendItem(oSvgMaskElement.width.animVal);
+    gc(); 
+}
+</script>
+</head>
+<body onload="go()">
+This test passes if it doesn't crash.
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index a9c81b6..56e0926 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,21 @@
+2010-11-03  Nikolas Zimmermann  <nzimmermann at rim.com>
+
+        Reviewed by Andreas Kling.
+
+        chrome.dll!WebCore::SVGLength::SVGLength ...
+        https://bugs.webkit.org/show_bug.cgi?id=48831
+
+        Test: svg/dom/baseVal-animVal-crash.html
+
+        * svg/properties/SVGListPropertyTearOff.h:
+        (WebCore::SVGListPropertyTearOff::initialize): Renamed removeItemFromListIfNeeded to processIncomingListItem, to reflect its new job.
+        (WebCore::SVGListPropertyTearOff::insertItemBefore): Ditto.
+        (WebCore::SVGListPropertyTearOff::replaceItem): Ditto.
+        (WebCore::SVGListPropertyTearOff::appendItem): Ditto.
+        (WebCore::SVGListPropertyTearOff::processIncomingListItem): Copy incoming item, if necessary, see inline comments.
+        * svg/properties/SVGPropertyTearOff.h:
+        (WebCore::SVGPropertyTearOff::detachWrapper): Remove association with SVGAnimatedProperty, when wrapper is detached.
+
 2010-11-03  Kenneth Rohde Christiansen  <kenneth at webkit.org>
 
         Reviewed by Andreas Kling.
diff --git a/WebCore/svg/properties/SVGListPropertyTearOff.h b/WebCore/svg/properties/SVGListPropertyTearOff.h
index 1710b16..2ba925c 100644
--- a/WebCore/svg/properties/SVGListPropertyTearOff.h
+++ b/WebCore/svg/properties/SVGListPropertyTearOff.h
@@ -111,7 +111,7 @@ public:
         ASSERT(values.size() == wrappers.size());
 
         // Spec: If the inserted item is already in a list, it is removed from its previous list before it is inserted into this list.
-        removeItemFromListIfNeeded(newItem.get(), 0);
+        processIncomingListItem(newItem, 0);
 
         // Spec: Clears all existing current items from the list and re-initializes the list to hold the single item specified by the parameter.
         m_animatedProperty->detachListWrappers(0);
@@ -173,7 +173,7 @@ public:
         ASSERT(values.size() == wrappers.size());
 
         // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list.
-        removeItemFromListIfNeeded(newItem.get(), &index);
+        processIncomingListItem(newItem, &index);
 
         // Spec: Inserts a new item into the list at the specified position. The index of the item before which the new item is to be
         // inserted. The first item is number 0. If the index is equal to 0, then the new item is inserted at the front of the list.
@@ -211,7 +211,7 @@ public:
 
         // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list.
         // Spec: If the item is already in this list, note that the index of the item to replace is before the removal of the item.
-        removeItemFromListIfNeeded(newItem.get(), &index);
+        processIncomingListItem(newItem, &index);
 
         // Detach the existing wrapper.
         RefPtr<ListItemTearOff>& oldItem = wrappers.at(index);
@@ -275,7 +275,7 @@ public:
         ASSERT(values.size() == wrappers.size());
 
         // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list.
-        removeItemFromListIfNeeded(newItem.get(), 0);
+        processIncomingListItem(newItem, 0);
 
         // Append the value and wrapper at the end of the list.
         values.append(newItem->propertyReference());
@@ -313,16 +313,31 @@ private:
         m_animatedProperty->commitChange();
     }
 
-    void removeItemFromListIfNeeded(ListItemTearOff* newItem, unsigned* indexToModify)
+    void processIncomingListItem(RefPtr<ListItemTearOff>& newItem, unsigned* indexToModify)
     {
-        // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list.
         SVGAnimatedProperty* animatedPropertyOfItem = newItem->animatedProperty();
-        if (!animatedPropertyOfItem || !animatedPropertyOfItem->isAnimatedListTearOff())
+
+        // newItem has been created manually, it doesn't belong to any SVGElement.
+        // (for example: "textElement.x.baseVal.appendItem(svgsvgElement.createSVGLength())")
+        if (!animatedPropertyOfItem)
+            return;
+
+        // newItem belongs to a SVGElement, but its associated SVGAnimatedProperty is not an animated list tear off.
+        // (for example: "textElement.x.baseVal.appendItem(rectElement.width.baseVal)")
+        if (!animatedPropertyOfItem->isAnimatedListTearOff()) {
+            // We have to copy the incoming newItem, as we're not allowed to insert this tear off as is into our wrapper cache.
+            // Otherwhise we'll end up having two SVGAnimatedPropertys that operate on the same SVGPropertyTearOff. Consider the example above:
+            // SVGRectElements SVGAnimatedLength 'width' property baseVal points to the same tear off object
+            // that's inserted into SVGTextElements SVGAnimatedLengthList 'x'. textElement.x.baseVal.getItem(0).value += 150 would
+            // mutate the rectElement width _and_ the textElement x list. That's obviously wrong, take care of that.
+            newItem = ListItemTearOff::create(newItem->propertyReference());
             return;
+        }
 
+        // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list.
         // 'newItem' is already living in another list. If it's not our list, synchronize the other lists wrappers after the removal.
         bool livesInOtherList = animatedPropertyOfItem != m_animatedProperty;
-        int removedIndex = static_cast<SVGAnimatedListPropertyTearOff<PropertyType>*>(animatedPropertyOfItem)->removeItemFromList(newItem, livesInOtherList);
+        int removedIndex = static_cast<SVGAnimatedListPropertyTearOff<PropertyType>*>(animatedPropertyOfItem)->removeItemFromList(newItem.get(), livesInOtherList);
         ASSERT(removedIndex != -1);
 
         if (!indexToModify)
diff --git a/WebCore/svg/properties/SVGPropertyTearOff.h b/WebCore/svg/properties/SVGPropertyTearOff.h
index 17588b4..25b8fa2 100644
--- a/WebCore/svg/properties/SVGPropertyTearOff.h
+++ b/WebCore/svg/properties/SVGPropertyTearOff.h
@@ -49,8 +49,6 @@ public:
     PropertyType& propertyReference() { return *m_value; }
     SVGAnimatedProperty* animatedProperty() const { return m_animatedProperty.get(); }
 
-    virtual int removeItemFromList(SVGAnimatedProperty*) { return -1; }
-
     // Used only by the list tear offs!
     void setValue(PropertyType& value)
     {
@@ -81,6 +79,7 @@ public:
         ASSERT(!m_valueIsCopy);
         m_value = new PropertyType(*m_value);
         m_valueIsCopy = true;
+        m_animatedProperty = 0;
     }
 
     void commitChange()

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list