Commit f889e443 authored by Sadrul Habib Chowdhury's avatar Sadrul Habib Chowdhury Committed by Commit Bot

layout tests: Fix some flakes with --enable-display-compositor-pixel-dump

A number of layout tests requests a testRunner.capturePixelsAsyncThen().
When not using the display-compositor for pixel-dumps, this triggers a
complete composition (i.e. this triggers a commit, raster, frame
submission, and processing in the display compositor). The tests that
use the testRunner.capturePixelsAsyncThen(callback) API expects these
steps to be completed before the callback is run. However, when using the
display-compositor for pixel-dumps, it immediately triggers the callback
without doing any of these steps. This caused a bunch of tests to fail,
or become flaky, when --enable-display-compositor-pixel-dump became the
default.

This patch fixes the flakiness, by making sure the callback is run only
after all the required steps have been performed. This is done by forcing
a redraw, and requesting a presentation-timestamp from viz. This does not
actually fix the readback itself, i.e. the pixels returned to the
callback is still invalid. However, this does fix the timing of running
the callback. That is sufficient to fix a number of the existing flaky
tests.

Bug: 891427
Change-Id: Ic09db1c3a0a410b2d5687999d7144260df7ea272
Reviewed-on: https://chromium-review.googlesource.com/c/1322364Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatardanakj <danakj@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607906}
parent 39bf8dc9
...@@ -603,6 +603,24 @@ void LayerTreeView::RequestDecode(const cc::PaintImage& image, ...@@ -603,6 +603,24 @@ void LayerTreeView::RequestDecode(const cc::PaintImage& image,
} }
} }
void LayerTreeView::RequestPresentationCallback(base::OnceClosure callback) {
layer_tree_host_->RequestPresentationTimeForNextFrame(base::BindOnce(
[](base::OnceClosure callback,
const gfx::PresentationFeedback& feedback) {
std::move(callback).Run();
},
std::move(callback)));
SetNeedsForcedRedraw();
if (CompositeIsSynchronous()) {
main_thread_->PostTask(
FROM_HERE,
base::BindOnce(&LayerTreeView::SynchronouslyComposite,
weak_factory_.GetWeakPtr(), /*raster=*/true, nullptr));
} else {
layer_tree_host_->SetNeedsCommit();
}
}
void LayerTreeView::SetOverscrollBehavior( void LayerTreeView::SetOverscrollBehavior(
const cc::OverscrollBehavior& behavior) { const cc::OverscrollBehavior& behavior) {
layer_tree_host_->SetOverscrollBehavior(behavior); layer_tree_host_->SetOverscrollBehavior(behavior);
......
...@@ -181,6 +181,7 @@ class CONTENT_EXPORT LayerTreeView ...@@ -181,6 +181,7 @@ class CONTENT_EXPORT LayerTreeView
void SetBrowserControlsShownRatio(float) override; void SetBrowserControlsShownRatio(float) override;
void RequestDecode(const cc::PaintImage& image, void RequestDecode(const cc::PaintImage& image,
base::OnceCallback<void(bool)> callback) override; base::OnceCallback<void(bool)> callback) override;
void RequestPresentationCallback(base::OnceClosure callback) override;
void SetOverscrollBehavior(const cc::OverscrollBehavior&) override; void SetOverscrollBehavior(const cc::OverscrollBehavior&) override;
......
...@@ -1717,12 +1717,15 @@ bool TestRunner::DumpPixelsAsync( ...@@ -1717,12 +1717,15 @@ bool TestRunner::DumpPixelsAsync(
if (!layout_test_runtime_flags_.is_printing() && if (!layout_test_runtime_flags_.is_printing() &&
base::CommandLine::ForCurrentProcess()->HasSwitch( base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableDisplayCompositorPixelDump)) { switches::kEnableDisplayCompositorPixelDump)) {
// Because of the plumbing, we should still call the callback with an empty frame->View()->MainFrameWidget()->RequestPresentationCallbackForTesting(
// bitmap, but return true so that the browser also captures the pixels. base::BindOnce(
[](base::OnceCallback<void(const SkBitmap&)> callback) {
SkBitmap bitmap; SkBitmap bitmap;
bitmap.allocN32Pixels(1, 1); bitmap.allocN32Pixels(1, 1);
bitmap.eraseColor(0); bitmap.eraseColor(0);
std::move(callback).Run(bitmap); std::move(callback).Run(bitmap);
},
std::move(callback)));
return true; return true;
} }
......
...@@ -23,18 +23,14 @@ crbug.com/807686 crbug.com/24182 jquery/manipulation.html [ Timeout Pass ] ...@@ -23,18 +23,14 @@ crbug.com/807686 crbug.com/24182 jquery/manipulation.html [ Timeout Pass ]
### These fail on all platforms: ### These fail on all platforms:
## Next 18 can be viewed in this run, by clicking on "Did not pass 11183": ## Next 18 can be viewed in this run, by clicking on "Did not pass 11183":
## https://test-results.appspot.com/data/layout_results/linux-blink-rel/1237/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html ## https://test-results.appspot.com/data/layout_results/linux-blink-rel/1237/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html
crbug.com/891427 http/tests/devtools/tracing/frame-model-instrumentation.js [ Failure ]
crbug.com/891427 rootscroller/rootscroller-during-fullscreen.html [ Failure Pass ] crbug.com/891427 rootscroller/rootscroller-during-fullscreen.html [ Failure Pass ]
crbug.com/891427 virtual/threaded/animations/responsive/viewport-unit-transform-responsive.html [ Timeout ] crbug.com/891427 virtual/threaded/animations/responsive/viewport-unit-transform-responsive.html [ Timeout ]
crbug.com/891427 virtual/threaded/animations/responsive/viewport-unit-translate-responsive.html [ Timeout ] crbug.com/891427 virtual/threaded/animations/responsive/viewport-unit-translate-responsive.html [ Timeout ]
crbug.com/891427 virtual/threaded/http/tests/devtools/tracing/frame-model-instrumentation.js [ Failure ]
crbug.com/891427 fast/scrolling/editor-command-scroll-page-scale.html [ Pass Failure ] crbug.com/891427 fast/scrolling/editor-command-scroll-page-scale.html [ Pass Failure ]
crbug.com/891427 virtual/fractional_scrolling/fast/scrolling/editor-command-scroll-page-scale.html [ Pass Failure ] crbug.com/891427 virtual/fractional_scrolling/fast/scrolling/editor-command-scroll-page-scale.html [ Pass Failure ]
crbug.com/891427 virtual/scroll_customization/fast/scrolling/editor-command-scroll-page-scale.html [ Pass Failure ] crbug.com/891427 virtual/scroll_customization/fast/scrolling/editor-command-scroll-page-scale.html [ Pass Failure ]
crbug.com/891427 virtual/threaded/fast/scrolling/editor-command-scroll-page-scale.html [ Failure ] crbug.com/891427 virtual/threaded/fast/scrolling/editor-command-scroll-page-scale.html [ Failure ]
crbug.com/891427 fast/replaced/replaced-breaking.html [ Failure ] crbug.com/891427 fast/replaced/replaced-breaking.html [ Failure ]
crbug.com/891427 http/tests/devtools/tracing/decode-resize.js [ Failure ]
crbug.com/891427 virtual/threaded/http/tests/devtools/tracing/decode-resize.js [ Failure ]
crbug.com/891427 virtual/android/fullscreen/rendering/backdrop-video.html [ Failure ] crbug.com/891427 virtual/android/fullscreen/rendering/backdrop-video.html [ Failure ]
crbug.com/891427 virtual/android/rootscroller/set-root-scroller.html [ Failure ] crbug.com/891427 virtual/android/rootscroller/set-root-scroller.html [ Failure ]
crbug.com/891427 virtual/android/rootscroller/set-rootscroller-before-load.html [ Failure ] crbug.com/891427 virtual/android/rootscroller/set-rootscroller-before-load.html [ Failure ]
...@@ -63,9 +59,6 @@ crbug.com/891427 virtual/user-activation-v2/fast/events/touch/gesture/touch-gest ...@@ -63,9 +59,6 @@ crbug.com/891427 virtual/user-activation-v2/fast/events/touch/gesture/touch-gest
# Next 2 here: https://ci.chromium.org/buildbot/tryserver.blink/win7-blink-rel/1183 # Next 2 here: https://ci.chromium.org/buildbot/tryserver.blink/win7-blink-rel/1183
crbug.com/891427 external/wpt/resource-timing/resource_timing_buffer_full_eventually.html [ Pass Failure Timeout Crash ] crbug.com/891427 external/wpt/resource-timing/resource_timing_buffer_full_eventually.html [ Pass Failure Timeout Crash ]
crbug.com/891427 fast/canvas/canvas-drawImage-live-video.html [ Pass Failure Timeout Crash ] crbug.com/891427 fast/canvas/canvas-drawImage-live-video.html [ Pass Failure Timeout Crash ]
# Next 2 here: https://ci.chromium.org/buildbot/tryserver.blink/mac10.13-blink-rel/1144
crbug.com/891427 virtual/threaded/http/tests/devtools/tracing/timeline-paint/timeline-paint-image.js [ Pass Failure Timeout Crash ]
crbug.com/891427 http/tests/devtools/tracing/timeline-paint/timeline-paint-image.js [ Pass Failure Timeout Crash ]
# Next 1 here: https://ci.chromium.org/buildbot/tryserver.blink/win7-blink-rel/1180 # Next 1 here: https://ci.chromium.org/buildbot/tryserver.blink/win7-blink-rel/1180
crbug.com/891427 editing/pasteboard/drop-text-without-selection.html [ Pass Failure Timeout Crash ] crbug.com/891427 editing/pasteboard/drop-text-without-selection.html [ Pass Failure Timeout Crash ]
# Next 4 here: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux-blink-rel/1541 # Next 4 here: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux-blink-rel/1541
......
...@@ -225,6 +225,11 @@ class WebLayerTreeView { ...@@ -225,6 +225,11 @@ class WebLayerTreeView {
virtual void RequestDecode(const cc::PaintImage& image, virtual void RequestDecode(const cc::PaintImage& image,
base::OnceCallback<void(bool)> callback) {} base::OnceCallback<void(bool)> callback) {}
// Runs |callback| after a new frame has been submitted to the display
// compositor, and the display-compositor has displayed it on screen. Forces a
// redraw so that a new frame is submitted.
virtual void RequestPresentationCallback(base::OnceClosure callback) {}
}; };
} // namespace blink } // namespace blink
......
...@@ -148,6 +148,12 @@ class WebWidget { ...@@ -148,6 +148,12 @@ class WebWidget {
virtual void CompositeAndReadbackAsync( virtual void CompositeAndReadbackAsync(
base::OnceCallback<void(const SkBitmap&)> callback) {} base::OnceCallback<void(const SkBitmap&)> callback) {}
// Runs |callback| after a new frame has been submitted to the display
// compositor, and the display-compositor has displayed it on screen. Forces a
// redraw so that a new frame is submitted.
virtual void RequestPresentationCallbackForTesting(
base::OnceClosure callback) {}
// Called to inform the WebWidget of a change in theme. // Called to inform the WebWidget of a change in theme.
// Implementors that cache rendered copies of widgets need to re-render // Implementors that cache rendered copies of widgets need to re-render
// on receiving this message // on receiving this message
......
...@@ -1590,6 +1590,11 @@ void WebViewImpl::UpdateAllLifecyclePhasesAndCompositeForTesting( ...@@ -1590,6 +1590,11 @@ void WebViewImpl::UpdateAllLifecyclePhasesAndCompositeForTesting(
} }
} }
void WebViewImpl::RequestPresentationCallbackForTesting(
base::OnceClosure callback) {
layer_tree_view_->RequestPresentationCallback(std::move(callback));
}
void WebViewImpl::PaintContent(cc::PaintCanvas* canvas, const WebRect& rect) { void WebViewImpl::PaintContent(cc::PaintCanvas* canvas, const WebRect& rect) {
// This should only be used when compositing is not being used for this // This should only be used when compositing is not being used for this
// WebView, and it is painting into the recording of its parent. // WebView, and it is painting into the recording of its parent.
......
...@@ -123,6 +123,8 @@ class CORE_EXPORT WebViewImpl final : public WebView, ...@@ -123,6 +123,8 @@ class CORE_EXPORT WebViewImpl final : public WebView,
void RecordEndOfFrameMetrics(base::TimeTicks frame_begin_time) override; void RecordEndOfFrameMetrics(base::TimeTicks frame_begin_time) override;
void UpdateLifecycle(LifecycleUpdate requested_update) override; void UpdateLifecycle(LifecycleUpdate requested_update) override;
void UpdateAllLifecyclePhasesAndCompositeForTesting(bool do_raster) override; void UpdateAllLifecyclePhasesAndCompositeForTesting(bool do_raster) override;
void RequestPresentationCallbackForTesting(
base::OnceClosure callback) override;
void PaintContent(cc::PaintCanvas*, const WebRect&) override; void PaintContent(cc::PaintCanvas*, const WebRect&) override;
void PaintContentIgnoringCompositing(cc::PaintCanvas*, void PaintContentIgnoringCompositing(cc::PaintCanvas*,
const WebRect&) override; const WebRect&) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment