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

inferno at chromium.org inferno at chromium.org
Wed Dec 22 18:26:00 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 65374915f9b2a517113d48ad5a30cbc309a9f6eb
Author: inferno at chromium.org <inferno at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Dec 10 21:42:36 2010 +0000

    2010-12-10  Emil Eklund  <eae at chromium.org>
    
            Reviewed by Adam Barth.
    
            Fix crash in Range::processContents when modified during mutation event.
            https://bugs.webkit.org/show_bug.cgi?id=50710
    
            Test: fast/dom/Range/range-extractContents.html
    
            * dom/Range.cpp:
            (WebCore::Range::processContents):
            Replace raw pointers with RefPtrs and add checks.
    2010-12-10  Emil Eklund  <eae at chromium.org>
    
            Reviewed by Adam Barth.
    
            Add testcase for range.extractContents crash.
            https://bugs.webkit.org/show_bug.cgi?id=50710
    
            * fast/dom/Range/range-extractContents-expected.txt: Added.
            * fast/dom/Range/range-extractContents.html: Added.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@73799 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 2cdc61e..fcb8a4e 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2010-12-10  Emil Eklund  <eae at chromium.org>
+
+        Reviewed by Adam Barth.
+
+        Add testcase for range.extractContents crash.
+        https://bugs.webkit.org/show_bug.cgi?id=50710
+
+        * fast/dom/Range/range-extractContents-expected.txt: Added.
+        * fast/dom/Range/range-extractContents.html: Added.
+
 2010-12-10  Peter Kasting  <pkasting at google.com>
 
         Unreviewed Chromium test expectations update.
diff --git a/LayoutTests/fast/dom/Range/range-extractContents-expected.txt b/LayoutTests/fast/dom/Range/range-extractContents-expected.txt
new file mode 100644
index 0000000..9ceb737
--- /dev/null
+++ b/LayoutTests/fast/dom/Range/range-extractContents-expected.txt
@@ -0,0 +1 @@
+PASS: No crash.
diff --git a/LayoutTests/fast/dom/Range/range-extractContents.html b/LayoutTests/fast/dom/Range/range-extractContents.html
new file mode 100644
index 0000000..f10ae50
--- /dev/null
+++ b/LayoutTests/fast/dom/Range/range-extractContents.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script type="text/javascript">
+        function log(msg)
+        {
+            document.body.appendChild(document.createTextNode(msg + '\n'));
+        }
+
+        function runTests()
+        {
+            if (window.layoutTestController)
+                layoutTestController.dumpAsText();
+
+            document.addEventListener("DOMSubtreeModified", function() {
+                document.getElementById('cont').innerHTML = '';
+            }, false);
+
+            var r = document.createRange();
+            try {
+                r.setStartBefore(document.getElementById('start'));
+                r.setEndAfter(document.getElementById('end'));
+                var fragment = r.extractContents();
+                document.body.appendChild(fragment);
+            } catch(e) {
+            }
+            log('PASS: No crash.');
+        }
+
+    </script>
+</head>
+<body onload="runTests();">
+    <p id="cont">
+        <span>This <span id="start">tests</span></span>
+        <span>that we don't crash when mutating the dom during</span>
+        <span>an <code id="end">extractContents</code> call.</span>
+    </p>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 85a60a1..b275e7a 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2010-12-10  Emil Eklund  <eae at chromium.org>
+
+        Reviewed by Adam Barth.
+
+        Fix crash in Range::processContents when modified during mutation event.
+        https://bugs.webkit.org/show_bug.cgi?id=50710
+
+        Test: fast/dom/Range/range-extractContents.html
+
+        * dom/Range.cpp:
+        (WebCore::Range::processContents):
+        Replace raw pointers with RefPtrs and add checks.
+
 2010-12-09  Enrica Casucci  <enrica at apple.com>
 
         Reviewed by Alexey Proskuryakov.
diff --git a/WebCore/dom/Range.cpp b/WebCore/dom/Range.cpp
index 96acf8e..cf05b9b 100644
--- a/WebCore/dom/Range.cpp
+++ b/WebCore/dom/Range.cpp
@@ -41,9 +41,12 @@
 #include <stdio.h>
 #include <wtf/text/CString.h>
 #include <wtf/RefCountedLeakCounter.h>
+#include <wtf/Vector.h>
 
 namespace WebCore {
 
+typedef Vector<RefPtr<Node> > NodeVector;
+
 using namespace std;
 
 #ifndef NDEBUG
@@ -595,9 +598,6 @@ bool Range::intersectsNode(Node* refNode, ExceptionCode& ec)
 
 PassRefPtr<DocumentFragment> Range::processContents(ActionType action, ExceptionCode& ec)
 {
-    // FIXME: To work properly with mutation events, we will have to take into account
-    // situations where the tree is being transformed while we work on it - ugh!
-
     RefPtr<DocumentFragment> fragment;
     if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS)
         fragment = DocumentFragment::create(m_ownerDocument.get());
@@ -655,21 +655,20 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
                 pi->setData(data, ec);
             }
         } else {
-            Node* n = m_start.container()->firstChild();
+            RefPtr<Node> n = m_start.container()->firstChild();
             int i;
             for (i = 0; n && i < m_start.offset(); i++) // skip until start offset
                 n = n->nextSibling();
             int endOffset = m_end.offset();
-            while (n && i < endOffset) { // delete until end offset
-                Node* next = n->nextSibling();
+            RefPtr<Node> next;
+            for (; n && i < endOffset; n = next, i++) { // delete until end offset
+                next = n->nextSibling();
                 if (action == EXTRACT_CONTENTS)
                     fragment->appendChild(n, ec); // will remove n from its parent
                 else if (action == CLONE_CONTENTS)
                     fragment->appendChild(n->cloneNode(true), ec);
                 else
-                    toContainerNode(m_start.container())->removeChild(n, ec);
-                n = next;
-                i++;
+                    toContainerNode(m_start.container())->removeChild(n.get(), ec);
             }
         }
         return fragment.release();
@@ -720,39 +719,47 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
         } else {
             if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS)
                 leftContents = m_start.container()->cloneNode(false);
