| Summary: | REGRESSION (r83820): Lots of compositing tests failing on Windows 7 Release (Tests) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | jamesr, simon.fraser | ||||
| Priority: | P2 | Keywords: | LayoutTestFailure, PlatformOnly | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| URL: | http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r83820%20(11621)/results.html | ||||||
| Attachments: |
|
||||||
|
Description
Adam Roben (:aroben)
2011-04-14 06:00:07 PDT
I suspect we just need to update the Windows results for these tests. 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. 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> I will take a look. Just to be sure, do FrameViews on windows use platform widgets? Sorry I missed this bot when checking baselines! 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. (In reply to comment #4) > Just to be sure, do FrameViews on windows use platform widgets? No. Thank you for looking into this! Created attachment 89630 [details]
Patch
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. 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? 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. Committed r83887: <http://trac.webkit.org/changeset/83887> |