Bug 72402 – [chromium] Merge chromium-gpu layout test configurations into non-gpu versions

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Adrienne Walker]

Chromium GPU tests have their own separate buildbot step and set of expectations.  This causes extra complexity, gardening pain, configuration overhead, and on-going confusion about who's responsible for them.  These tests should not be special.

There's no reason why these tests can't be part of the layout_tests step.  Arguably, we could just not disable hardware acceleration for all layout tests and let the GPU tests trigger hardware acceleration as-needed.

The only real sticking point is the canvas tests which are run in both hardware and software mode.  senorblanco argues that we could just run these in hardware, which would sidestep the problem entirely.

[James Robinson]

Completely agree.  I think the best way to tackle this is piece-by-piece.  The easiest thing to tackle is the compositing/webgl/3d animation tests - they should just run as normal layout tests using osmesa as appropriate.  I'll take a look at doing this today.

We can leave canvas 2d in a split configuration for now, with the assumption that longer-term skia could be responsible for testing these different paths.

[James Robinson]

I think I can just make DRT always enable compositing (with the normal compositing triggers) and then just run the compositing/ directories through the normal DRT path.  Our normal compositing triggers should not trip on any tests outside of these directories.

[James Robinson]

Hm, another potential issue is media/ - currently we run that through the compositing and non-compositing paths separately and have separate baselines for the two.  If we think running these tests two times is valuable then I could rig up in the same way as canvas 2d to run without the compositor on the main layout test run and then run with the compositor triggered by media in webkit_gpu_tests.

[Adrienne Walker]

Nutty idea: could we add a feature to DRT such that it mirrors a given directory, but with a variation like a js file forcibly injected into each one? Then we could have a virtual mirror of media/ called media-gpu/, and containing nothing but expectations.  With this, we could still run tests in both configs, wouldn't have to manually duplicate them, and could eliminate the separate buildbot step.  On the other hand, this forces different complexity into DRT and gardening tools.  But, maybe this feature would be useful in cases other than GPU land?

[James Robinson]

Created attachment 115236 [details]
work in progress

[James Robinson]

We currently have 562 files (expectations and tests) in LayoutTests/platform/chromium*gpu*/compositing/*.  I plan to do the following in order to land this:

1.) Copy all existing compositing expectations in gpu directories to the non-gpu-specific directory.  For example, everything in platform/chromium-gpu-win/compositing/ will be copied to platform/chromium-win/

2.) Land the code change to DumpRenderTree and our test scripts to run the compositing tests on the normal layout test run and not in the GPU run

3.) Remove the compositing expectations from platform/chromium-*gpu*/

4.) Remove the SKIP from these lines in test_expectations.txt:

WONTFIX SKIP CPU CPU-CG : compositing = PASS TIMEOUT FAIL
WONTFIX SKIP CPU CPU-CG : platform/chromium/compositing = PASS TIMEOUT FAIL
WONTFIX SKIP CPU CPU-CG : animations/3d = PASS TIMEOUT FAIL

to start running the tests.

5.) Garden the set of failures from these lines down to zero

6.) Remove the = FAIL lines for compositing/ and animations/3d from test_expectations.txt

7.) Profit

[Ojan Vafai]

6a) Remove run-webkit-tests support for the special -gpu configurations
6b) Remove all knowledge of the -gpu bots from the flakiness dashboard

[James Robinson]

(In reply to comment #7)
> 6a) Remove run-webkit-tests support for the special -gpu configurations
> 6b) Remove all knowledge of the -gpu bots from the flakiness dashboard

Can't do those until the canvas 2d and media tests are resolved. I'm only planning to move the compositing/ tests over right now since we only ever want to run those in one configuration (with compositing enabled via osmesa software rasterizer).  For canvas 2d and media we'll need to do a bit more work.

[Dirk Pranke]

