Title: [232941] trunk
- Revision
- 232941
- Author
- s...@apple.com
- Date
- 2018-06-18 12:28:33 -0700 (Mon, 18 Jun 2018)
Log Message
Document should not be mutated under SMILTimeContainer::updateAnimations()
https://bugs.webkit.org/show_bug.cgi?id=186658
Reviewed by Simon Fraser.
Source/WebCore:
To update the animation of an SVG <animate> element, we call
SVGAnimateElementBase::resetAnimatedType(). It ensures the pointer m_animator
is valid. If it animates a css property, it calls computeCSSPropertyValue()
which calls resolveStyle() via other calls. resolveStyle() may call delayed
callbacks through the destructor of PostResolutionCallbackDisabler. These
callbacks may fire events. These events may execute JS event handlers.
If one of these event handlers deletes the same SVG <animate> we animate,
we will end up calling SVGAnimateElementBase::resetAnimatedPropertyType()
of the same <animate> element. This function will delete the same m_animator
which resetAnimatedType() still holds and will use later. This code
re-entrance is unexpected and unwanted.
The fix is to disable mutating the DOM while updating the SVG animations.
Test: svg/dom/css-animate-input-foucs-crash.html
* svg/animation/SMILTimeContainer.cpp:
(WebCore::SMILTimeContainer::updateAnimations):
LayoutTests:
* svg/dom/css-animate-input-foucs-crash-expected.txt: Added.
* svg/dom/css-animate-input-foucs-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (232940 => 232941)
--- trunk/LayoutTests/ChangeLog 2018-06-18 19:13:19 UTC (rev 232940)
+++ trunk/LayoutTests/ChangeLog 2018-06-18 19:28:33 UTC (rev 232941)
@@ -1,3 +1,13 @@
+2018-06-18 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ Document should not be mutated under SMILTimeContainer::updateAnimations()
+ https://bugs.webkit.org/show_bug.cgi?id=186658
+
+ Reviewed by Simon Fraser.
+
+ * svg/dom/css-animate-input-foucs-crash-expected.txt: Added.
+ * svg/dom/css-animate-input-foucs-crash.html: Added.
+
2018-06-18 Wenson Hsieh <wenson_hs...@apple.com>
fast/forms/button-set-display-flex-justifyContent-center.html is failing on macOS Mojave
Added: trunk/LayoutTests/svg/dom/css-animate-input-foucs-crash-expected.txt (0 => 232941)
--- trunk/LayoutTests/svg/dom/css-animate-input-foucs-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/dom/css-animate-input-foucs-crash-expected.txt 2018-06-18 19:28:33 UTC (rev 232941)
@@ -0,0 +1,4 @@
+This test passes if it doesn't crash.
+
+
+
Added: trunk/LayoutTests/svg/dom/css-animate-input-foucs-crash.html (0 => 232941)
--- trunk/LayoutTests/svg/dom/css-animate-input-foucs-crash.html (rev 0)
+++ trunk/LayoutTests/svg/dom/css-animate-input-foucs-crash.html 2018-06-18 19:28:33 UTC (rev 232941)
@@ -0,0 +1,23 @@
+<body>
+ <p>This test passes if it doesn't crash.</p>
+ <svg id="svgRoot">
+ <animate attributeName="fill" />
+ </svg>
+ <div id="inputParent" _onfocusin_="onFoucsIn()">
+ <keygen id="input">
+ </div>
+ <details _ontoggle_="onToggle()" open="true"></details>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ function onFoucsIn() {
+ svgRoot.remove();
+ }
+
+ function onToggle() {
+ input.autofocus = true;
+ inputParent.after(inputParent);
+ }
+ </script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (232940 => 232941)
--- trunk/Source/WebCore/ChangeLog 2018-06-18 19:13:19 UTC (rev 232940)
+++ trunk/Source/WebCore/ChangeLog 2018-06-18 19:28:33 UTC (rev 232941)
@@ -1,3 +1,29 @@
+2018-06-18 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ Document should not be mutated under SMILTimeContainer::updateAnimations()
+ https://bugs.webkit.org/show_bug.cgi?id=186658
+
+ Reviewed by Simon Fraser.
+
+ To update the animation of an SVG <animate> element, we call
+ SVGAnimateElementBase::resetAnimatedType(). It ensures the pointer m_animator
+ is valid. If it animates a css property, it calls computeCSSPropertyValue()
+ which calls resolveStyle() via other calls. resolveStyle() may call delayed
+ callbacks through the destructor of PostResolutionCallbackDisabler. These
+ callbacks may fire events. These events may execute JS event handlers.
+ If one of these event handlers deletes the same SVG <animate> we animate,
+ we will end up calling SVGAnimateElementBase::resetAnimatedPropertyType()
+ of the same <animate> element. This function will delete the same m_animator
+ which resetAnimatedType() still holds and will use later. This code
+ re-entrance is unexpected and unwanted.
+
+ The fix is to disable mutating the DOM while updating the SVG animations.
+
+ Test: svg/dom/css-animate-input-foucs-crash.html
+
+ * svg/animation/SMILTimeContainer.cpp:
+ (WebCore::SMILTimeContainer::updateAnimations):
+
2018-06-18 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r232935.
Modified: trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp (232940 => 232941)
--- trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp 2018-06-18 19:13:19 UTC (rev 232940)
+++ trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp 2018-06-18 19:28:33 UTC (rev 232941)
@@ -31,6 +31,7 @@
#include "Page.h"
#include "SVGSMILElement.h"
#include "SVGSVGElement.h"
+#include "ScopedEventQueue.h"
namespace WebCore {
@@ -254,6 +255,9 @@
{
SMILTime earliestFireTime = SMILTime::unresolved();
+ // Don't mutate the DOM while updating the animations.
+ EventQueueScope scope;
+
#ifndef NDEBUG
// This boolean will catch any attempts to schedule/unschedule scheduledAnimations during this critical section.
// Similarly, any elements removed will unschedule themselves, so this will catch modification of animationsToApply.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
