[webkit-reviews] review denied: [Bug 37310] [Chromium] Support HTML5 <progress> element on Linux. : [Attachment 57630] patch v1

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 37310: [Chromium] Support HTML5 <progress> element on Linux.
https://bugs.webkit.org/show_bug.cgi?id=37310

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
WebCore/rendering/RenderThemeChromiumSkia.cpp:775
 +  static const int progressDeltaPixelsPerSecond = 100;
Please add comments about how did you determine these values.


WebCore/rendering/RenderThemeChromiumSkia.cpp:810
 +	return progressAnimationInterval*progressAnimationFrmaes*2; // "2" for
back and forth
Add spaces around *.


WebCore/rendering/RenderThemeChromiumSkia.cpp:826
 +	paintInfo.context->drawTiledImage(barImage,
renderObject->style()->colorSpace(), rect, IntPoint(0, 0), barTileSize);
We had better assign renderObject->style()->colorSpace() to a variable because
it is used multiple times.


WebCore/rendering/RenderThemeChromiumSkia.h:126
 +	    IntRect determinateProgressValueRectFor(RenderProgress*, const
IntRect&) const;
We should make these internal functions to private or protected.