- Revision
- 285529
- Author
- grao...@webkit.org
- Date
- 2021-11-09 13:25:30 -0800 (Tue, 09 Nov 2021)
Log Message
REGRESSION(r272201): Safari showed red distortion on webview after using Web Inspector, returning from another app
https://bugs.webkit.org/show_bug.cgi?id=231358
<rdar://problem/81505208>
Reviewed by Devin Rousso.
Source/WebCore:
Test: inspector/page/setShowPaintRects.html
When we added support for animating individual transform properties, we moved to a model where all animations
for a given CSS property were wrapped in a dedicated CAAnimationGroup when running accelerated. Those groups
have an infinite duration and thus would never call the -animationDidStop:finished: CAAnimationDelegate method.
An option would have been to set the delegate on all animations contained in a CAAnimationGroup in
PlatformCALayerCocoa::addAnimationForKey(), but as it turns out, the CAAnimationDelegate methods for children
of an animation group aren't fired.
Since the only current use for the -animationDidStop:finished: delegate method is to eventually message back
into WebInspectorClient::animationEndedForLayer() for opacity animations, which don't need to run contained in
groups, our approach to address the issue is to only group animations for transform-related properties and
leave other animations as simple leaf animations.
To do this, inside of GraphicsLayerCA::updateAnimations(), we replace the addAnimation() lambda with a new
addLeafAnimation() lambda which we use for opacity, background-color and filter animations.
In order to be able to test the successful removal of repaint rects as a result of the CAAnimationDelegate
method firing, Patrick Angle contributed the required changes under inspector/ to expose the number of paint
rects to layout tests.
* inspector/InspectorClient.h:
(WebCore::InspectorClient::paintRectCount const):
* inspector/InspectorController.cpp:
(WebCore::InspectorController::paintRectCount const):
* inspector/InspectorController.h:
* inspector/InspectorOverlay.h:
(WebCore::InspectorOverlay::paintRectCount const):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::updateAnimations):
* testing/Internals.cpp:
(WebCore::Internals::inspectorHighlightRects):
(WebCore::Internals::inspectorPaintRectCount):
* testing/Internals.h:
* testing/Internals.idl:
Source/WebKit:
* WebProcess/Inspector/WebInspectorClient.h:
LayoutTests:
Add a new test, written by Patrick Angle, that tracks whether paint rects have appeared
and then disappeared as content repaints in an inspected page.
* inspector/page/setShowPaintRects-expected.txt: Added.
* inspector/page/setShowPaintRects.html: Added.
* platform/ios-wk1/TestExpectations:
* platform/mac-wk1/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (285528 => 285529)
--- trunk/LayoutTests/ChangeLog 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/LayoutTests/ChangeLog 2021-11-09 21:25:30 UTC (rev 285529)
@@ -1,3 +1,19 @@
+2021-11-09 Antoine Quint <grao...@webkit.org>
+
+ REGRESSION(r272201): Safari showed red distortion on webview after using Web Inspector, returning from another app
+ https://bugs.webkit.org/show_bug.cgi?id=231358
+ <rdar://problem/81505208>
+
+ Reviewed by Devin Rousso.
+
+ Add a new test, written by Patrick Angle, that tracks whether paint rects have appeared
+ and then disappeared as content repaints in an inspected page.
+
+ * inspector/page/setShowPaintRects-expected.txt: Added.
+ * inspector/page/setShowPaintRects.html: Added.
+ * platform/ios-wk1/TestExpectations:
+ * platform/mac-wk1/TestExpectations:
+
2021-11-09 Devin Rousso <drou...@apple.com>
REGRESSION(r271735): PaymentShippingOption.selected ignored
Added: trunk/LayoutTests/inspector/page/setShowPaintRects-expected.txt (0 => 285529)
--- trunk/LayoutTests/inspector/page/setShowPaintRects-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector/page/setShowPaintRects-expected.txt 2021-11-09 21:25:30 UTC (rev 285529)
@@ -0,0 +1,13 @@
+Tests for the Page.setShowPaintRects.
+
+
+== Running test suite: Page.setShowPaintRects
+-- Running test case: Page.setShowPaintRects.Enabled
+PASS: Should not have paint rects displayed.
+PASS: Should have paint rects displayed.
+PASS: Should not have paint rects displayed.
+
+-- Running test case: Page.setShowPaintRects.Disabled
+PASS: Should not have paint rects displayed.
+PASS: Should not have had any paint rects displayed.
+
Added: trunk/LayoutTests/inspector/page/setShowPaintRects.html (0 => 285529)
--- trunk/LayoutTests/inspector/page/setShowPaintRects.html (rev 0)
+++ trunk/LayoutTests/inspector/page/setShowPaintRects.html 2021-11-09 21:25:30 UTC (rev 285529)
@@ -0,0 +1,79 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script>
+
+window._test_hasPaintRects = false;
+window._test_hasPaintRectsObserver = setInterval(() => {
+ let paintRectsCount = window.internals.inspectorPaintRectCount();
+
+ // Exact paint rect counts may vary, so only check if there are paint rects.
+ if (!!paintRectsCount != window._test_hasPaintRects) {
+ window._test_hasPaintRects = !!paintRectsCount;
+ TestPage.dispatchEventToFrontend("TestHasPaintRectsDidChange");
+ }
+}, 0);
+
+function test()
+{
+ async function expectHasPaintRectsDidChangeTo(expected) {
+ await InspectorTest.awaitEvent("TestHasPaintRectsDidChange");
+ await expectHasPaintRects(expected);
+ }
+
+ async function expectHasPaintRects(expected) {
+ let hasPaintRects = await InspectorTest.evaluateInPage(`window._test_hasPaintRects`);
+ InspectorTest.expectEqual(hasPaintRects, expected, `Should ${expected ? "" : "not "}have paint rects displayed.`);
+ }
+
+ let suite = InspectorTest.createAsyncSuite("Page.setShowPaintRects");
+
+ suite.addTestCase({
+ name: "Page.setShowPaintRects.Enabled",
+ description: "Test that paint rects are correctly created and removed while enabled.",
+ async test() {
+ await PageAgent.setShowPaintRects(true);
+ await expectHasPaintRects(false);
+
+ let hasPaintRectsPromise = expectHasPaintRectsDidChangeTo(true);
+
+ await InspectorTest.evaluateInPage(`document.getElementById("test").style.backgroundColor = "papayawhip"`);
+ await hasPaintRectsPromise;
+ await expectHasPaintRectsDidChangeTo(false);
+ }
+ });
+
+ suite.addTestCase({
+ name: "Page.setShowPaintRects.Disabled",
+ description: "Test that paint rects are not created while disabled.",
+ async test() {
+ await PageAgent.setShowPaintRects(false);
+ await expectHasPaintRects(false);
+
+ let hasPaintRectsDidChange = Promise.race([
+ InspectorTest.awaitEvent("TestHasPaintRectsDidChange").then(() => true),
+ new Promise((resolve) => { setTimeout(resolve, 500, false); }),
+ ]);
+
+ await InspectorTest.evaluateInPage(`document.getElementById("test").style.backgroundColor = "rebeccapurple"`);
+ InspectorTest.expectFalse(await hasPaintRectsDidChange, `Should not have had any paint rects displayed.`);
+ }
+ });
+
+ suite.runTestCasesAndFinish();
+}
+</script>
+<style>
+#test {
+ width: 100px;
+ height: 100px;
+ background-color: gainsboro;
+}
+</style>
+</head>
+<body _onload_="runTest()">
+<p>Tests for the Page.setShowPaintRects.</p>
+<div id="test"></div>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios-wk1/TestExpectations (285528 => 285529)
--- trunk/LayoutTests/platform/ios-wk1/TestExpectations 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/LayoutTests/platform/ios-wk1/TestExpectations 2021-11-09 21:25:30 UTC (rev 285529)
@@ -1953,3 +1953,6 @@
webkit.org/b/223193 imported/w3c/web-platform-tests/html/canvas/element/path-objects/2d.path.stroke.scale2.html [ Failure ]
webkit.org/b/229247 http/tests/fetch/keepalive-fetch-2.html [ Pass Failure ]
+
+# DumpRenderTree doesn't invoke inspector instrumentation when repainting.
+webkit.org/b/232852 inspector/page/setShowPaintRects.html [ Skip ]
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (285528 => 285529)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2021-11-09 21:25:30 UTC (rev 285529)
@@ -1514,3 +1514,6 @@
webkit.org/b/232446 [ Catalina Debug ] media/track/track-element-dom-change-crash.html [ Pass Crash ]
webkit.org/b/232585 [ Catalina Debug ] media/track/track-element-load-event.html [ Pass Crash ]
+
+# DumpRenderTree doesn't invoke inspector instrumentation when repainting.
+webkit.org/b/232852 inspector/page/setShowPaintRects.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (285528 => 285529)
--- trunk/Source/WebCore/ChangeLog 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/ChangeLog 2021-11-09 21:25:30 UTC (rev 285529)
@@ -1,3 +1,48 @@
+2021-11-09 Antoine Quint <grao...@webkit.org>
+
+ REGRESSION(r272201): Safari showed red distortion on webview after using Web Inspector, returning from another app
+ https://bugs.webkit.org/show_bug.cgi?id=231358
+ <rdar://problem/81505208>
+
+ Reviewed by Devin Rousso.
+
+ Test: inspector/page/setShowPaintRects.html
+
+ When we added support for animating individual transform properties, we moved to a model where all animations
+ for a given CSS property were wrapped in a dedicated CAAnimationGroup when running accelerated. Those groups
+ have an infinite duration and thus would never call the -animationDidStop:finished: CAAnimationDelegate method.
+
+ An option would have been to set the delegate on all animations contained in a CAAnimationGroup in
+ PlatformCALayerCocoa::addAnimationForKey(), but as it turns out, the CAAnimationDelegate methods for children
+ of an animation group aren't fired.
+
+ Since the only current use for the -animationDidStop:finished: delegate method is to eventually message back
+ into WebInspectorClient::animationEndedForLayer() for opacity animations, which don't need to run contained in
+ groups, our approach to address the issue is to only group animations for transform-related properties and
+ leave other animations as simple leaf animations.
+
+ To do this, inside of GraphicsLayerCA::updateAnimations(), we replace the addAnimation() lambda with a new
+ addLeafAnimation() lambda which we use for opacity, background-color and filter animations.
+
+ In order to be able to test the successful removal of repaint rects as a result of the CAAnimationDelegate
+ method firing, Patrick Angle contributed the required changes under inspector/ to expose the number of paint
+ rects to layout tests.
+
+ * inspector/InspectorClient.h:
+ (WebCore::InspectorClient::paintRectCount const):
+ * inspector/InspectorController.cpp:
+ (WebCore::InspectorController::paintRectCount const):
+ * inspector/InspectorController.h:
+ * inspector/InspectorOverlay.h:
+ (WebCore::InspectorOverlay::paintRectCount const):
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::updateAnimations):
+ * testing/Internals.cpp:
+ (WebCore::Internals::inspectorHighlightRects):
+ (WebCore::Internals::inspectorPaintRectCount):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2021-11-09 Megan Gardner <megan_gard...@apple.com>
Scroll To Text Fragment directive parsing
Modified: trunk/Source/WebCore/inspector/InspectorClient.h (285528 => 285529)
--- trunk/Source/WebCore/inspector/InspectorClient.h 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/inspector/InspectorClient.h 2021-11-09 21:25:30 UTC (rev 285529)
@@ -60,6 +60,7 @@
virtual bool overridesShowPaintRects() const { return false; }
virtual void setShowPaintRects(bool) { }
virtual void showPaintRect(const FloatRect&) { }
+ virtual unsigned paintRectCount() const { return 0; }
virtual void didSetSearchingForNode(bool) { }
virtual void elementSelectionChanged(bool) { }
virtual void timelineRecordingChanged(bool) { }
Modified: trunk/Source/WebCore/inspector/InspectorController.cpp (285528 => 285529)
--- trunk/Source/WebCore/inspector/InspectorController.cpp 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/inspector/InspectorController.cpp 2021-11-09 21:25:30 UTC (rev 285529)
@@ -364,6 +364,14 @@
return m_overlay->gridOverlayCount();
}
+unsigned InspectorController::paintRectCount() const
+{
+ if (m_inspectorClient->overridesShowPaintRects())
+ return m_inspectorClient->paintRectCount();
+
+ return m_overlay->paintRectCount();
+}
+
bool InspectorController::shouldShowOverlay() const
{
return m_overlay->shouldShowOverlay();
Modified: trunk/Source/WebCore/inspector/InspectorController.h (285528 => 285529)
--- trunk/Source/WebCore/inspector/InspectorController.h 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/inspector/InspectorController.h 2021-11-09 21:25:30 UTC (rev 285529)
@@ -106,6 +106,7 @@
void setIsUnderTest(bool isUnderTest) { m_isUnderTest = isUnderTest; }
WEBCORE_EXPORT void evaluateForTestInFrontend(const String& script);
WEBCORE_EXPORT unsigned gridOverlayCount() const;
+ WEBCORE_EXPORT unsigned paintRectCount() const;
InspectorClient* inspectorClient() const { return m_inspectorClient; }
InspectorFrontendClient* inspectorFrontendClient() const { return m_inspectorFrontendClient; }
Modified: trunk/Source/WebCore/inspector/InspectorOverlay.h (285528 => 285529)
--- trunk/Source/WebCore/inspector/InspectorOverlay.h 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/inspector/InspectorOverlay.h 2021-11-09 21:25:30 UTC (rev 285529)
@@ -200,6 +200,7 @@
void setShowPaintRects(bool);
void showPaintRect(const FloatRect&);
+ unsigned paintRectCount() const { return m_paintRects.size(); }
void setShowRulers(bool);
void setShowRulersDuringElementSelection(bool enabled) { m_showRulersDuringElementSelection = enabled; }
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (285528 => 285529)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2021-11-09 21:25:30 UTC (rev 285529)
@@ -3085,9 +3085,11 @@
animation.m_playState = PlayState::Playing;
};
- auto addAnimation = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
- prepareAnimationForAddition(animation, additive);
- addAnimationGroup(animation.m_property, { animation.m_animation });
+ auto addLeafAnimation = [&](LayerPropertyAnimation& animation) {
+ ASSERT(animation.m_beginTime);
+ *animation.m_beginTime += animationGroupBeginTime;
+ prepareAnimationForAddition(animation, Additive::No);
+ setAnimationOnLayer(animation);
};
enum class TransformationMatrixSource { UseIdentityMatrix, AskClient };
@@ -3122,8 +3124,10 @@
// is the initial base value transform animation and must override the current transform value for this layer.
// Otherwise, it is meant to apply the underlying value for one specific transform-related property and be additive
// to be combined with the other base value transform animations and interpolating animations.
- if (auto* animation = makeBaseValueTransformAnimation(property, matrixSource, beginTimeOfEarliestPropertyAnimation))
- addAnimation(*animation, matrixSource == TransformationMatrixSource::AskClient ? Additive::Yes : Additive::No);
+ if (auto* animation = makeBaseValueTransformAnimation(property, matrixSource, beginTimeOfEarliestPropertyAnimation)) {
+ prepareAnimationForAddition(*animation, matrixSource == TransformationMatrixSource::AskClient ? Additive::Yes : Additive::No);
+ addAnimationGroup(animation->m_property, { animation->m_animation });
+ }
};
// Iterate through all animations to set the begin time of any new animations.
@@ -3132,7 +3136,10 @@
animation.m_beginTime = currentTime - animationGroupBeginTime;
}
- // Now, remove all animation groups from the layer so that we no longer have any layer animations.
+ // Now, remove all animation groups and leaf animations from the layer so that
+ // we no longer have any layer animations.
+ for (auto& animation : m_animations)
+ removeCAAnimationFromLayer(animation);
for (auto& animationGroup : m_animationGroups)
removeCAAnimationFromLayer(animationGroup);
@@ -3187,7 +3194,7 @@
#if ENABLE(FILTERS_LEVEL_2)
case AnimatedPropertyWebkitBackdropFilter:
#endif
- addAnimation(animation, Additive::No);
+ addLeafAnimation(animation);
break;
case AnimatedPropertyInvalid:
ASSERT_NOT_REACHED();
Modified: trunk/Source/WebCore/testing/Internals.cpp (285528 => 285529)
--- trunk/Source/WebCore/testing/Internals.cpp 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/testing/Internals.cpp 2021-11-09 21:25:30 UTC (rev 285529)
@@ -1766,6 +1766,15 @@
return DOMRectList::create(highlight.quads);
}
+ExceptionOr<unsigned> Internals::inspectorPaintRectCount()
+{
+ auto document = contextDocument();
+ if (!document || !document->page())
+ return Exception { InvalidAccessError };
+
+ return document->page()->inspectorController().paintRectCount();
+}
+
ExceptionOr<unsigned> Internals::markerCountForNode(Node& node, const String& markerType)
{
OptionSet<DocumentMarker::MarkerType> markerTypes;
Modified: trunk/Source/WebCore/testing/Internals.h (285528 => 285529)
--- trunk/Source/WebCore/testing/Internals.h 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/testing/Internals.h 2021-11-09 21:25:30 UTC (rev 285529)
@@ -287,6 +287,7 @@
ExceptionOr<Ref<DOMRectList>> inspectorHighlightRects();
ExceptionOr<unsigned> inspectorGridOverlayCount();
+ ExceptionOr<unsigned> inspectorPaintRectCount();
ExceptionOr<unsigned> markerCountForNode(Node&, const String&);
ExceptionOr<RefPtr<Range>> markerRangeForNode(Node&, const String& markerType, unsigned index);
Modified: trunk/Source/WebCore/testing/Internals.idl (285528 => 285529)
--- trunk/Source/WebCore/testing/Internals.idl 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebCore/testing/Internals.idl 2021-11-09 21:25:30 UTC (rev 285529)
@@ -373,6 +373,7 @@
unsigned long inspectorGridOverlayCount();
DOMRectList inspectorHighlightRects();
+ unsigned long inspectorPaintRectCount();
unsigned long markerCountForNode(Node node, DOMString markerType);
Range? markerRangeForNode(Node node, DOMString markerType, unsigned long index);
Modified: trunk/Source/WebKit/ChangeLog (285528 => 285529)
--- trunk/Source/WebKit/ChangeLog 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebKit/ChangeLog 2021-11-09 21:25:30 UTC (rev 285529)
@@ -1,3 +1,13 @@
+2021-11-09 Antoine Quint <grao...@webkit.org>
+
+ REGRESSION(r272201): Safari showed red distortion on webview after using Web Inspector, returning from another app
+ https://bugs.webkit.org/show_bug.cgi?id=231358
+ <rdar://problem/81505208>
+
+ Reviewed by Devin Rousso.
+
+ * WebProcess/Inspector/WebInspectorClient.h:
+
2021-11-09 Tim Horton <timothy_hor...@apple.com>
Add runtime flag for momentum scrolling
Modified: trunk/Source/WebKit/WebProcess/Inspector/WebInspectorClient.h (285528 => 285529)
--- trunk/Source/WebKit/WebProcess/Inspector/WebInspectorClient.h 2021-11-09 21:17:19 UTC (rev 285528)
+++ trunk/Source/WebKit/WebProcess/Inspector/WebInspectorClient.h 2021-11-09 21:25:30 UTC (rev 285529)
@@ -72,6 +72,7 @@
bool overridesShowPaintRects() const override { return true; }
void showPaintRect(const WebCore::FloatRect&) override;
+ unsigned paintRectCount() const override { return m_paintRectLayers.size(); }
void setDeveloperPreferenceOverride(WebCore::InspectorClient::DeveloperPreference, std::optional<bool>) final;
