Changeset 109362 in webkit

Timestamp:

Mar 1, 2012, 10:19:03 AM (15 years ago)

Author:

inferno@chromium.org

Message:

Protect functions using two container node function, each of which can fire mutation events.
​https://bugs.webkit.org/show_bug.cgi?id=78397

Reviewed by Ryosuke Niwa.

Source/WebCore:

Tests: fast/dom/document-set-title-mutation-crash.html

fast/dom/option-text-mutation-crash.html

• dom/Node.cpp:

(WebCore::Node::setTextContent):

• dom/Text.cpp:

(WebCore::Text::replaceWholeText):

• editing/markup.cpp:

(WebCore::trimFragment):
(WebCore::replaceChildrenWithFragment):
(WebCore::replaceChildrenWithText):

• html/HTMLOptionElement.cpp:

(WebCore::HTMLOptionElement::setText):

• html/HTMLScriptElement.cpp:

(WebCore::HTMLScriptElement::setText):

• html/HTMLTableElement.cpp:

(WebCore::HTMLTableElement::insertRow):

• html/HTMLTextAreaElement.cpp:

(WebCore::HTMLTextAreaElement::setDefaultValue):

• html/HTMLTitleElement.cpp:

(WebCore::HTMLTitleElement::setText):

LayoutTests:

• fast/dom/document-set-title-mutation-crash-expected.txt: Added.
• fast/dom/document-set-title-mutation-crash.html: Added.
• fast/dom/option-text-mutation-crash-expected.txt: Added.
• fast/dom/option-text-mutation-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/document-set-title-mutation-crash-expected.txt (added)
• LayoutTests/fast/dom/document-set-title-mutation-crash.html (added)
• LayoutTests/fast/dom/option-text-mutation-crash-expected.txt (added)
• LayoutTests/fast/dom/option-text-mutation-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Node.cpp (modified) (1 diff)
• Source/WebCore/dom/Text.cpp (modified) (1 diff)
• Source/WebCore/editing/markup.cpp (modified) (6 diffs)
• Source/WebCore/html/HTMLOptionElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLScriptElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLTableElement.cpp (modified) (3 diffs)
• Source/WebCore/html/HTMLTextAreaElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLTitleElement.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-03-01  Abhishek Arya  <inferno@chromium.org>
+
+        Protect functions using two container node function, each of which can fire mutation events.
+        https://bugs.webkit.org/show_bug.cgi?id=78397
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/dom/document-set-title-mutation-crash-expected.txt: Added.
+        * fast/dom/document-set-title-mutation-crash.html: Added.
+        * fast/dom/option-text-mutation-crash-expected.txt: Added.
+        * fast/dom/option-text-mutation-crash.html: Added.
+
 2012-03-01  Shinya Kawanaka  <shinyak@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-01  Abhishek Arya  <inferno@chromium.org>
+
+        Protect functions using two container node function, each of which can fire mutation events.
+        https://bugs.webkit.org/show_bug.cgi?id=78397
+
+        Reviewed by Ryosuke Niwa.
+
+        Tests: fast/dom/document-set-title-mutation-crash.html
+               fast/dom/option-text-mutation-crash.html
+
+        * dom/Node.cpp:
+        (WebCore::Node::setTextContent):
+        * dom/Text.cpp:
+        (WebCore::Text::replaceWholeText):
+        * editing/markup.cpp:
+        (WebCore::trimFragment):
+        (WebCore::replaceChildrenWithFragment):
+        (WebCore::replaceChildrenWithText):
+        * html/HTMLOptionElement.cpp:
+        (WebCore::HTMLOptionElement::setText):
+        * html/HTMLScriptElement.cpp:
+        (WebCore::HTMLScriptElement::setText):
+        * html/HTMLTableElement.cpp:
+        (WebCore::HTMLTableElement::insertRow):
+        * html/HTMLTextAreaElement.cpp:
+        (WebCore::HTMLTextAreaElement::setDefaultValue):
+        * html/HTMLTitleElement.cpp:
+        (WebCore::HTMLTitleElement::setText):
+
 2012-03-01  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebCore/dom/Node.cpp

