[Webkit-unassigned] [Bug 37308] [Chromium] Support HTML5 <progress> element on Windows.

[bugzilla-daemon at webkit.org]

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





--- Comment #15 from MORITA Hajime <morrita at google.com>  2010-04-21 18:32:12 PST ---
Darin, thank you for reviewing!
I updated the patch.

> > +    WebCore::ThemePainter painter(i.context, r);
> 
> nit: no need for the WebCore:: prefix when you are in the WebCore namespace.
Fixed.

> > +    ChromiumBridge::paintProgressBar(painter.context(),
> > +                                     PP_BAR,
> > +                                     0,
> > +                                     r, 
> > +                                     fillPart,
> > +                                     0,
> > +                                     fillRect);
> 
> It looks like barState and fillState are not used.  Should those
> parameters be excluded?  Also, is there any reason to pass PP_BAR
> given that it is a constant?  It seems like the callee would know
> to pass PP_BAR on to Windows.
Agreed and fixed.
I try to align args to other APIs. But it didn't make sense in this case.

> > +    virtual void paintProgressBar(
> > +        WebCanvas*, int barPart, int barState, const WebRect& barRect, 
> > +        int fillPart, int fillState, const WebRect& fillRect) {} // FIXME: make pure virtual after implementation landed.
> >  };
> 
> ^^^ we should actually make any embedder implemented methods non-pure.
> the old way was for them to be pure, but that makes it harder to change
> the API.  the new way is to make methods like these non-pure.
I see. So I removed the FIXME.
We might need to export notImplemented() for such case.

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