[Webkit-unassigned] [Bug 27561] ImageDecoder enhancements for WINCE port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27561


Yong Li <yong.li at torchmobile.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |27511




--- Comment #4 from Yong Li <yong.li at torchmobile.com>  2009-07-22 14:50:51 PDT ---

(In reply to comment #2)
> A few comments:
> 
> * Can you break this into distinct patches for the different pieces here?  This
> patch is very large and hard to review.

I will do it.

> 
> * You don't include ImageFrameSink.*, so no one can actually compile or use
> this.

ImageFrameSink.* is in bug 27511, still being under review

> 
> * "#if PLATFORM (WINCE)" and similar all over, especially with "#else <existing
> code>", really sucks for readability.  Making this worse is the fact that
> frequently the two blocks of code are very similar or even share many common
> parts, and differ only slightly.  A combination of helper functions and
> subclasses seems like it could do this much more readably.

Just don't want to affect other platforms :)

> 
> * goto?  Really?  It doesn't look necessary at all (a helper function plus a
> "continue;" would have served just as well).
> 
> Overall it seems like these capabilities should be added to the existing code
> in a way that blends more seamlessly and is usable by any port.  If I can
> manage to get back to working on Image system cleanup, maybe we can find a way
> of doing that stepwise.

I don't except that this patch can be merged soon. Agree with that more cleanup
is necessary to finally merge it up. Just hope that people can see it, and
discuss on this topic, and finally dig out a way how to make it more common.

Thanks for your quick response.

-Yong

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.