Bug 45335 - Scrollbars fail to render in composited iframes.
Summary: Scrollbars fail to render in composited iframes.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-07 16:38 PDT by Simon Fraser (smfr)
Modified: 2010-09-09 17:03 PDT (History)
7 users (show)

See Also:


Attachments
Patch (17.87 KB, patch)
2010-09-09 14:08 PDT, Simon Fraser (smfr)
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-09-07 16:38:26 PDT
On Windows, scrollbars don't render in iframes that fall into compositing layers. For example, see LayoutTests/compositing/iframes/composited-parent-iframe.html
Comment 1 Nat Duca 2010-09-07 17:08:18 PDT
Tying to chrome bug:
 http://code.google.com/p/chromium/issues/detail?id=54758
Comment 2 Nat Duca 2010-09-07 17:09:40 PDT
Or rather, that's just a potentially related bug. :)
Comment 3 Simon Fraser (smfr) 2010-09-07 18:17:09 PDT
It looks like the ScrollbarThemeWIn is drawing the scrollbar using the HDC from the window, rather than the compositing layer.
Comment 4 Simon Fraser (smfr) 2010-09-09 14:08:12 PDT
Created attachment 67095 [details]
Patch
Comment 5 Adam Roben (:aroben) 2010-09-09 14:16:06 PDT
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.
Comment 6 Simon Fraser (smfr) 2010-09-09 16:32:22 PDT
http://trac.webkit.org/changeset/67125
Comment 7 WebKit Review Bot 2010-09-09 17:03:19 PDT
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