[webkit-reviews] review granted: [Bug 45335] Scrollbars fail to render in composited iframes. : [Attachment 67095] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 45335: Scrollbars fail to render in composited iframes.
https://bugs.webkit.org/show_bug.cgi?id=45335

Attachment 67095: Patch
https://bugs.webkit.org/attachment.cgi?id=67095&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
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.