Always notify the MediaPlayer of any seek.

Always notify the MediaPlayer of any seek.

This change removes short circuit logic for seeking to the current position.
There are certain situations, like seeking to a truncated duration, where the
HTMLMediaElement is not in the best position to determine whether a call to
the MediaPlayer is necessary or not. This change allows the
MediaPlayer/WebMediaPlayer implementation to determine what the best course
of action should be. This also makes the seeking behavior more spec
compliant because even a seek to the current position should have the
seeking attribute be true until a stable state occurs. This is guaranteed
when calling into the MediaPlayer and allows all seeks to be treated the
same at the HTMLMediaElement level.

BUG=266631
TEST=LayoutTests/media/video-seek-to-current-position.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179347

[Srirama, 2014-07-31 16:50:37]

This is to land acolwell's patch https://codereview.chromium.org/340553004/
Regarding the failed test cases:
The test cases change the playback rate and check for current time to make sure
current time is not zero. And the test cases depend on timeupdate event. Looks
like the timeupdate event is getting fired before the actual play starts and
current time gets updated. So after analyzing i found that playing event is
reliable for checking timeupdate. Playing event makes sure that current time is
updated.
PTAL

[Srirama, 2014-07-31 18:12:20]

On 2014/07/31 16:50:37, Srirama wrote:
> This is to land acolwell's patch https://codereview.chromium.org/340553004/
> Regarding the failed test cases:
> The test cases change the playback rate and check for current time to make
sure
> current time is not zero. And the test cases depend on timeupdate event. Looks
> like the timeupdate event is getting fired before the actual play starts and
> current time gets updated. So after analyzing i found that playing event is
> reliable for checking timeupdate. Playing event makes sure that current time
is
> updated.
> PTAL

small correction in the previous comment:
Looks like the timeupdate event is getting fired before the actual play starts
and
current time doesn't get updated as expected by test case.

[Srirama, 2014-07-31 18:21:52]

The CQ bit was checked by srirama.m@samsung.com

[Srirama, 2014-07-31 18:21:54]

The CQ bit was unchecked by srirama.m@samsung.com

[acolwell GONE FROM CHROMIUM, 2014-07-31 18:55:53]

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-effective-playback-rate.html (right):

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
LayoutTests/media/media-controller-effective-playback-rate.html:31:
waitForEventOnce("play", play);
nit: Why not simply wait for playing here instead of play?

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-playbackrate.html (right):

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
LayoutTests/media/media-controller-playbackrate.html:35:
waitForEventOnce("playing", playing);
nit: Just inline the function here. You don't really need to create a variable
for it since the function is only used once.

[Srirama, 2014-07-31 19:37:26]

PTAL

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-effective-playback-rate.html (right):

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
LayoutTests/media/media-controller-effective-playback-rate.html:31:
waitForEventOnce("play", play);
On 2014/07/31 18:55:53, acolwell wrote:
> nit: Why not simply wait for playing here instead of play?

Done.

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-playbackrate.html (right):

https://codereview.chromium.org/431903003/diff/20001/LayoutTests/media/media-...
LayoutTests/media/media-controller-playbackrate.html:35:
waitForEventOnce("playing", playing);
On 2014/07/31 18:55:53, acolwell wrote:
> nit: Just inline the function here. You don't really need to create a variable
> for it since the function is only used once.

Done.

[acolwell GONE FROM CHROMIUM, 2014-07-31 20:00:16]

