Bug 139889

Summary: Some compositing reflections tests are broken
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, esprehn+autocc, glenn, kondapallykalyan, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch thorton: review+

Description Simon Fraser (smfr) 2014-12-22 17:27:55 PST 
Running pixel tests shows some compositing tests which fail now:

compositing/reflections/animation-inside-reflection.html
compositing/reflections/compositing-change-inside-reflection.html
compositing/reflections/masked-reflection-on-composited.html
compositing/reflections/nested-reflection-on-overflow.html
compositing/reflections/reflection-opacity.html
compositing/reflections/remove-add-reflection.html
compositing/reflections/transform-inside-reflection.html

Regression range:
http://trac.webkit.org/log/trunk/?rev=160672&stop_rev=160666
so probably http://trac.webkit.org/changeset/160672
Comment 1 Simon Fraser (smfr) 2014-12-22 18:12:40 PST 
PlatformCALayerMac::updateCustomAppearance() nukes the background color under ScrollbarThemeMac::removeOverhangAreaBackground().
Comment 2 Simon Fraser (smfr) 2015-01-06 15:48:33 PST 
Created attachment 244104 [details]
Patch
Comment 3 Simon Fraser (smfr) 2015-01-06 15:49:43 PST 
Comment on attachment 244104 [details]
Patch

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

> Source/WebCore/rendering/RenderGeometryMap.cpp:143
> +//    ASSERT(enclosingIntRect(rendererMappedResult) == enclosingIntRect(FloatQuad(result).boundingBox()));
> +    if (enclosingIntRect(rendererMappedResult) != enclosingIntRect(FloatQuad(result).boundingBox()))
> +        WTFLogAlways("Wah\n");

Ignore this bit.
Comment 4 Tim Horton 2015-01-06 15:51:03 PST 
Comment on attachment 244104 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        color via resetting the custom appearance on the clone. Fix by having

And it's OK to do this because we won't have reflections on custom appearance'd things? Can we add an assertion to that effect?

> Source/WebCore/rendering/RenderGeometryMap.cpp:141
> +//    ASSERT(enclosingIntRect(rendererMappedResult) == enclosingIntRect(FloatQuad(result).boundingBox()));

no.

> Source/WebCore/rendering/RenderGeometryMap.cpp:143
> +        WTFLogAlways("Wah\n");

no.
Comment 5 Simon Fraser (smfr) 2015-01-06 16:07:00 PST 
(In reply to comment #4)
> Comment on attachment 244104 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244104&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        color via resetting the custom appearance on the clone. Fix by having
> 
> And it's OK to do this because we won't have reflections on custom
> appearance'd things? Can we add an assertion to that effect?

We are actually just copying over the custom appearance from the original.
Comment 6 Simon Fraser (smfr) 2015-01-06 16:09:06 PST 
https://trac.webkit.org/r178002