Eliminate MediaPlayer & MediaPlayerClient abstractions(ground work for eliminating rate APIs)

Eliminate MediaPlayer & MediaPlayerClient abstractions(ground work for eliminating rate APIs)

BUG=350571

[Srirama, 2014-05-30 12:25:56]

This patch is to facilitate removal of rate, setRate APIs from
WebMediaPlayerClientImpl.
I have moved the validation of rate value to Webmediaplayer_impl.
This should land before the blink change
https://codereview.chromium.org/304353005/

[acolwell GONE FROM CHROMIUM, 2014-06-02 22:37:02]

https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
content/renderer/media/webmediaplayer_impl.cc:405: if (rate == playback_rate_)
Do we really need this? Shouldn't the caller in Blink simply not make the call
to setRate() if rate is identical?

[Srirama, 2014-06-03 11:10:21]

On 2014/06/02 22:37:02, acolwell wrote:
>
https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
> File content/renderer/media/webmediaplayer_impl.cc (right):
> 
>
https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
> content/renderer/media/webmediaplayer_impl.cc:405: if (rate == playback_rate_)
> Do we really need this? Shouldn't the caller in Blink simply not make the call
> to setRate() if rate is identical?

I just thought instead of querying webmediaplayer_impl for rate value and
compare before calling setRate, if we move the condition to webmediaplayer_impl
we need not have two function calls.
If this is not required i will add the conditions at the blink side itself.
Please suggest.

[acolwell GONE FROM CHROMIUM, 2014-06-03 16:32:53]

On 2014/06/03 11:10:21, Srirama wrote:
> On 2014/06/02 22:37:02, acolwell wrote:
> >
>
https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
> > File content/renderer/media/webmediaplayer_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
> > content/renderer/media/webmediaplayer_impl.cc:405: if (rate ==
playback_rate_)
> > Do we really need this? Shouldn't the caller in Blink simply not make the
call
> > to setRate() if rate is identical?
> 
> I just thought instead of querying webmediaplayer_impl for rate value and
> compare before calling setRate, if we move the condition to
webmediaplayer_impl
> we need not have two function calls.
> If this is not required i will add the conditions at the blink side itself.
> Please suggest.

But this would mean that all WebMediaPlayer implementations would have to have
this same logic. The WebMediaPlayer should always just mirror what
HTMLMediaElement set last. You shouldn't have to query WebMediaPlayer for its
last value. HTMLMediaElement should always know what the last rate it set was
because it was the one that set it last.

[Srirama, 2014-06-03 17:08:47]

On 2014/06/03 16:32:53, acolwell wrote:
> On 2014/06/03 11:10:21, Srirama wrote:
> > On 2014/06/02 22:37:02, acolwell wrote:
> > >
> >
>
https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
> > > File content/renderer/media/webmediaplayer_impl.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/303403002/diff/20001/content/renderer/media/w...
> > > content/renderer/media/webmediaplayer_impl.cc:405: if (rate ==
> playback_rate_)
> > > Do we really need this? Shouldn't the caller in Blink simply not make the
> call
> > > to setRate() if rate is identical?
> > 
> > I just thought instead of querying webmediaplayer_impl for rate value and
> > compare before calling setRate, if we move the condition to
> webmediaplayer_impl
> > we need not have two function calls.
> > If this is not required i will add the conditions at the blink side itself.
> > Please suggest.
> 
> But this would mean that all WebMediaPlayer implementations would have to have
> this same logic. The WebMediaPlayer should always just mirror what
> HTMLMediaElement set last. You shouldn't have to query WebMediaPlayer for its
> last value. HTMLMediaElement should always know what the last rate it set was
> because it was the one that set it last.

Thanks for the explanation. I got it now.