[webkit-reviews] review granted: [Bug 28272] WINCE PORT: graphics files only for WINCE : [Attachment 72813] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 28272: WINCE PORT: graphics files only for WINCE
https://bugs.webkit.org/show_bug.cgi?id=28272

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

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=72813&action=review

> WebCore/platform/graphics/wince/SharedBitmap.cpp:54
> +    if (result) {

This could be an early return instead.

> WebCore/platform/graphics/wince/SharedBitmap.cpp:79
> +    m_pixelData.set(new unsigned[bufferSize]);

I think the more modern way to do this is:

m_pixelData = adoptArrayPtr(new unsigned[bufferSize]);

> WebCore/platform/graphics/wince/SharedBitmap.cpp:176
> +	   m_hbitmap.clear();

Some people have started to use: m_hbitmap = nullptr;

I don't know if that's the recommended style for all new code.

> WebCore/platform/graphics/wince/SharedBitmap.cpp:199
> +    OwnPtr<HBITMAP> hbmp(CreateDIBSection(0, bmpInfo, DIB_RGB_COLORS,
pixels, 0, 0));

You could write this as: OwnPtr<HBITMAP> hbmp =
adoptPtr(CreateDIBSection(...));

I don't know whether that's the recommended style for all new code. I find it
slightly more readable.

> WebCore/platform/graphics/wince/SharedBitmap.cpp:296
> +HBITMAP SharedBitmap::clipBitmap(const IntRect& rect, bool useAlpha,
BitmapInfo& bmpInfo, void*& pixels)

This could return a PassOwnPtr<HBITMAP>.

> WebCore/platform/graphics/wince/SharedBitmap.cpp:309
> +    HBITMAP newBmp = CreateDIBSection(0, &bmpInfo, DIB_RGB_COLORS, &pixels,
0, 0);

...and you could store this in an OwnPtr.

> WebCore/platform/graphics/wince/SharedBitmap.cpp:343
> +    return newBmp;

This should be: return newBmp.release();

> WebCore/platform/graphics/wince/SharedBitmap.h:146
> +#endif // SharedBitmap_h
>  \ No newline at end of file

You should add a newline.