- Revision
- 258336
- Author
- [email protected]
- Date
- 2020-03-12 10:09:34 -0700 (Thu, 12 Mar 2020)
Log Message
REGRESSION (r254054): finance.google.com watch list renders initially then disappears for 5+ seconds before reappearing
https://bugs.webkit.org/show_bug.cgi?id=208972
<rdar://problem/59727171>
Reviewed by Zalan Bujtas.
Source/WebCore:
After r254054 we could get a style change in which opacity was unchanged, but
hasAutoUsedZIndex() in the style changed (because Adjuster::adjustAnimatedStyle() can set it).
In this case we failed to trigger layout, which means that we failed to recompute visual
overflow when a layer changed from being self-painting to non-self-painting (which affects
visual overflow computation and has hasAutoUsedZIndex() as input). We'd thus fail to paint some
renderers because their visual overflow didn't intersect the paint dirty rect.
Fix by having RenderStyle::changeRequiresLayout() return true if hasAutoUsedZIndex() differs
between the styles. This has minimal performance impact; rareNonInheritedDataChangeRequiresLayout()
already returns true if opacity, filters and other stacking-context-affecting properties change.
Test: fast/overflow/animation-recompute-overflow.html
* rendering/RenderBox.cpp:
(WebCore::RenderBox::addOverflowFromChild):
(WebCore::RenderBox::addLayoutOverflow):
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::changeRequiresLayout const):
LayoutTests:
Ref test, and some rebaselines where repaint order changed.
* css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:
* fast/overflow/animation-recompute-overflow-expected.html: Added.
* fast/overflow/animation-recompute-overflow.html: Added.
* platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (258335 => 258336)
--- trunk/LayoutTests/ChangeLog 2020-03-12 16:32:25 UTC (rev 258335)
+++ trunk/LayoutTests/ChangeLog 2020-03-12 17:09:34 UTC (rev 258336)
@@ -1,3 +1,18 @@
+2020-03-11 Simon Fraser <[email protected]>
+
+ REGRESSION (r254054): finance.google.com watch list renders initially then disappears for 5+ seconds before reappearing
+ https://bugs.webkit.org/show_bug.cgi?id=208972
+ <rdar://problem/59727171>
+
+ Reviewed by Zalan Bujtas.
+
+ Ref test, and some rebaselines where repaint order changed.
+
+ * css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:
+ * fast/overflow/animation-recompute-overflow-expected.html: Added.
+ * fast/overflow/animation-recompute-overflow.html: Added.
+ * platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt:
+
2020-03-12 Antti Koivisto <[email protected]>
Accurate style invalidation for user action pseudo classes
Modified: trunk/LayoutTests/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt (258335 => 258336)
--- trunk/LayoutTests/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt 2020-03-12 16:32:25 UTC (rev 258335)
+++ trunk/LayoutTests/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt 2020-03-12 17:09:34 UTC (rev 258336)
@@ -11,9 +11,6 @@
Test if unsetting a parent's stacking context correctly updates its parent isolation.
(repaint rects
- (rect 48 54 60 60)
- (rect 48 54 60 60)
- (rect 48 54 60 60)
(rect 48 172 60 60)
(rect 48 172 60 60)
(rect 28 290 60 60)
@@ -26,9 +23,12 @@
(rect 48 644 60 60)
(rect 68 644 60 60)
(rect 48 644 60 60)
+ (rect 48 172 60 60)
(rect 28 290 60 60)
(rect 48 644 60 60)
+ (rect 88 54 20 60)
(rect 48 54 60 60)
+ (rect 88 172 20 60)
(rect 48 290 60 60)
(rect 28 526 60 60)
(rect 88 644 20 60)
Added: trunk/LayoutTests/fast/overflow/animation-recompute-overflow-expected.html (0 => 258336)
--- trunk/LayoutTests/fast/overflow/animation-recompute-overflow-expected.html (rev 0)
+++ trunk/LayoutTests/fast/overflow/animation-recompute-overflow-expected.html 2020-03-12 17:09:34 UTC (rev 258336)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>The green box should not disappear</title>
+ <style>
+ .box {
+ width: 400px;
+ background-color: green;
+ height: 0px;
+ opacity: 1;
+ }
+
+ .content {
+ height: 200px;
+ width: 200px;
+ background-color: green;
+ }
+ </style>
+</head>
+<body>
+ <div class="box">
+ <div class="content"></div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/overflow/animation-recompute-overflow.html (0 => 258336)
--- trunk/LayoutTests/fast/overflow/animation-recompute-overflow.html (rev 0)
+++ trunk/LayoutTests/fast/overflow/animation-recompute-overflow.html 2020-03-12 17:09:34 UTC (rev 258336)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>The green box should not disappear</title>
+ <style>
+ .box {
+ width: 400px;
+ background-color: green;
+ height: 0px;
+ opacity: 0.8;
+ transition: opacity 0.1s;
+ }
+
+ body.changed .box {
+ opacity: 1;
+ }
+
+ .content {
+ height: 200px;
+ width: 200px;
+ background-color: green;
+ }
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ window.addEventListener('load', () => {
+
+ let box = document.getElementsByClassName('box')[0];
+ box.addEventListener('transitionend', () => {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, false);
+ setTimeout(() => {
+ document.body.classList.add('changed');
+ }, 0);
+ }, false);
+ </script>
+</head>
+<body>
+ <div class="box">
+ <div class="content"></div>
+ </div>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt (258335 => 258336)
--- trunk/LayoutTests/platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt 2020-03-12 16:32:25 UTC (rev 258335)
+++ trunk/LayoutTests/platform/ios/css3/blending/repaint/blend-mode-isolate-stacking-context-expected.txt 2020-03-12 17:09:34 UTC (rev 258336)
@@ -11,9 +11,6 @@
Test if unsetting a parent's stacking context correctly updates its parent isolation.
(repaint rects
- (rect 48 56 60 60)
- (rect 48 56 60 60)
- (rect 48 56 60 60)
(rect 48 176 60 60)
(rect 48 176 60 60)
(rect 28 296 60 60)
@@ -26,9 +23,12 @@
(rect 48 656 60 60)
(rect 68 656 60 60)
(rect 48 656 60 60)
+ (rect 48 176 60 60)
(rect 28 296 60 60)
(rect 48 656 60 60)
+ (rect 88 56 20 60)
(rect 48 56 60 60)
+ (rect 88 176 20 60)
(rect 48 296 60 60)
(rect 28 536 60 60)
(rect 88 656 20 60)
Modified: trunk/Source/WebCore/ChangeLog (258335 => 258336)
--- trunk/Source/WebCore/ChangeLog 2020-03-12 16:32:25 UTC (rev 258335)
+++ trunk/Source/WebCore/ChangeLog 2020-03-12 17:09:34 UTC (rev 258336)
@@ -1,3 +1,31 @@
+2020-03-11 Simon Fraser <[email protected]>
+
+ REGRESSION (r254054): finance.google.com watch list renders initially then disappears for 5+ seconds before reappearing
+ https://bugs.webkit.org/show_bug.cgi?id=208972
+ <rdar://problem/59727171>
+
+ Reviewed by Zalan Bujtas.
+
+ After r254054 we could get a style change in which opacity was unchanged, but
+ hasAutoUsedZIndex() in the style changed (because Adjuster::adjustAnimatedStyle() can set it).
+
+ In this case we failed to trigger layout, which means that we failed to recompute visual
+ overflow when a layer changed from being self-painting to non-self-painting (which affects
+ visual overflow computation and has hasAutoUsedZIndex() as input). We'd thus fail to paint some
+ renderers because their visual overflow didn't intersect the paint dirty rect.
+
+ Fix by having RenderStyle::changeRequiresLayout() return true if hasAutoUsedZIndex() differs
+ between the styles. This has minimal performance impact; rareNonInheritedDataChangeRequiresLayout()
+ already returns true if opacity, filters and other stacking-context-affecting properties change.
+
+ Test: fast/overflow/animation-recompute-overflow.html
+
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::addOverflowFromChild):
+ (WebCore::RenderBox::addLayoutOverflow):
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::RenderStyle::changeRequiresLayout const):
+
2020-03-12 Daniel Bates <[email protected]>
FocusController::setFocusedElement() should tell client of refocused element
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (258335 => 258336)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2020-03-12 16:32:25 UTC (rev 258335)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2020-03-12 17:09:34 UTC (rev 258336)
@@ -4614,14 +4614,14 @@
fragmentedFlow->addFragmentsOverflowFromChild(this, child, delta);
// Only propagate layout overflow from the child if the child isn't clipping its overflow. If it is, then
- // its overflow is internal to it, and we don't care about it. layoutOverflowRectForPropagation takes care of this
+ // its overflow is internal to it, and we don't care about it. layoutOverflowRectForPropagation takes care of this
// and just propagates the border box rect instead.
LayoutRect childLayoutOverflowRect = child->layoutOverflowRectForPropagation(&style());
childLayoutOverflowRect.move(delta);
addLayoutOverflow(childLayoutOverflowRect);
-
+
// Add in visual overflow from the child. Even if the child clips its overflow, it may still
- // have visual overflow of its own set from box shadows or reflections. It is unnecessary to propagate this
+ // have visual overflow of its own set from box shadows or reflections. It is unnecessary to propagate this
// overflow if we are clipping our own overflow.
if (child->hasSelfPaintingLayer() || hasOverflowClip())
return;
@@ -4640,7 +4640,7 @@
LayoutRect overflowRect(rect);
if (hasOverflowClip() || isRenderView()) {
// Overflow is in the block's coordinate space and thus is flipped for horizontal-bt and vertical-rl
- // writing modes. At this stage that is actually a simplification, since we can treat horizontal-tb/bt as the same
+ // writing modes. At this stage that is actually a simplification, since we can treat horizontal-tb/bt as the same
// and vertical-lr/rl as the same.
bool hasTopOverflow = isTopLayoutOverflowAllowed();
bool hasLeftOverflow = isLeftLayoutOverflowAllowed();
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (258335 => 258336)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2020-03-12 16:32:25 UTC (rev 258335)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2020-03-12 17:09:34 UTC (rev 258336)
@@ -817,6 +817,9 @@
if (m_boxData->boxSizing() != other.m_boxData->boxSizing())
return true;
+
+ if (m_boxData->hasAutoUsedZIndex() != other.m_boxData->hasAutoUsedZIndex())
+ return true;
}
if (m_surroundData->margin != other.m_surroundData->margin)
