[webkit-reviews] review denied: [Bug 39370] [chromium] Fix slider status when buffering : [Attachment 56511] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Victoria <vrk at google.com>'s request
for review:
Bug 39370: [chromium] Fix slider status when buffering
https://bugs.webkit.org/show_bug.cgi?id=39370

Attachment 56511: Patch
https://bugs.webkit.org/attachment.cgi?id=56511&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Thanks Alpha for looking it over.

Just a few minor nits to address.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog

> +
> +	   * rendering/RenderMediaControlsChromium.cpp:
> +	   (WebCore::paintMediaSlider):
> +	   Added logic to align the buffering bar with the thumb. Half of the
thumb image is transparent, so the buffer bar is adjusted to fill in this gap.

ChangeLog entries tend to wrap text somewhere around 80 columns.


> diff --git a/WebCore/rendering/RenderMediaControlsChromium.cpp
b/WebCore/rendering/RenderMediaControlsChromium.cpp
> +static Image* getMediaSliderThumb()
> +{
> +    static Image* mediaSliderThumb  = platformResource("mediaSliderThumb");

Two spaces before =

> +
> +    double bufferedWidth = 0.0;
> +    if (mediaElement->percentLoaded() > 0.0) {
> +	   // Account for width of slider thumb

Please add a period to the end of the comment.

> +	   Image* mediaSliderThumb = getMediaSliderThumb();
> +	   double thumbWidth = mediaSliderThumb->width() / 2.0 + 1.0;
> +	   double rectWidth = bufferedRect.width() - thumbWidth;
> +	   if (rectWidth < 0)
> +	       rectWidth = 0;

I'm not sure why you initialized "double bufferedWidth" with 0.0 above but are
using 0 here. (It doesn't matter but inconsistency raises flags for me.)