[webkit-reviews] review requested: [Bug 35415] [Chromium] Create WebKit API for WebCore::ImageDecoder : [Attachment 50123] patch 4

[bugzilla-daemon at webkit.org]

Kavita Kanetkar <kkanetkar at chromium.org> has asked  for review:
Bug 35415: [Chromium] Create WebKit API for WebCore::ImageDecoder
https://bugs.webkit.org/show_bug.cgi?id=35415

Attachment 50123: patch 4
https://bugs.webkit.org/attachment.cgi?id=50123&action=review

------- Additional Comments from Kavita Kanetkar <kkanetkar at chromium.org>

> Index: WebKit/chromium/src/WebImageDecoder.cpp
...
> +void WebImageDecoder::init(Type type)
> +{
> +    m_private = 0;
> +    if (type == TypeBMP)
> +	   m_private = static_cast<ImageDecoder*>(new BMPImageDecoder());
> +    else if (type == TypeICO)
> +	   m_private = new ICOImageDecoder();

nit: please use a switch/case above so that if the enum gets a new value,
the compiler will complain about an unimplemented case in the switch
statement.

I am using VS cpp compiler and the compiler didn't give me any warning about
unimplemented switch case. Am I suppressing warnings some how?

> +    WEBKIT_API void setData(const WebData* data, bool allDataReceived);

normally, WebData is passed using a "const WebData&".  why make it a
pointer instead?

The underlying WebCore::ImageDecoder's setData() was taking a pointer. Changed
it now to take a &

Thanks,
Kavita.