Bug 45335 – Scrollbars fail to render in composited iframes.

• Component: Frames
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

On Windows, scrollbars don't render in iframes that fall into compositing layers. For example, see LayoutTests/compositing/iframes/composited-parent-iframe.html

[Nat Duca]

Tying to chrome bug:
 http://code.google.com/p/chromium/issues/detail?id=54758

[Nat Duca]

Or rather, that's just a potentially related bug. :)

[Simon Fraser (smfr)]

It looks like the ScrollbarThemeWIn is drawing the scrollbar using the HDC from the window, rather than the compositing layer.

[Simon Fraser (smfr)]

Created attachment 67095 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 67095 [details]
Patch

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

It would be nice to split this into two patches: one to add and use LocalWindowsContext, and one to fix get/releaseWindowsContext to work when there's no HDC.

You also need to patch the releaseWindowsContext implementation in GraphicsContextCairoWin.cpp.

> WebCore/platform/graphics/GraphicsContext.h:360
> +        class LocalWindowsContext {
It seems a little unfortunate to put this in GraphicsContext.h, since very few files will use this class but lots and lots of files will use GraphicsContext. Putting it in its own header file would drastically reduce the number of files that have to recompile when we change this class.

Should this class be Noncopyable?

> WebCore/platform/graphics/GraphicsContext.h:369
> +            LocalWindowsContext(GraphicsContext* graphicsContext, const IntRect& rect, bool supportAlphaBlend = true, bool mayCreateBitmap = true)
> +                : m_graphicsContext(graphicsContext)
> +                , m_hdc(0)
> +                , m_rect(rect)
> +                , m_supportAlphaBlend(supportAlphaBlend)
> +                , m_mayCreateBitmap(mayCreateBitmap)
> +            {
> +                m_hdc = m_graphicsContext->getWindowsContext(m_rect, m_supportAlphaBlend, m_mayCreateBitmap);
We usually try not to set members twice in constructors. You could either use the parameters to call getWindowsContext, or move m_hdc after the other members and use the members to call getWindowsContext.

The code changes seem fine, so r+. But do consider these comments.

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/67125

[WebKit Review Bot]

http://trac.webkit.org/changeset/67125 might have broken Qt Windows 32-bit Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/67123
http://trac.webkit.org/changeset/67124
http://trac.webkit.org/changeset/67125