[webkit-reviews] review denied: [Bug 30462] MediaPlayer doesn't set properties on new privates : [Attachment 41825] patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Benjamin Otte
<otte at gnome.org>'s request for review:
Bug 30462: MediaPlayer doesn't set properties on new privates
https://bugs.webkit.org/show_bug.cgi?id=30462

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

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
>      LOG_VERBOSE(Media, "Seek: %" GST_TIME_FORMAT, GST_TIME_ARGS(sec));
> -    if (!gst_element_seek( m_playBin, m_rate,
> +    if (!gst_element_seek( m_playBin, m_player->rate(),
			     ^
You could take the oportunity to fix this weird space here =).

> -    g_object_set(G_OBJECT(m_playBin), "mute", mute, NULL);
> +    g_object_set(G_OBJECT(m_playBin), "volume", (double) volume, NULL);

You should use static_cast<double>() here, mind the space.

> @@ -673,12 +657,11 @@ void MediaPlayerPrivate::paint(GraphicsContext*
context, const IntRect& rect)
>  
>      // paint the rectangle on the context and draw the surface inside.
>      cairo_translate(cr, rect.x() + gapWidth, rect.y() + gapHeight);
> -    cairo_rectangle(cr, 0, 0, rect.width(), rect.height());
>      cairo_scale(cr, doublePixelAspectRatioNumerator /
doublePixelAspectRatioDenominator,
>		   doublePixelAspectRatioDenominator /
doublePixelAspectRatioNumerator);
>      cairo_scale(cr, scale, scale);
>      cairo_set_source_surface(cr, src, 0, 0);
> -    cairo_fill(cr);
> +    cairo_paint(cr);
>      cairo_restore(cr);

These changes look unrelated, can they go in a separate patch?

> -	       void setMuted(bool);

Just so it's crystal clear: this is being removed because the mute logic is
already handled by HTMLMediaElement? I'll say r- mainly because of the
unrelated changes above, but otherwise looks fine to me, so should be good to
land after fixing the nits.