RESOLVED FIXED 37310
[Chromium] Support HTML5 <progress> element on Linux. 
https://bugs.webkit.org/show_bug.cgi?id=37310
Summary [Chromium] Support HTML5 <progress> element on Linux.
Hajime Morrita
Reported 2010-04-08 23:15:35 PDT
Porting <progress> to Chromium Linux. Note that chromium has platform-specific rendering code. So we need bugs for each platform.
Attachments
patch v0 (27.27 KB, patch)
2010-06-01 18:42 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
patch v1 (10.75 KB, patch)
2010-06-02 00:02 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
patch v2 (11.03 KB, patch)
2010-06-02 17:53 PDT, Hajime Morrita 		tkent: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2010-06-01 18:42:22 PDT
Created attachment 57613 [details] patch v0
Hajime Morrita
Comment 2 2010-06-01 18:48:26 PDT
A change on Chromium side is here: http://codereview.chromium.org/2478002
Kent Tamura
Comment 3 2010-06-01 19:12:29 PDT
Comment on attachment 57613 [details] patch v0 Please update WebKit source and regenerate the patch. Could you split the patch into two parts? graphics/skia change and RenderThemeChromium* change. WebCore/ChangeLog:12 + an image resampling mode. across multiple paintSkBitmap() calls. You probably want to remove "." after "mode"? WebCore/platform/graphics/skia/PlatformContextSkia.cpp:648 + const float kFractionalChangeThreshold = 0.025f; We don't prepend "k" to constant values though I know you didn't write this code. r- because of svn-apply failure.
Hajime Morrita
Comment 4 2010-06-02 00:02:40 PDT
Created attachment 57630 [details] patch v1
Hajime Morrita
Comment 5 2010-06-02 00:05:05 PDT
Hi, Kent-san thank you for reviewing. I updated and rebased the patch > Could you split the patch into two parts? graphics/skia change and RenderThemeChromium* change. OK. I split skia change out to Bug 40045, whose patch will come shortly. Other feedback will be taken on that patch.
Kent Tamura
Comment 6 2010-06-02 06:58:55 PDT
Comment on attachment 57630 [details] patch v1 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.
Hajime Morrita
Comment 7 2010-06-02 17:53:25 PDT
Created attachment 57720 [details] patch v2
Hajime Morrita
Comment 8 2010-06-02 17:55:40 PDT
Hi Kent-san, thank you for reviewing! I updated the patch. (In reply to comment #6) > (From update of attachment 57630 [details]) > WebCore/rendering/RenderThemeChromiumSkia.cpp:775 > + static const int progressDeltaPixelsPerSecond = 100; > Please add comments about how did you determine these values. Added comment. > WebCore/rendering/RenderThemeChromiumSkia.cpp:810 > + return progressAnimationInterval*progressAnimationFrmaes*2; // "2" for back and forth > Add spaces around *. Done. > > > 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. Done. > > > WebCore/rendering/RenderThemeChromiumSkia.h:126 > + IntRect determinateProgressValueRectFor(RenderProgress*, const IntRect&) const; > We should make these internal functions to private or protected. Moved to protected.
Hajime Morrita
Comment 9 2010-06-02 21:46:54 PDT
Committed r60604: <http://trac.webkit.org/changeset/60604>
Note You need to log in before you can comment on or make changes to this bug.