Porting <progress> to Chromium Linux. Note that chromium has platform-specific rendering code. So we need bugs for each platform.
Created attachment 57613 [details] patch v0
A change on Chromium side is here: http://codereview.chromium.org/2478002
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.
Created attachment 57630 [details] patch v1
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.
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.
Created attachment 57720 [details] patch v2
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.
Committed r60604: <http://trac.webkit.org/changeset/60604>