(In reply to comment #4)
> Nutty idea: could we add a feature to DRT such that it mirrors a given directory, but with a variation like a js file forcibly injected into each one? Then we could have a virtual mirror of media/ called media-gpu/, and containing nothing but expectations.  With this, we could still run tests in both configs, wouldn't have to manually duplicate them, and could eliminate the separate buildbot step.  On the other hand, this forces different complexity into DRT and gardening tools.  But, maybe this feature would be useful in cases other than GPU land?

We'd have to modify NRWT to know how to mirror the trees so that it "found" both sets of tests, but that is presumably doable.

Also, your idea would I think require us to have 2x the number of expectations. I don't know how many of our expectations are duplicated today, or whether this would be much of an issue. I am curious how many of these tests could be expressed as reference tests. I also wonder if we could figure out how to express CPU/GPU as a reference test (i.e., the CPU's version should match the GPU's version)?

[ If we didn't duplicate the expectations, we'd have to define a fallback path and modifiers for what happens when you get differing results, which would put us back to where we are today. ]

[WebKit Review Bot]

Comment on attachment 115236 [details]
work in progress

Attachment 115236 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10372697

New failing tests:
fast/media/mq-transform-03.html
transitions/default-timing-function.html
animations/missing-values-first-keyframe.html
fast/repaint/block-selection-gap-in-composited-layer.html
fast/media/mq-transform-02.html
animations/opacity-transform-animation.html
animations/missing-values-last-keyframe.html

[James Robinson]

http://trac.webkit.org/changeset/100329, http://trac.webkit.org/changeset/100333 and http://trac.webkit.org/changeset/100337 complete step 1.

[James Robinson]

(In reply to comment #10)
> (From update of attachment 115236 [details])
> Attachment 115236 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/10372697
> 
> New failing tests:
> fast/media/mq-transform-03.html
> transitions/default-timing-function.html
> animations/missing-values-first-keyframe.html
> fast/repaint/block-selection-gap-in-composited-layer.html
> fast/media/mq-transform-02.html
> animations/opacity-transform-animation.html
> animations/missing-values-last-keyframe.html

These are most likely failing due to the compositing trigger for animating a non-3d transform. Currently they run with compositing disabled on the main layout test run. With this patch, they would run with compositing enabled.  I think we should just rebaseline with the compositing expectations, since that's the mode we run in actual chromium.

[James Robinson]

Created attachment 115263 [details]
Patch

[WebKit Review Bot]

Attachment 115263 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

ERROR: FAILURES FOR <lucid, x86_64, release, cpu> in LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:2735 Path does not exist. plugins/invalidate_rect.html
LayoutTests/platform/chromium/test_expectations.txt:2735:  Path does not exist. plugins/invalidate_rect.html  [test/expectations] [2]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dirk Pranke]

Comment on attachment 115263 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=115263&action=review

> Tools/ChangeLog:10
> +        and video are still controlled by an explicit trigger so that they are true when platform=chromium-gpu and false

Nit: maybe say "media" instead of video (or both) for people that aren't aware that the two are synonymous?

> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:431
> +            'accelerated_video': '--enable-accelerated-video',

I think you need to change --accelerated-compositing to --accelerated-video in run_webkit_tests.py as well (line220).

[James Robinson]

Committed r100355: <http://trac.webkit.org/changeset/100355>

[James Robinson]

Comment on attachment 115263 [details]
Patch

Clearing flags, this patch landed.

[James Robinson]

(In reply to comment #16)
> Committed r100355: <http://trac.webkit.org/changeset/100355>

This completes step 2 from comment #6.  I'm leaving this bug open to cover the rest of the steps there.

[James Robinson]

Committed r100360: <http://trac.webkit.org/changeset/100360>

[James Robinson]

http://trac.webkit.org/changeset/100360 handles steps 3 and 4.

[James Robinson]

Committed r100415: <http://trac.webkit.org/changeset/100415>

[James Robinson]

OK, the compositing tests are now running on the main webkit_tests run only and I've gardened down to 16 failures, which seems small enough to handle by more specialized bugs.  I'm going to mark this bug resolved.

Open issues are how to handle the canvas 2d and media tests.  Right now, those tests are still using webkit_gpu_tests.  I'll open up bugs for those two categories tomorrow to discuss options.