lgtm

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-effective-playback-rate.html (right):

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
LayoutTests/media/media-controller-effective-playback-rate.html:31:
waitForEventOnce("playing", function() {
nit: { on next line.

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-playbackrate.html (right):

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
LayoutTests/media/media-controller-playbackrate.html:35:
waitForEventOnce("playing", function() {
ditto

[Srirama, 2014-07-31 20:20:18]

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-effective-playback-rate.html (right):

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
LayoutTests/media/media-controller-effective-playback-rate.html:31:
waitForEventOnce("playing", function() {
On 2014/07/31 20:00:16, acolwell wrote:
> nit: { on next line.

Done.

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
File LayoutTests/media/media-controller-playbackrate.html (right):

https://codereview.chromium.org/431903003/diff/60001/LayoutTests/media/media-...
LayoutTests/media/media-controller-playbackrate.html:35:
waitForEventOnce("playing", function() {
On 2014/07/31 20:00:16, acolwell wrote:
> ditto

Done.

[Srirama, 2014-07-31 20:20:30]

The CQ bit was checked by srirama.m@samsung.com

[commit-bot: I haz the power, 2014-07-31 20:21:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/srirama.m@samsung.com/431903003/80001

[commit-bot: I haz the power, 2014-07-31 21:17:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[commit-bot: I haz the power, 2014-07-31 21:41:50]

Change committed as 179347

[alancutter (OOO until 2018), 2014-08-01 05:57:07]

A revert of this CL has been created in
https://codereview.chromium.org/431273003/ by alancutter@chromium.org.

The reason for reverting is: Test now flakily crashing on all platforms after
this change:
http/tests/media/media-source/mediasource-duration.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=htt...

Example build:
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34873

BUG=399523.

[Srirama, 2014-08-08 06:56:09]

On 2014/08/01 05:57:07, alancutter wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/431273003/ by mailto:alancutter@chromium.org.
> 
> The reason for reverting is: Test now flakily crashing on all platforms after
> this change:
> http/tests/media/media-source/mediasource-duration.html
> 
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=htt...
> 
> Example build:
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34873
> 
> BUG=399523.

Initially i didn't understand what exactly is the problem. I was confused a bit
because all the bots passed while submitting the patch.
Now i understand that the there is a crash sometimes while running this test as
pointer by alancutter.
From the logs the issue is here
08:17.371 18880   [6:13:0731/210817:656360164:FATAL:audio_splicer.cc(440)] Check
failed: output_bus.
I didn't find any other backtrace info from the logs.
As i understand i need to run the test more than once to get the crash.
Any pointers on whether we can get the backtrace here would help me.

[Srirama, 2014-08-08 16:32:33]

On 2014/08/08 06:56:09, Srirama wrote:
> On 2014/08/01 05:57:07, alancutter wrote:
> > A revert of this CL has been created in
> > https://codereview.chromium.org/431273003/ by
mailto:alancutter@chromium.org.
> > 
> > The reason for reverting is: Test now flakily crashing on all platforms
after
> > this change:
> > http/tests/media/media-source/mediasource-duration.html
> > 
> >
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=htt...
> > 
> > Example build:
> >
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34873
> > 
> > BUG=399523.
> 
> Initially i didn't understand what exactly is the problem. I was confused a
bit
> because all the bots passed while submitting the patch.
> Now i understand that the there is a crash sometimes while running this test
as
> pointer by alancutter.
> From the logs the issue is here
> 08:17.371 18880   [6:13:0731/210817:656360164:FATAL:audio_splicer.cc(440)]
Check
> failed: output_bus.
> I didn't find any other backtrace info from the logs.
> As i understand i need to run the test more than once to get the crash.
> Any pointers on whether we can get the backtrace here would help me.

Looks like the output_bus was not getting filed in
ExtractCrossfadeFromPreSplice() as it may not be entering in some cases. Do we
need to check why this is happening or is it expected and we can avoid the CHECK
for output_bus and other parameters when it is not getting filled?

[DaleCurtis, 2014-08-08 19:21:16]

This may be related to http://crbug.com/400442 which I just landed a fix for. 
Otherwise it could be negatively impacting splice frames in some way.  I'd have
to take a closer look, but I think you're on the right track.

[Srirama, 2014-08-11 06:30:25]

On 2014/08/08 19:21:16, DaleCurtis wrote:
> This may be related to http://crbug.com/400442 which I just landed a fix for. 
> Otherwise it could be negatively impacting splice frames in some way.  I'd
have
> to take a closer look, but I think you're on the right track.

Great, i have verified and it is fixed with your changes. Thanks you so much, i
was wondering how to fix this issue as i don't have much idea on the chromium
side code.
I ran the test case with 1000 iterations (run-webkit-tests -- iterations=1000
mediasource-duration.html) and it is passing all. Before the fix it used to fail
2 or 3 times out of 10 iterations.
I will land this CL again now.