- Revision
- 223210
- Author
- [email protected]
- Date
- 2017-10-11 14:33:58 -0700 (Wed, 11 Oct 2017)
Log Message
Avoid triggering layout from style change
https://bugs.webkit.org/show_bug.cgi?id=178184
Source/WebCore:
rdar://problem/34699113
Reviewed by Zalan Bujtas.
It's bad for RenderBox::styleDidChange() to scroll RenderLayers, because that
can trigger layout via FrameView::updateWidgetPositions() and ScrollingCoordinator::absoluteEventTrackingRegions().
So postpone the scrolling until after layout.
Test: fast/scrolling/adjust-scroll-offset-on-zoom.html
* rendering/RenderBox.cpp:
(WebCore::RenderBox::styleDidChange):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateLayerPositions):
(WebCore::RenderLayer::setPostLayoutScrollPosition):
(WebCore::RenderLayer::applyPostLayoutScrollPositionIfNeeded):
* rendering/RenderLayer.h:
LayoutTests:
Reviewed by Zalan Bujtas.
* fast/scrolling/adjust-scroll-offset-on-zoom-expected.txt: Added.
* fast/scrolling/adjust-scroll-offset-on-zoom.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (223209 => 223210)
--- trunk/LayoutTests/ChangeLog 2017-10-11 20:54:56 UTC (rev 223209)
+++ trunk/LayoutTests/ChangeLog 2017-10-11 21:33:58 UTC (rev 223210)
@@ -1,3 +1,13 @@
+2017-10-11 Simon Fraser <[email protected]>
+
+ Avoid triggering layout from style change
+ https://bugs.webkit.org/show_bug.cgi?id=178184
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/scrolling/adjust-scroll-offset-on-zoom-expected.txt: Added.
+ * fast/scrolling/adjust-scroll-offset-on-zoom.html: Added.
+
2017-10-11 Ryosuke Niwa <[email protected]>
Sanitize URL in pasteboard for other applications and cross origin content
Added: trunk/LayoutTests/fast/scrolling/adjust-scroll-offset-on-zoom-expected.txt (0 => 223210)
--- trunk/LayoutTests/fast/scrolling/adjust-scroll-offset-on-zoom-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/adjust-scroll-offset-on-zoom-expected.txt 2017-10-11 21:33:58 UTC (rev 223210)
@@ -0,0 +1,4 @@
+This test should not crash.
+
+
+
Added: trunk/LayoutTests/fast/scrolling/adjust-scroll-offset-on-zoom.html (0 => 223210)
--- trunk/LayoutTests/fast/scrolling/adjust-scroll-offset-on-zoom.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/adjust-scroll-offset-on-zoom.html 2017-10-11 21:33:58 UTC (rev 223210)
@@ -0,0 +1,36 @@
+<html>
+<head>
+<script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ function appendMarquee()
+ {
+ document.getElementById("a").appendChild(b);
+ }
+
+ function onSVGLoad()
+ {
+ var frameset = document.createElement("frameset");
+ frameset._onresize_ = frameset.onload;
+
+ var svg = document.getElementById("svg");
+ svg.currentScale = 200;
+
+ setTimeout(function() {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ }
+</script>
+</head>
+<body _onload_="appendMarquee()">
+ <p>This test should not crash.</p>
+<iframe></iframe>
+<div id="a" _onwheel_="b()"><marquee id="b"></marquee></div>
+<input autofocus>
+<svg id="svg" _onload_="onSVGLoad()">
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (223209 => 223210)
--- trunk/Source/WebCore/ChangeLog 2017-10-11 20:54:56 UTC (rev 223209)
+++ trunk/Source/WebCore/ChangeLog 2017-10-11 21:33:58 UTC (rev 223210)
@@ -1,3 +1,25 @@
+2017-10-11 Simon Fraser <[email protected]>
+
+ Avoid triggering layout from style change
+ https://bugs.webkit.org/show_bug.cgi?id=178184
+ rdar://problem/34699113
+
+ Reviewed by Zalan Bujtas.
+
+ It's bad for RenderBox::styleDidChange() to scroll RenderLayers, because that
+ can trigger layout via FrameView::updateWidgetPositions() and ScrollingCoordinator::absoluteEventTrackingRegions().
+ So postpone the scrolling until after layout.
+
+ Test: fast/scrolling/adjust-scroll-offset-on-zoom.html
+
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::styleDidChange):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::updateLayerPositions):
+ (WebCore::RenderLayer::setPostLayoutScrollPosition):
+ (WebCore::RenderLayer::applyPostLayoutScrollPositionIfNeeded):
+ * rendering/RenderLayer.h:
+
2017-10-11 Youenn Fablet <[email protected]>
Bump default cache storage quota to 20MB
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (223209 => 223210)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2017-10-11 20:54:56 UTC (rev 223209)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2017-10-11 21:33:58 UTC (rev 223210)
@@ -320,14 +320,10 @@
// If our zoom factor changes and we have a defined scrollLeft/Top, we need to adjust that value into the
// new zoomed coordinate space.
if (hasOverflowClip() && layer() && oldStyle && oldStyle->effectiveZoom() != newStyle.effectiveZoom()) {
- if (int left = layer()->scrollOffset().x()) {
- left = (left / oldStyle->effectiveZoom()) * newStyle.effectiveZoom();
- layer()->scrollToXOffset(left);
- }
- if (int top = layer()->scrollOffset().y()) {
- top = (top / oldStyle->effectiveZoom()) * newStyle.effectiveZoom();
- layer()->scrollToYOffset(top);
- }
+ ScrollPosition scrollPosition = layer()->scrollPosition();
+ float zoomScaleFactor = newStyle.effectiveZoom() / oldStyle->effectiveZoom();
+ scrollPosition.scale(zoomScaleFactor);
+ layer()->setPostLayoutScrollPosition(scrollPosition);
}
// Our opaqueness might have changed without triggering layout.
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (223209 => 223210)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2017-10-11 20:54:56 UTC (rev 223209)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2017-10-11 21:33:58 UTC (rev 223210)
@@ -484,6 +484,9 @@
updateLayerPosition(); // For relpositioned layers or non-positioned layers,
// we need to keep in sync, since we may have shifted relative
// to our parent layer.
+
+ applyPostLayoutScrollPositionIfNeeded();
+
if (geometryMap)
geometryMap->pushMappingsToAncestor(this, parent());
@@ -2305,6 +2308,20 @@
}
}
+void RenderLayer::setPostLayoutScrollPosition(std::optional<ScrollPosition> position)
+{
+ m_postLayoutScrollPosition = position;
+}
+
+void RenderLayer::applyPostLayoutScrollPositionIfNeeded()
+{
+ if (!m_postLayoutScrollPosition)
+ return;
+
+ scrollToOffset(scrollOffsetFromPosition(m_postLayoutScrollPosition.value()), ScrollOffsetClamped);
+ m_postLayoutScrollPosition = std::nullopt;
+}
+
void RenderLayer::scrollToXPosition(int x, ScrollOffsetClamping clamp)
{
ScrollPosition position(x, m_scrollPosition.y());
Modified: trunk/Source/WebCore/rendering/RenderLayer.h (223209 => 223210)
--- trunk/Source/WebCore/rendering/RenderLayer.h 2017-10-11 20:54:56 UTC (rev 223209)
+++ trunk/Source/WebCore/rendering/RenderLayer.h 2017-10-11 21:33:58 UTC (rev 223210)
@@ -212,6 +212,9 @@
void scrollToXPosition(int x, ScrollOffsetClamping = ScrollOffsetUnclamped);
void scrollToYPosition(int y, ScrollOffsetClamping = ScrollOffsetUnclamped);
+ void setPostLayoutScrollPosition(std::optional<ScrollPosition>);
+ void applyPostLayoutScrollPositionIfNeeded();
+
ScrollOffset scrollOffset() const { return scrollOffsetFromPosition(m_scrollPosition); }
IntSize scrollableContentsSize() const;
@@ -1122,6 +1125,7 @@
IntSize m_layerSize;
ScrollPosition m_scrollPosition;
+ std::optional<ScrollPosition> m_postLayoutScrollPosition;
// The width/height of our scrolled area.
IntSize m_scrollSize;
