[webkit-reviews] review denied: [Bug 31586] [GTK] set playbin mute property depending on volume value : [Attachment 47533] new API in MediaPlayer for mute control

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 31586: [GTK] set playbin mute property depending on volume value
https://bugs.webkit.org/show_bug.cgi?id=31586

Attachment 47533: new API in MediaPlayer for mute control
https://bugs.webkit.org/attachment.cgi?id=47533&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
> +    virtual bool supportsMuting() const { return false; }
> +    virtual void setMuted(bool) { }
> 
These methods are unnecessary because MediaPlayerPrivateInterface has default
implementations. I know that many other methods in NullMediaPlayerPrivate use
this pattern, but we should change the pattern (and come back and clean this
up).

> diff --git a/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h 
> diff --git a/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm 
> diff --git a/WebCore/platform/graphics/qt/MediaPlayerPrivatePhonon.cpp 
> diff --git a/WebCore/platform/graphics/qt/MediaPlayerPrivatePhonon.h 
> diff --git a/WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeWin.cpp

> diff --git a/WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeWin.h 
> diff --git a/WebCore/platform/graphics/wince/MediaPlayerPrivateWince.h 
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> diff --git a/WebKit/chromium/public/WebMediaPlayerClient.h 
> diff --git a/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp 
> diff --git a/WebKit/chromium/src/WebMediaPlayerClientImpl.h 
None of these is necessary as each media engine subclasses
MediaPlayerPrivateInterface, which has default implementation for both methods.
I would prefer that we leave them out for now and add them when a port adds the
functionality.