Merge r188014 - Crash when removing children of a MathMLSelectElement

https://bugs.webkit.org/show_bug.cgi?id=147704 <rdar://problem/21940321> Reviewed by Ryosuke Niwa. Source/WebCore: When MathMLSelectElement::childrenChanged() is called after its children have been removed, MathMLSelectElement calls updateSelectedChild() which accesses m_selectedChild. However, in this case, m_selectedChild is the previously selected child and it may be destroyed as this point if it was removed. To avoid this problem, MathMLSelectElement now keep a strong ref to the currently selected element. Test: mathml/maction-removeChild.html * mathml/MathMLSelectElement.h: LayoutTests: Add layout test that reproduces the crash under guardmalloc. * mathml/maction-removeChild-expected.txt: Added. * mathml/maction-removeChild.html: Added.
WebKit · Aug 6, 2015 · 21faa8d · 21faa8d
1 parent 79e1434
commit 21faa8d
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 1 deletion.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2015-08-05  Chris Dumez  <cdumez@apple.com>
+
+        Crash when removing children of a MathMLSelectElement
+        https://bugs.webkit.org/show_bug.cgi?id=147704
+        <rdar://problem/21940321>
+
+        Reviewed by Ryosuke Niwa.
+
+        Add layout test that reproduces the crash under guardmalloc.
+
+        * mathml/maction-removeChild-expected.txt: Added.
+        * mathml/maction-removeChild.html: Added.
+
 2015-07-28  Simon Fraser  <simon.fraser@apple.com>
 
         Animations sometimes fail to start

diff --git a/LayoutTests/mathml/maction-removeChild-expected.txt b/LayoutTests/mathml/maction-removeChild-expected.txt
@@ -0,0 +1,3 @@
+This test passes if it does not crash
+
+
diff --git a/LayoutTests/mathml/maction-removeChild.html b/LayoutTests/mathml/maction-removeChild.html
@@ -0,0 +1,19 @@
+<!doctype html>
+<html>
+  <body>
+    <p>This test passes if it does not crash</p>
+    <math>
+      <maction id="testSelect" actiontype="toggle" selection="2">
+        <mi>g</mi>
+        <mspace/>
+      </maction>
+    </math>
+    <script>
+      if (window.testRunner)
+        testRunner.dumpAsText();
+
+      var testSelect = document.getElementById("testSelect");
+      testSelect.innerHTML = "123.123.123";
+    </script>
+  </body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2015-08-05  Chris Dumez  <cdumez@apple.com>
+
+        Crash when removing children of a MathMLSelectElement
+        https://bugs.webkit.org/show_bug.cgi?id=147704
+        <rdar://problem/21940321>
+
+        Reviewed by Ryosuke Niwa.
+
+        When MathMLSelectElement::childrenChanged() is called after its
+        children have been removed, MathMLSelectElement calls
+        updateSelectedChild() which accesses m_selectedChild. However,
+        in this case, m_selectedChild is the previously selected child
+        and it may be destroyed as this point if it was removed. To avoid
+        this problem, MathMLSelectElement now keep a strong ref to the
+        currently selected element.
+
+        Test: mathml/maction-removeChild.html
+
+        * mathml/MathMLSelectElement.h:
+
 2015-08-03  Brady Eidson  <beidson@apple.com>
 
         Crash when signing into twitter calling WebCore::DocumentLoader::responseReceived(WebCore::CachedResource*, WebCore::ResourceResponse const&).

diff --git a/Source/WebCore/mathml/MathMLSelectElement.h b/Source/WebCore/mathml/MathMLSelectElement.h
@@ -56,7 +56,7 @@ class MathMLSelectElement final : public MathMLInlineContainerElement {
     Element* getSelectedSemanticsChild();
 
     void updateSelectedChild() override;
-    Element* m_selectedChild;
+    RefPtr<Element> m_selectedChild;
 };
 
 }