Bug 39370

Summary: [chromium] Fix slider status when buffering
Product: WebKit Reporter: Victoria Kirst <vrk>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hclam, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Description Victoria Kirst 2010-05-19 11:52:25 PDT
When you load a video, the slider shows the entire video has been buffered, when only a small portion of it has been buffered.
Comment 1 Victoria Kirst 2010-05-19 12:40:32 PDT
Created attachment 56511 [details]
Patch
Comment 2 Hin-Chung Lam 2010-05-19 12:45:47 PDT
Victoria is working with me on HTML5 video in Chromium. This change looks good to me.
Comment 3 David Levin 2010-05-19 16:19:03 PDT
Comment on attachment 56511 [details]
Patch

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.)
Comment 4 Victoria Kirst 2010-05-20 09:57:58 PDT
Created attachment 56604 [details]
Patch
Comment 5 WebKit Commit Bot 2010-05-21 07:44:18 PDT
Comment on attachment 56604 [details]
Patch

Clearing flags on attachment: 56604

Committed r59932: <http://trac.webkit.org/changeset/59932>
Comment 6 WebKit Commit Bot 2010-05-21 07:44:29 PDT
All reviewed patches have been landed.  Closing bug.