Commit c41b2d08 authored by Mason Freed's avatar Mason Freed Committed by Commit Bot

Enable display compositor pixel dumps by default.

SHERIFFS: PLEASE DO NOT REVERT THIS CL BECAUSE OF A SMALL AMOUNT OF LAYOUT
TEST FLAKINESS. If a few layout tests begin to show flakiness after this CL
lands, please add them to TestExpectations and email me (masonfreed@) to
investigate. Though I tried to identify all potentially flaky tests, a few
may have slipped through.

With this CL, the --enable-display-compositor-pixel-dump flag becomes the default
for content_shell. With this flag in place, layout test pixel dumps are performed
from the browser side, instead of from the renderer side. Note that to avoid a
significant amount of layout test flakiness, another change was also made to
not add the --run-all-compositor-stages-before-draw flag by default. There is
a bug (crbug.com/894613) tracking that problem separately.

With the flip of this switch, several modifications had to be made to the
TestExpectations file. First, there are a number of tests that change their
appearance slightly when being captured from the browser, and these tests need
to be rebaselined. These are summarized below, and will be rebaselined as a
separate CL, once this one lands and has had time to stabilize.

These bugs track the items added to TestExpectations:
 - crbug.com/887140: HDR support
 - crbug.com/881040: Media controls now contain an overflow menu.
 - crbug.com/667551: A bunch of tests are listed under this bug, and just
                     require rebaselining to fix non-material single-pixel
                     antialiasing failures.
 - crbug.com/891427: These either start failing, or become flaky, when the
                     --enable-display-compositor-pixel-dump flag is enabled.
                     They need to be debugged prior to re-enabling.
 - crbug.com/895556: These tests double their background size when the flag
                     is enabled. They need to be fixed or rebaselined.

Bug: 667551, 891427, 881040, 887140, 894613, 895556
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I46946e6377f659c9dedc0dfaa20e7658e8cc519d
Reviewed-on: https://chromium-review.googlesource.com/c/1213864
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Reviewed-by: default avatarPhilip Jägenstedt <foolip@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Reviewed-by: default avatarStephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603127}
parent 198fc70d
......@@ -213,6 +213,13 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {
command_line.AppendSwitch(cc::switches::kEnableGpuBenchmarking);
command_line.AppendSwitch(switches::kEnableLogging);
command_line.AppendSwitch(switches::kAllowFileAccessFromFiles);
#if !defined(OS_ANDROID)
// TODO(crbug/567947) Enable display compositor pixel dumps for Android
// once testing becomes possible on post-kitkat OSes, and once we've
// had a chance to debug the layout test failures that occur when this
// flag is present.
command_line.AppendSwitch(switches::kEnableDisplayCompositorPixelDump);
#endif
// only default to a software GL if the flag isn't already specified.
if (!command_line.HasSwitch(switches::kUseGpuInTests) &&
!command_line.HasSwitch(switches::kUseGL)) {
......@@ -247,7 +254,9 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {
// checker imaging, since it's imcompatible with single threaded compositor
// and display compositor pixel dumps.
if (command_line.HasSwitch(switches::kEnableDisplayCompositorPixelDump)) {
command_line.AppendSwitch(switches::kRunAllCompositorStagesBeforeDraw);
// TODO(crbug.com/894613) Add kRunAllCompositorStagesBeforeDraw back here
// once you figure out why it causes so much layout test flakiness.
// command_line.AppendSwitch(switches::kRunAllCompositorStagesBeforeDraw);
command_line.AppendSwitch(cc::switches::kDisableCheckerImaging);
}
......
......@@ -505,7 +505,7 @@ dump mode (now the standard), the standard behavior for tests is to
synchronously composite without rastering (to save time). However, animations
run upon surface activation, which only happens once rasterization is performed.
Therefore, for these tests, an additional setting needs to be set. Near the
beginning of these tests, call `continuouslyRunAnimations()` defined in
beginning of these tests, call `setAnimationRequiresRaster()` defined in
[third_party/WebKit/LayoutTests/resources/compositor-controls.js](../../third_party/WebKit/LayoutTests/resources/compositor-controls.js)
which will enable full rasterization during the test.
......
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