Bug 180431 – [Win] Scrollbars are sometimes not rendered in accelerated compositing mode.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Per Arne Vollan]

Resizing the window will bring them back again.

[Per Arne Vollan]

<rdar://problem/34531264>

[Per Arne Vollan]

Created attachment 328464 [details]
Patch

[Brent Fulgham]

Comment on attachment 328464 [details]
Patch

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

I think this looks good, but I would like to hear from Alan or Simon on the early return int he RenderLayerCompositer code. It sure sounds reasonable, but I don't know that code very well.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2884
> +        return true;

Let's double-check this with Alan or Simon.

> Source/WebKitLegacy/win/WebView.h:573
> +    void paintContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, WebCore::GraphicsLayerPaintingPhase, const WebCore::FloatRect& inClip, WebCore::GraphicsLayerPaintBehavior) override;

So this happened because the GraphicsLayerClient Api changed and we didn't notice?

[EWS Watchlist]

Comment on attachment 328464 [details]
Patch

Attachment 328464 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5503183

New failing tests:
tiled-drawing/scrolling/frames/fixed-inside-frame.html
compositing/iframes/connect-compositing-iframe.html
compositing/iframes/overlapped-iframe.html
compositing/visible-rect/iframe-with-layers-outside-viewport.html
compositing/iframes/scrolling-iframe.html
compositing/iframes/become-overlapped-iframe.html
compositing/visible-rect/iframe-and-layers.html
compositing/iframes/composited-parent-iframe.html
compositing/iframes/invisible-nested-iframe-show.html
compositing/iframes/connect-compositing-iframe-delayed.html
compositing/iframes/page-cache-layer-tree.html
compositing/iframes/resizer.html
compositing/iframes/connect-compositing-iframe2.html
compositing/iframes/iframe-resize.html
compositing/iframes/connect-compositing-iframe3.html
compositing/iframes/enter-compositing-iframe.html

[EWS Watchlist]

Created attachment 328476 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Per Arne Vollan]

Created attachment 328477 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #3)
> Comment on attachment 328464 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=328464&action=review
> 
> I think this looks good, but I would like to hear from Alan or Simon on the
> early return int he RenderLayerCompositer code. It sure sounds reasonable,
> but I don't know that code very well.
> 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:2884
> > +        return true;
> 
> Let's double-check this with Alan or Simon.
> 
> > Source/WebKitLegacy/win/WebView.h:573
> > +    void paintContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, WebCore::GraphicsLayerPaintingPhase, const WebCore::FloatRect& inClip, WebCore::GraphicsLayerPaintBehavior) override;
> 
> So this happened because the GraphicsLayerClient Api changed and we didn't
> notice?

No, this was a drive-by fix, and not actually needed.

Thanks for reviewing!

[Simon Fraser (smfr)]

Comment on attachment 328477 [details]
Patch

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

> Source/WebKitLegacy/win/ChangeLog:9
> +        Make sure virtual methods are actually overridden by adding the 'override' keyword.

What you're really saying here is "fix the signatures of GraphicsLayerClient that should have been overridden, and use 'override' to make sure this doesn't break in future.

[Per Arne Vollan]

(In reply to Simon Fraser (smfr) from comment #8)
> Comment on attachment 328477 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=328477&action=review
> 
> > Source/WebKitLegacy/win/ChangeLog:9
> > +        Make sure virtual methods are actually overridden by adding the 'override' keyword.
> 
> What you're really saying here is "fix the signatures of GraphicsLayerClient
> that should have been overridden, and use 'override' to make sure this
> doesn't break in future.

Thanks Simon! I'll update the patch :)

[Per Arne Vollan]

Created attachment 328478 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 328478 [details]
Patch

Why is this not testable?

[Per Arne Vollan]

(In reply to Simon Fraser (smfr) from comment #11)
> Comment on attachment 328478 [details]
> Patch
> 
> Why is this not testable?

This should not be very hard to create a test case for, but I believe it is already covered.

[Per Arne Vollan]

Committed r225535: <https://trac.webkit.org/changeset/225535/webkit>.