+            NodeVector nodes;
             Node* n = m_start.container()->firstChild();
-            for (int i = 0; n && i < m_start.offset(); i++) // skip until start offset
-                n = n->nextSibling();
-            while (n) { // process until end
-                Node* next = n->nextSibling();
+            for (int i = 0; n; n = n->nextSibling(), i++) {
+                if (i < m_start.offset())
+                    continue; // Skip until start offset.
+                nodes.append(n);
+            }
+            for (NodeVector::const_iterator it = nodes.begin(); it != nodes.end(); it++) {
+                Node* n = it->get();
                 if (action == EXTRACT_CONTENTS)
-                    leftContents->appendChild(n, ec); // will remove n from start container
+                    leftContents->appendChild(n, ec); // Will remove n from start container.
                 else if (action == CLONE_CONTENTS)
                     leftContents->appendChild(n->cloneNode(true), ec);
                 else
                     toContainerNode(m_start.container())->removeChild(n, ec);
-                n = next;
             }
         }
 
-        ContainerNode* leftParent = m_start.container()->parentNode();
-        Node* n = m_start.container()->nextSibling();
-        for (; leftParent != commonRoot; leftParent = leftParent->parentNode()) {
+        NodeVector ancestorNodes;
+        for (ContainerNode* n = m_start.container()->parentNode(); n && n != commonRoot; n = n->parentNode())
+            ancestorNodes.append(n);
+        RefPtr<Node> n = m_start.container()->nextSibling();
+        for (NodeVector::const_iterator it = ancestorNodes.begin(); it != ancestorNodes.end(); it++) {
+            Node* leftParent = it->get();
             if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
                 RefPtr<Node> leftContentsParent = leftParent->cloneNode(false);
-                leftContentsParent->appendChild(leftContents, ec);
-                leftContents = leftContentsParent;
+                if (leftContentsParent) { // Might have been removed already during mutation event.
+                    leftContentsParent->appendChild(leftContents, ec);
+                    leftContents = leftContentsParent;
+                }
             }
 
-            Node* next;
+            RefPtr<Node> next;
             for (; n; n = next) {
                 next = n->nextSibling();
                 if (action == EXTRACT_CONTENTS)
-                    leftContents->appendChild(n, ec); // will remove n from leftParent
+                    leftContents->appendChild(n.get(), ec); // will remove n from leftParent
                 else if (action == CLONE_CONTENTS)
                     leftContents->appendChild(n->cloneNode(true), ec);
                 else
-                    leftParent->removeChild(n, ec);
+                    leftParent->removeChild(n.get(), ec);
             }
             n = leftParent->nextSibling();
         }
@@ -786,15 +793,12 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
                 rightContents = m_end.container()->cloneNode(false);
             Node* n = m_end.container()->firstChild();
             if (n && m_end.offset()) {
-                for (int i = 0; i + 1 < m_end.offset(); i++) { // skip to end.offset()
-                    Node* next = n->nextSibling();
-                    if (!next)
-                        break;
-                    n = next;
+                NodeVector nodes;
+                for (int i = 0; i + 1 < m_end.offset() && n; i++, n = n->nextSibling()) {
+                    nodes.append(n);
                 }
-                Node* prev;
-                for (; n; n = prev) {
-                    prev = n->previousSibling();
+                for (int i = nodes.size() - 1; i >= 0; i--) {
+                    Node* n = nodes[i].get();
                     if (action == EXTRACT_CONTENTS)
                         rightContents->insertBefore(n, rightContents->firstChild(), ec); // will remove n from its parent
                     else if (action == CLONE_CONTENTS)
@@ -868,11 +872,12 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
     if ((action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) && leftContents)
         fragment->appendChild(leftContents, ec);
 
-    Node* next;
-    Node* n;
     if (processStart) {
-        for (n = processStart; n && n != processEnd; n = next) {
-            next = n->nextSibling();
+        NodeVector nodes;
+        for (Node* n = processStart; n && n != processEnd; n = n->nextSibling())
+            nodes.append(n);
+        for (NodeVector::const_iterator it = nodes.begin(); it != nodes.end(); it++) {
+            Node* n = it->get();
             if (action == EXTRACT_CONTENTS)
                 fragment->appendChild(n, ec); // will remove from commonRoot
             else if (action == CLONE_CONTENTS)

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list