[webkit-reviews] review requested: [Bug 35415] [Chromium] Create WebKit API for WebCore::ImageDecoder : [Attachment 50123] patch 4
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Mar 5 14:44:00 PST 2010
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.
More information about the webkit-reviews
mailing list