@@ -2060 +2060 @@
         case ENTITY_REFERENCE_NODE:
         case DOCUMENT_FRAGMENT_NODE: {
-            ContainerNode* container = toContainerNode(this);
+            RefPtr<ContainerNode> container = toContainerNode(this);
 #if ENABLE(MUTATION_OBSERVERS)
             ChildListMutationScope mutation(this);

trunk/Source/WebCore/dom/Text.cpp

@@ -148 +148 @@
 
     RefPtr<Text> protectedThis(this); // Mutation event handlers could cause our last ref to go away
-    ContainerNode* parent = parentNode(); // Protect against mutation handlers moving this node during traversal
+    RefPtr<ContainerNode> parent = parentNode(); // Protect against mutation handlers moving this node during traversal
     ExceptionCode ignored = 0;
     for (RefPtr<Node> n = startText; n && n != this && n->isTextNode() && n->parentNode() == parent;) {

trunk/Source/WebCore/editing/markup.cpp

@@ -694 +694 @@
 {
     ExceptionCode ec = 0;
-    Node* next;
+    RefPtr<Node> next;
     for (RefPtr<Node> node = fragment->firstChild(); node; node = next) {
         if (nodeBeforeContext->isDescendantOf(node.get())) {
@@ -708 +708 @@
 
     ASSERT(nodeAfterContext->parentNode());
-    for (Node* node = nodeAfterContext; node; node = next) {
+    for (RefPtr<Node> node = nodeAfterContext; node; node = next) {
         next = node->traverseNextSibling();
-        node->parentNode()->removeChild(node, ec);
+        node->parentNode()->removeChild(node.get(), ec);
         ASSERT(!ec);
     }
@@ -1022 +1022 @@
 }
 
-void replaceChildrenWithFragment(ContainerNode* containerNode, PassRefPtr<DocumentFragment> fragment, ExceptionCode& ec)
-{
+void replaceChildrenWithFragment(ContainerNode* container, PassRefPtr<DocumentFragment> fragment, ExceptionCode& ec)
+{
+    RefPtr<ContainerNode> containerNode(container);
+
 #if ENABLE(MUTATION_OBSERVERS)
-    ChildListMutationScope mutation(containerNode);
+    ChildListMutationScope mutation(containerNode.get());
 #endif
 
@@ -1033 +1035 @@
     }
 
-    if (hasOneTextChild(containerNode) && hasOneTextChild(fragment.get())) {
+    if (hasOneTextChild(containerNode.get()) && hasOneTextChild(fragment.get())) {
         toText(containerNode->firstChild())->setData(toText(fragment->firstChild())->data(), ec);
         return;
     }
 
-    if (hasOneChild(containerNode)) {
+    if (hasOneChild(containerNode.get())) {
         containerNode->replaceChild(fragment, containerNode->firstChild(), ec);
         return;
@@ -1047 +1049 @@
 }
 
-void replaceChildrenWithText(ContainerNode* containerNode, const String& text, ExceptionCode& ec)
-{
+void replaceChildrenWithText(ContainerNode* container, const String& text, ExceptionCode& ec)
+{
+    RefPtr<ContainerNode> containerNode(container);
+
 #if ENABLE(MUTATION_OBSERVERS)
-    ChildListMutationScope mutation(containerNode);
+    ChildListMutationScope mutation(containerNode.get());
 #endif
 
-    if (hasOneTextChild(containerNode)) {
+    if (hasOneTextChild(containerNode.get())) {
         toText(containerNode->firstChild())->setData(text, ec);
         return;
@@ -1060 +1064 @@
     RefPtr<Text> textNode = Text::create(containerNode->document(), text);
 
-    if (hasOneChild(containerNode)) {
+    if (hasOneChild(containerNode.get())) {
         containerNode->replaceChild(textNode.release(), containerNode->firstChild(), ec);
         return;

trunk/Source/WebCore/html/HTMLOptionElement.cpp

@@ -138 +138 @@
 void HTMLOptionElement::setText(const String &text, ExceptionCode& ec)
 {
+    RefPtr<Node> protectFromMutationEvents(this);
+
     // Changing the text causes a recalc of a select's items, which will reset the selected
     // index to the first item if the select is single selection with a menu list. We attempt to
     // preserve the selected item.
-    HTMLSelectElement* select = ownerSelectElement();
+    RefPtr<HTMLSelectElement> select = ownerSelectElement();
     bool selectIsMenuList = select && select->usesMenuList();
     int oldSelectedIndex = selectIsMenuList ? select->selectedIndex() : -1;

trunk/Source/WebCore/html/HTMLScriptElement.cpp

@@ -83 +83 @@
 void HTMLScriptElement::setText(const String &value)
 {
+    RefPtr<Node> protectFromMutationEvents(this);
+
     ExceptionCode ec = 0;
     int numChildren = childNodeCount();

trunk/Source/WebCore/html/HTMLTableElement.cpp

@@ -188 +188 @@
     }
 
-    HTMLTableRowElement* lastRow = 0;
-    HTMLTableRowElement* row = 0;
+    RefPtr<Node> protectFromMutationEvents(this);
+
+    RefPtr<HTMLTableRowElement> lastRow = 0;
+    RefPtr<HTMLTableRowElement> row = 0;
     if (index == -1)
         lastRow = HTMLTableRowsCollection::lastRow(this);
     else {
         for (int i = 0; i <= index; ++i) {
-            row = HTMLTableRowsCollection::rowAfter(this, lastRow);
+            row = HTMLTableRowsCollection::rowAfter(this, lastRow.get());
             if (!row) {
                 if (i != index) {
@@ -206 +208 @@
     }
 
-    ContainerNode* parent;
+    RefPtr<ContainerNode> parent;
     if (lastRow)
         parent = row ? row->parentNode() : lastRow->parentNode();
@@ -221 +223 @@
 
     RefPtr<HTMLTableRowElement> newRow = HTMLTableRowElement::create(document());
-    parent->insertBefore(newRow, row, ec);
+    parent->insertBefore(newRow, row.get(), ec);
     return newRow.release();
 }

trunk/Source/WebCore/html/HTMLTextAreaElement.cpp

@@ -385 +385 @@
 void HTMLTextAreaElement::setDefaultValue(const String& defaultValue)
 {
+    RefPtr<Node> protectFromMutationEvents(this);
+
     // To preserve comments, remove only the text nodes, then add a single text node.
-
     Vector<RefPtr<Node> > textNodes;
     for (Node* n = firstChild(); n; n = n->nextSibling()) {

trunk/Source/WebCore/html/HTMLTitleElement.cpp

@@ -89 +89 @@
 void HTMLTitleElement::setText(const String &value)
 {
+    RefPtr<Node> protectFromMutationEvents(this);
+
     ExceptionCode ec = 0;
     int numChildren = childNodeCount();
@@ -94 +96 @@
     if (numChildren == 1 && firstChild()->isTextNode())
         toText(firstChild())->setData(value, ec);
-    else {  
+    else {
         // We make a copy here because entity of "value" argument can be Document::m_title,
         // which goes empty during removeChildren() invocation below,