Bug 58535 – REGRESSION (r83820): Lots of compositing tests failing on Windows 7 Release (Tests)

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Roben (:aroben)]

Lots of compositing tests have been failing on Windows 7 Release (Tests) since r83820. See the URL for the full list and failure diffs.

[Adam Roben (:aroben)]

I suspect we just need to update the Windows results for these tests.

[Adam Roben (:aroben)]

The Windows results seem to have extra scrollbar layers compared to Mac. I think this is because frames use native scrollbars on Mac, which can't be put into layers.

[Adam Roben (:aroben)]

Checked in expected Windows results. James, can you confirm that the new results are correct? If so we can close this bug.

Committed r83856: <http://trac.webkit.org/changeset/83856>

[James Robinson]

I will take a look. Just to be sure, do FrameViews on windows use platform widgets?

Sorry I missed this bot when checking baselines!

[James Robinson]

Oops, this has found a real bug!  The intention with r83820 was that new scrollbar layers would be created on chromium always and on the apple ports only when overlay scrollbars were in use (as that's the behavior Simon wanted for them), but I guarded the behavior with #if PLATFORM(MAC).  I'll have to change these:

http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayerBacking.cpp?rev=83820#L568
http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayerBacking.cpp?rev=83820#L577
http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayerBacking.cpp?rev=83820#L586
http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp?rev=83820#L1469

and then revert the baseline updates.  I'll get to it in a hour or two if nobody beats me to it.

[Adam Roben (:aroben)]

(In reply to comment #4)
> Just to be sure, do FrameViews on windows use platform widgets?

No.

Thank you for looking into this!

[James Robinson]

Created attachment 89630 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 89630 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerBacking.cpp:571
>  #endif

Now I wonder if these aren't all backwards. You want scrollbars layers in Chromium. Mac/Windows _only_ wants them if there are overlay scrollbars. So I think the logic is wrong.

[James Robinson]

Comment on attachment 89630 [details]
Patch

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

>> Source/WebCore/rendering/RenderLayerBacking.cpp:571
>>  #endif
> 
> Now I wonder if these aren't all backwards. You want scrollbars layers in Chromium. Mac/Windows _only_ wants them if there are overlay scrollbars. So I think the logic is wrong.

On chromium this function returns true iff there is a horizontal scrollbar.  On non-chromium this function returns true iff the page is using overlay scrollbars _and_ there is a horizontal scrollbar.  Maybe there are too many negations here to be clear?

[Simon Fraser (smfr)]

Comment on attachment 89630 [details]
Patch

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

>>> Source/WebCore/rendering/RenderLayerBacking.cpp:571
>>>  #endif
>> 
>> Now I wonder if these aren't all backwards. You want scrollbars layers in Chromium. Mac/Windows _only_ wants them if there are overlay scrollbars. So I think the logic is wrong.
> 
> On chromium this function returns true iff there is a horizontal scrollbar.  On non-chromium this function returns true iff the page is using overlay scrollbars _and_ there is a horizontal scrollbar.  Maybe there are too many negations here to be clear?

I was confused by the early return. So this is OK.

[James Robinson]

Committed r83887: <http://trac.webkit.org/changeset/83887>