- Revision
- 226491
- Author
- [email protected]
- Date
- 2018-01-06 21:48:47 -0800 (Sat, 06 Jan 2018)
Log Message
Crash under RenderLayer::scrollTo() with marquee
https://bugs.webkit.org/show_bug.cgi?id=181349
rdar://problem/36190168
Reviewed by Zalan Bujtas.
Source/WebCore:
Don't call updateWidgetPositions() synchonously during RenderLayer scrolling, because it
can run arbitrary script which may trigger destruction of this RenderLayer.
Instead, queue up updateWidgetPositions() on a zero-delay timer.
Under some circumstances this may allow a paint to occur before the widgets have been
updated (which could be fixed with a more invasive change), but in practice I saw no
painting issues with plug-ins or iframes inside overflow scroll, in WebKit or LegacyWebKit.
Test: fast/scrolling/marquee-scroll-crash.html
* page/FrameView.cpp:
(WebCore::FrameView::FrameView):
(WebCore::FrameView::updateWidgetPositions):
(WebCore::FrameView::scheduleUpdateWidgetPositions):
(WebCore::FrameView::updateWidgetPositionsTimerFired):
* page/FrameView.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollTo):
LayoutTests:
* fast/scrolling/marquee-scroll-crash-expected.txt: Added.
* fast/scrolling/marquee-scroll-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (226490 => 226491)
--- trunk/LayoutTests/ChangeLog 2018-01-07 05:18:47 UTC (rev 226490)
+++ trunk/LayoutTests/ChangeLog 2018-01-07 05:48:47 UTC (rev 226491)
@@ -1,3 +1,14 @@
+2018-01-06 Simon Fraser <[email protected]>
+
+ Crash under RenderLayer::scrollTo() with marquee
+ https://bugs.webkit.org/show_bug.cgi?id=181349
+ rdar://problem/36190168
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/scrolling/marquee-scroll-crash-expected.txt: Added.
+ * fast/scrolling/marquee-scroll-crash.html: Added.
+
2018-01-05 Dean Jackson <[email protected]>
Accurately clip copyTexImage2D and copyTexSubImage2D
Added: trunk/LayoutTests/fast/scrolling/marquee-scroll-crash-expected.txt (0 => 226491)
--- trunk/LayoutTests/fast/scrolling/marquee-scroll-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/marquee-scroll-crash-expected.txt 2018-01-07 05:48:47 UTC (rev 226491)
@@ -0,0 +1 @@
+Test passes if it does not crash.
Added: trunk/LayoutTests/fast/scrolling/marquee-scroll-crash.html (0 => 226491)
--- trunk/LayoutTests/fast/scrolling/marquee-scroll-crash.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/marquee-scroll-crash.html 2018-01-07 05:48:47 UTC (rev 226491)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ function testcase()
+ {
+ var iframe = document.getElementById('iframe');
+ var frameDocElement = iframe.contentDocument.documentElement;
+ frameDocElement.innerHTML = '<object>cc</object>';
+ frameDocElement.addEventListener('beforeload', frameBeforeLoad, true);
+ window.getSelection().selectAllChildren(document.body);
+ }
+
+ function frameBeforeLoad(event)
+ {
+ document.write('Test passes if it does not crash.');
+ }
+ </script>
+ </head>
+ <body _onload_='testcase();'>
+ <iframe id='iframe'></iframe>
+ <marquee>marquee</marquee>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (226490 => 226491)
--- trunk/Source/WebCore/ChangeLog 2018-01-07 05:18:47 UTC (rev 226490)
+++ trunk/Source/WebCore/ChangeLog 2018-01-07 05:48:47 UTC (rev 226491)
@@ -1,3 +1,31 @@
+2018-01-06 Simon Fraser <[email protected]>
+
+ Crash under RenderLayer::scrollTo() with marquee
+ https://bugs.webkit.org/show_bug.cgi?id=181349
+ rdar://problem/36190168
+
+ Reviewed by Zalan Bujtas.
+
+ Don't call updateWidgetPositions() synchonously during RenderLayer scrolling, because it
+ can run arbitrary script which may trigger destruction of this RenderLayer.
+
+ Instead, queue up updateWidgetPositions() on a zero-delay timer.
+
+ Under some circumstances this may allow a paint to occur before the widgets have been
+ updated (which could be fixed with a more invasive change), but in practice I saw no
+ painting issues with plug-ins or iframes inside overflow scroll, in WebKit or LegacyWebKit.
+
+ Test: fast/scrolling/marquee-scroll-crash.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::FrameView):
+ (WebCore::FrameView::updateWidgetPositions):
+ (WebCore::FrameView::scheduleUpdateWidgetPositions):
+ (WebCore::FrameView::updateWidgetPositionsTimerFired):
+ * page/FrameView.h:
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::scrollTo):
+
2018-01-05 Dean Jackson <[email protected]>
Accurately clip copyTexImage2D and copyTexSubImage2D
Modified: trunk/Source/WebCore/page/FrameView.cpp (226490 => 226491)
--- trunk/Source/WebCore/page/FrameView.cpp 2018-01-07 05:18:47 UTC (rev 226490)
+++ trunk/Source/WebCore/page/FrameView.cpp 2018-01-07 05:48:47 UTC (rev 226491)
@@ -171,6 +171,7 @@
: m_frame(frame)
, m_canHaveScrollbars(true)
, m_updateEmbeddedObjectsTimer(*this, &FrameView::updateEmbeddedObjectsTimerFired)
+ , m_updateWidgetPositionsTimer(*this, &FrameView::updateWidgetPositionsTimerFired)
, m_isTransparent(false)
, m_baseBackgroundColor(Color::white)
, m_mediaType("screen")
@@ -4908,6 +4909,7 @@
void FrameView::updateWidgetPositions()
{
+ m_updateWidgetPositionsTimer.stop();
// updateWidgetPosition() can possibly cause layout to be re-entered (via plug-ins running
// scripts in response to NPP_SetWindow, for example), so we need to keep the Widgets
// alive during enumeration.
@@ -4919,6 +4921,17 @@
}
}
+void FrameView::scheduleUpdateWidgetPositions()
+{
+ if (!m_updateWidgetPositionsTimer.isActive())
+ m_updateWidgetPositionsTimer.startOneShot(0_s);
+}
+
+void FrameView::updateWidgetPositionsTimerFired()
+{
+ updateWidgetPositions();
+}
+
void FrameView::notifyWidgets(WidgetNotification notification)
{
for (auto& widget : collectAndProtectWidgets(m_widgetsInRenderTree))
Modified: trunk/Source/WebCore/page/FrameView.h (226490 => 226491)
--- trunk/Source/WebCore/page/FrameView.h 2018-01-07 05:18:47 UTC (rev 226490)
+++ trunk/Source/WebCore/page/FrameView.h 2018-01-07 05:48:47 UTC (rev 226491)
@@ -595,6 +595,8 @@
void setHasFlippedBlockRenderers(bool b) { m_hasFlippedBlockRenderers = b; }
void updateWidgetPositions();
+ void scheduleUpdateWidgetPositions();
+
void didAddWidgetToRenderTree(Widget&);
void willRemoveWidgetFromRenderTree(Widget&);
@@ -742,6 +744,9 @@
void updateEmbeddedObjectsTimerFired();
bool updateEmbeddedObjects();
void updateEmbeddedObject(RenderEmbeddedObject&);
+
+ void updateWidgetPositionsTimerFired();
+
void scrollToAnchor();
void scrollPositionChanged(const ScrollPosition& oldPosition, const ScrollPosition& newPosition);
void scrollableAreaSetChanged();
@@ -792,6 +797,8 @@
bool m_contentIsOpaque;
Timer m_updateEmbeddedObjectsTimer;
+ Timer m_updateWidgetPositionsTimer;
+
bool m_firstLayoutCallbackPending;
bool m_isTransparent;
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (226490 => 226491)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2018-01-07 05:18:47 UTC (rev 226490)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2018-01-07 05:48:47 UTC (rev 226491)
@@ -2461,7 +2461,7 @@
#if ENABLE(DASHBOARD_SUPPORT)
view.frameView().updateAnnotatedRegions();
#endif
- view.frameView().updateWidgetPositions();
+ view.frameView().scheduleUpdateWidgetPositions();
if (!m_updatingMarqueePosition) {
// Avoid updating compositing layers if, higher on the stack, we're already updating layer
