Bug 58772 – [chromium, win] Multiple failures of shadow rendering layout tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Andrey Kosyakov]

The following tests fail on chromium win since around r83936:

fast/multicol/shadow-breaking.html
fast/repaint/shadow-multiple-horizontal.html
fast/repaint/shadow-multiple-strict-horizontal.html
fast/repaint/shadow-multiple-strict-vertical.html
fast/repaint/shadow-multiple-vertical.html
fast/text/shadow-translucent-fill.html
fast/transforms/shadows.html
svg/css/group-with-shadow.svg
svg/css/shadow-changes.svg
svg/css/text-shadow-multiple.xhtml
transitions/svg-text-shadow-transition.html
svg/css/composite-shadow-text.svg

Note the shadow is missing on many elements:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20chromium.org&tests=fast%2Fmulticol%2Fshadow-breaking.html%2Cfast%2Frepaint%2Fshadow-multiple-horizontal.html%2Cfast%2Frepaint%2Fshadow-multiple-strict-horizontal.html%2Cfast%2Frepaint%2Fshadow-multiple-strict-vertical.html%2Cfast%2Frepaint%2Fshadow-multiple-vertical.html%2Cfast%2Ftext%2Fshadow-translucent-fill.html%2Cfast%2Ftransforms%2Fshadows.html%2Csvg%2Fcss%2Fgroup-with-shadow.svg%2Csvg%2Fcss%2Fshadow-changes.svg%2Csvg%2Fcss%2Ftext-shadow-multiple.xhtml%2Ctransitions%2Fsvg-text-shadow-transition.html%2Csvg%2Fcss%2Fcomposite-shadow-text.svg

[Stephen White]

According to the flakiness dashboard, it started failing at either r83935 or r83936.

http://trac.webkit.org/log/?verbose=on&rev=83936&stop_rev=83935

Mike and Brian, over to you.

[Mike Reed]

Created attachment 90549 [details]
Patch

[Mike Reed]

When I changed the shadow code before, I incorrectly assumed that the shadow's alpha should be modulated by the fillColor's alpha. Wrong (at least for webkit). This change ignores the fillColor's alpha by overwriting it for the shadow layer (hence the kSrc_Mode).

[Stephen White]

Looks good (unofficially).

Nit:  Probably better to use C++-style comments (//) for consistency with the rest of the file.  There's nothing in the style guide about it, though, so you can feel free to ignore me.  :)

[Kenneth Russell]

Comment on attachment 90549 [details]
Patch

Sounds fine. r=me

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 90549 [details]:

http/tests/misc/favicon-loads-with-icon-loading-override.html bug 58412 (author: [e-mail])
http/tests/xmlhttprequest/basic-auth-nouser.html bug 52978
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 90549 [details]
Patch

Clearing flags on attachment: 90549

Committed r84535: <http://trac.webkit.org/changeset/84535>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Mike Reed]

Created attachment 90722 [details]
Patch

[Mike Reed]

Reopening to update test_expectations and baseline images

[Mike Reed]

Created attachment 90724 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 90724 [details]
Patch

OK.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 90724 [details]:

media/video-playbackrate.html bug 58629 (author: [e-mail])
http/tests/xmlhttprequest/cross-origin-authorization.html bug 52398 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 90724 [details]
Patch

Clearing flags on attachment: 90724

Committed r84663: <http://trac.webkit.org/changeset/84663>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.