Here's my first prototype using BrowserTests, which is a bit painful since browser test interactions are based on waiting for title changes in C++ code. The mechanism by which it induces suspend is a bit suspect too:

https://chromium-review.googlesource.com/#/c/chromium/src/+/881832

I'll next prototype it as a layout tests using windows.internals methods to control the delegate state. This seems like it may be more promising if our eventual home for the resource scheduler is Blink, but has issues now since it's essentially hole-punching through blink to the content layer. 

https://chromium-review.googlesource.com/#/c/chromium/src/+/882283 here's the layout test based prototype which I find much easier to use.

https://chromium-review.googlesource.com/#/c/chromium/src/+/882319 is based on the layout test version, but emits a "suspended" event for testing purposes. It's much nicer than polling via rAF, but I have no idea what I'm allowed to do in Blink tests.

Mounir: any advice you want to provide would be greatly appreciated :)

the layout test looks better, in my opinion.  it's testing something much closer to the actual use-case than the browser test.

getting rid of as many of the "for testing" methods as possible would make it a much clearer choice.  i never trust them.  :)

I concur on the ForTesting methods, but to fully test our matrix of suspend operations I think we need the ones in my CL. The only optional one is the IsSuspended() one if we choose the event based approach (but even then allows for nice asserts). Frank, did you have an idea for reducing the number of for testing methods?

+foolip for opinions on the patch sets in c#3/c#4 as well since I'll need OWNER approval for additions to Source/core/testing/Internals.*

If/Once mlamouri+foolip are on-board with the layout test approach I'll start crafting a swath of tests which verify our existing behaviors.

FWIW, I think the LT based approach can ditch the setPageVisibilityForMediaElement() API for now, it requires at least a couple of other methods to work since what we do with background elements varies by platform and we'd need a way for the layout test to specify one feature state or another, or craft one set of tests for desktop and one set for android. In either case it's a bit outside of the intent for this bug.

#3/#4 looks pretty good to me, certainly if doing them as LayoutTests is the most convenient then that's nice.

On #3, I guess there isn't anything that a script can do to reliably cause the pipeline to be suspended? Not even putting in a third-party iframe and scrolling out of view? If so then just adding APIs to tickle those bits makes sense.

With #4, is the idea that web developers might have to react to the suspend/resume, because it cannot be made entirely transparent?

You can reliably trigger suspend in a few circumstances but they're all timeout based, on the order of ~10 - 15s, which would be too long for practical layout tests -- so we'd at least need a timeout setter, at which point having an explicit suspend seems like a better test primitive.

For #4, the problem is that it's entirely transparent, so to test I need either a rAF polling isSuspended to detect the suspend or emit a new test only 'suspended' event to avoid polling in the tests. I definitely wouldn't try to emit that event in Chrome proper.

Sorry for the delay dalecurtis@. My comments:

#2 - sgtm if we can guarantee stability and the tests are not too slow. I'm not really opposed to slightly complex tests if that means not touching the production code. The fact that it's running in //content as a browser test over Blink isn't a critical issue IMO.

#4 - I'm not super comfortable with firing an event for tests only. It's a bit weird, especially given that it's an event we already fire. If we really want to go that route, maybe it should be renamed something that is clear such as "cr-tests-playback-suspended".

#3 - I think it is a lot of special testing methods and I might have a biased against this. I didn't look into details apart from that.

Did you consider writing a "webkit_unit_tests". That might be helpful because you will still be in Blink layer, will be able to mock a thing or two instead of creating now internals/ForTesting methods. Happy to provide examples if you want.

It wouldn't be any easier as a webkit unit test. I'd still need the test methods to poke the WMP state. We also already have lots of unittests for this functionality with varying degrees of mock-outs, this bug is about creating e2e integration tests due to a stable-channel regression.

The event we already fire is "suspend" the event the test added is "suspended", which is confusing none-the-less and combined with the addition of prod-code to avoid rAF testing makes #4 a not so great solution :)

As mentioned in c#9 though, I think the only two test methods we are a method to force the stale state and inspect the suspend state. It seems like approach #3 is the best compromise between ease of testing, speed, and production code changes. I'm against going down the browser test route after writing the layout test variants; the browser tests are just too cumbersome and will require adding some IPCs to actually test the full set of behaviors.

I'll start writing the layout test version based on variant #3 today. I'll probably still check-in the one browser test I did write though since it is useful in testing the "tab close -> resume" case that won't have coverage on the layout test side of things.

#11, slow tests are terrible, so an internal API to either shorten the timeout or trigger it immediately makes sense.

I don't know of any other test-only events being emitted, but as long as it really is only emitted when running tests, it seems pretty reasonable. In some way it might be "nicer" if it's fired on the internals object, but I can't see any other cases like that, so on the media element seems OK.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82662e2171c9437e4141aeafd03c0841b96d2bc8

commit 82662e2171c9437e4141aeafd03c0841b96d2bc8
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Jan 31 20:22:21 2018

Add layout tests exercising suspend/resume of src= media elements.

Testing is accomplished by a couple new window.internals.* methods
which allow modification and inspection of suspend state:
- forceStaleStateForMediaElement(element)
- isMediaElementSuspended(element)

Unfortunately due to the async nature of suspend the method for
testing suspend must be polled once a idle suspend is initiated via
the forceStaleStateForMediaElement(). Test helper methods have been
added to media/suspend-util.js to alleviate test cruft.

forceStaleStateForMediaElement() does not actually force a suspend,
it simply marks the element as stale. If an element is playing,
seeking, or otherwise in a non-suspendable state, no suspend will
occur. This is intentional to ensure tests cover our state machine
as close to reality as possible.

Unfortunately the only way to force given ready states for the
media element is through throttled loading; this CL makes extensive
use of throttled loading and adds a throttling limit to our layout
test loader to avoid timeouts with slow initial rates. These tests
were run with a --repeat-each=200 several times without flake, but
I'll also watch the flakiness dashboard for these afterward.

As a result of the above combined with the fact that we are not
yet propagating play state to WMPI, we can reach a timeout state
in the 'suspend after metadata' src= test. It's marked as pass
and timeout in TestExpectations until the following lands:

https://chromium-review.googlesource.com/c/chromium/src/+/876924

This is part 1 of 2, the 2nd part will add MSE based variants of
these tests. No new internals methods are expected though, just
a appendBuffer() based mechanism for triggering ready states.

BUG= 694855 ,  756897 
TEST=it's all tests!

Change-Id: I84285ea49161f24cdd885bfc8b9224aa51fa5643
Reviewed-on: https://chromium-review.googlesource.com/882283
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Mounir Lamouri (slow) <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533380}
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/content/shell/renderer/layout_test/layout_test_content_renderer_client.h
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-future-data.html
[add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-metadata.html
[add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-before-have-metadata.html
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/video-throttled-load.cgi
[add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/media/media-src-suspend-after-have-enough-data.html
[add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/media/suspend-util.js
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/Source/core/testing/Internals.idl
[modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/public/platform/WebMediaPlayer.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b

commit 9f1fa9c053e29a586ccbd52c1f58d16e6de3318b
Author: Tommy Steimel <steimel@chromium.org>
Date: Wed Jan 31 21:33:04 2018

Revert "Add layout tests exercising suspend/resume of src= media elements."

This reverts commit 82662e2171c9437e4141aeafd03c0841b96d2bc8.

Reason for revert: http/tests/media/media-src-suspend-after-have-future-data.html is failing on WebKit Mac bots

Original change's description:
> Add layout tests exercising suspend/resume of src= media elements.
> 
> Testing is accomplished by a couple new window.internals.* methods
> which allow modification and inspection of suspend state:
> - forceStaleStateForMediaElement(element)
> - isMediaElementSuspended(element)
> 
> Unfortunately due to the async nature of suspend the method for
> testing suspend must be polled once a idle suspend is initiated via
> the forceStaleStateForMediaElement(). Test helper methods have been
> added to media/suspend-util.js to alleviate test cruft.
> 
> forceStaleStateForMediaElement() does not actually force a suspend,
> it simply marks the element as stale. If an element is playing,
> seeking, or otherwise in a non-suspendable state, no suspend will
> occur. This is intentional to ensure tests cover our state machine
> as close to reality as possible.
> 
> Unfortunately the only way to force given ready states for the
> media element is through throttled loading; this CL makes extensive
> use of throttled loading and adds a throttling limit to our layout
> test loader to avoid timeouts with slow initial rates. These tests
> were run with a --repeat-each=200 several times without flake, but
> I'll also watch the flakiness dashboard for these afterward.
> 
> As a result of the above combined with the fact that we are not
> yet propagating play state to WMPI, we can reach a timeout state
> in the 'suspend after metadata' src= test. It's marked as pass
> and timeout in TestExpectations until the following lands:
> 
> https://chromium-review.googlesource.com/c/chromium/src/+/876924
> 
> This is part 1 of 2, the 2nd part will add MSE based variants of
> these tests. No new internals methods are expected though, just
> a appendBuffer() based mechanism for triggering ready states.
> 
> BUG= 694855 ,  756897 
> TEST=it's all tests!
> 
> Change-Id: I84285ea49161f24cdd885bfc8b9224aa51fa5643
> Reviewed-on: https://chromium-review.googlesource.com/882283
> Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
> Reviewed-by: Peter Beverloo <peter@chromium.org>
> Reviewed-by: Mounir Lamouri (slow) <mlamouri@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533380}

TBR=dalecurtis@chromium.org,peter@chromium.org,mlamouri@chromium.org,foolip@chromium.org

Change-Id: I9225560f2d01b6f874ddb219641f72b3d5863741
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  694855 ,  756897 
Reviewed-on: https://chromium-review.googlesource.com/896111
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533418}
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/content/shell/renderer/layout_test/layout_test_content_renderer_client.h
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-future-data.html
[delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-metadata.html
[delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-before-have-metadata.html
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/LayoutTests/http/tests/media/video-throttled-load.cgi
[delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/media/media-src-suspend-after-have-enough-data.html
[delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/media/suspend-util.js
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/Source/core/testing/Internals.idl
[modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/public/platform/WebMediaPlayer.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dde5fe4f2d111efb75f80516266be9f572c4d13d

commit dde5fe4f2d111efb75f80516266be9f572c4d13d
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Feb 01 18:43:45 2018

Add media pipeline forced suspend browser tests.

This specifically tests the force suspend signal sent by the browser
when tabs are closed. Full suspend/resume testing is handled by
layout tests for ease of test writing and ready state manipulation:

https://chromium-review.googlesource.com/c/chromium/src/+/882283

This is part 1/2, a later CL will add an MSE variant of this test.

BUG= 756897 
TEST=it's all tests!

Change-Id: I588ccf5feb285f972e392c17498c2eaa071a86c6
Reviewed-on: https://chromium-review.googlesource.com/881832
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533754}
[modify] https://crrev.com/dde5fe4f2d111efb75f80516266be9f572c4d13d/content/browser/media/media_browsertest.cc
[modify] https://crrev.com/dde5fe4f2d111efb75f80516266be9f572c4d13d/content/browser/media/media_browsertest.h
[add] https://crrev.com/dde5fe4f2d111efb75f80516266be9f572c4d13d/content/browser/media/media_suspend_browsertest.cc
[modify] https://crrev.com/dde5fe4f2d111efb75f80516266be9f572c4d13d/content/test/BUILD.gn
[add] https://crrev.com/dde5fe4f2d111efb75f80516266be9f572c4d13d/media/test/data/media_suspend_test.html

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f81942cadc658e33042acb8d932cef238373db77

commit f81942cadc658e33042acb8d932cef238373db77
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Feb 07 10:01:45 2018

Remove kHaveFutureData restriction from play state notifications.

HTMLMediaElement should tell WebMediaPlayer about play state changes
as soon as the kHaveMetaData state is reached.

This will eventually allow us to delete a bunch of guessing-game type
code for idle suspension and instead rely on actually getting a play()
call from Blink to wake up when needed.

We can't stop WMPI from reaching the future data state quite yet though,
because it has implications on the delivery of 'canplay' and 'canplaythrough'
events that need to be part of a larger preload=metadata experiment. See old
discussion on this here:

https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html

Note: This change exposed some bugs in how the ready state maximum is used
and the states at which a video frame is expected. These issues are fixed
in the HTMLVideoElement.

BUG= 694855 ,  756897 
TEST=existing suspend tests no longer flake.

Change-Id: I45478c8238dbd48d526933caacc4cf106583d415
Reviewed-on: https://chromium-review.googlesource.com/876743
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534973}
[modify] https://crrev.com/f81942cadc658e33042acb8d932cef238373db77/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/f81942cadc658e33042acb8d932cef238373db77/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp
[modify] https://crrev.com/f81942cadc658e33042acb8d932cef238373db77/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed

commit 0affd0504bdad5b2a5c4777f46f200a6a43eb9ed
Author: Guido Urdaneta <guidou@chromium.org>
Date: Wed Feb 07 15:55:09 2018

Revert "Remove kHaveFutureData restriction from play state notifications."

This reverts commit f81942cadc658e33042acb8d932cef238373db77.

Reason for revert: Causes reliable failure on Mac bot for the following test:
http/tests/media/media-src-suspend-after-have-future-data.html

First failure:
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12/10636




Original change's description:
> Remove kHaveFutureData restriction from play state notifications.
> 
> HTMLMediaElement should tell WebMediaPlayer about play state changes
> as soon as the kHaveMetaData state is reached.
> 
> This will eventually allow us to delete a bunch of guessing-game type
> code for idle suspension and instead rely on actually getting a play()
> call from Blink to wake up when needed.
> 
> We can't stop WMPI from reaching the future data state quite yet though,
> because it has implications on the delivery of 'canplay' and 'canplaythrough'
> events that need to be part of a larger preload=metadata experiment. See old
> discussion on this here:
> 
> https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html
> 
> Note: This change exposed some bugs in how the ready state maximum is used
> and the states at which a video frame is expected. These issues are fixed
> in the HTMLVideoElement.
> 
> BUG= 694855 ,  756897 
> TEST=existing suspend tests no longer flake.
> 
> Change-Id: I45478c8238dbd48d526933caacc4cf106583d415
> Reviewed-on: https://chromium-review.googlesource.com/876743
> Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534973}

TBR=dalecurtis@chromium.org,mlamouri@chromium.org

Change-Id: I2bfb38a8cbace8551d8a000b84c05f7b496b852d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  694855 ,  756897 
Reviewed-on: https://chromium-review.googlesource.com/906770
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535013}
[modify] https://crrev.com/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp
[modify] https://crrev.com/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp

See also https://bugs.chromium.org/p/chromium/issues/detail?id=809998#c5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4

commit b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Tue Feb 13 21:39:28 2018

Remove kHaveFutureData restriction from play state notifications.

This reland marks media-src-suspend-after-have-future-data and
media-src-suspend-after-have-metadata as SlowTest(s) now that they no
longer permanently time out. Each takes ~2ish seconds in release build
and so meets the criteria for being a SlowTest.

This also changes WMPI to not try and get the current time value prior
to the HaveCurrentData state (which Blink already respects).

========= [ Original Description Follows ] ===============

HTMLMediaElement should tell WebMediaPlayer about play state changes
as soon as the kHaveMetaData state is reached.

This will eventually allow us to delete a bunch of guessing-game type
code for idle suspension and instead rely on actually getting a play()
call from Blink to wake up when needed.

We can't stop WMPI from reaching the future data state quite yet though,
because it has implications on the delivery of 'canplay' and 'canplaythrough'
events that need to be part of a larger preload=metadata experiment. See old
discussion on this here:

https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html

Note: This change exposed some bugs in how the ready state maximum is used
and the states at which a video frame is expected. These issues are fixed
in the HTMLVideoElement.

BUG= 694855 ,  756897 ,  809998 
TEST=existing suspend tests no longer flake.

Change-Id: Ie6297df474c1f5da56ca6c0e81efa636fbc349bf
Reviewed-on: https://chromium-review.googlesource.com/915081
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536487}
[modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp
[modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc5d47c8ab0d958bdd967a834c472106f9697375

commit dc5d47c8ab0d958bdd967a834c472106f9697375
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Feb 15 11:15:19 2018

Revert "Remove kHaveFutureData restriction from play state notifications."

This reverts commit b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=812423

Original change's description:
> Remove kHaveFutureData restriction from play state notifications.
> 
> This reland marks media-src-suspend-after-have-future-data and
> media-src-suspend-after-have-metadata as SlowTest(s) now that they no
> longer permanently time out. Each takes ~2ish seconds in release build
> and so meets the criteria for being a SlowTest.
> 
> This also changes WMPI to not try and get the current time value prior
> to the HaveCurrentData state (which Blink already respects).
> 
> ========= [ Original Description Follows ] ===============
> 
> HTMLMediaElement should tell WebMediaPlayer about play state changes
> as soon as the kHaveMetaData state is reached.
> 
> This will eventually allow us to delete a bunch of guessing-game type
> code for idle suspension and instead rely on actually getting a play()
> call from Blink to wake up when needed.
> 
> We can't stop WMPI from reaching the future data state quite yet though,
> because it has implications on the delivery of 'canplay' and 'canplaythrough'
> events that need to be part of a larger preload=metadata experiment. See old
> discussion on this here:
> 
> https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html
> 
> Note: This change exposed some bugs in how the ready state maximum is used
> and the states at which a video frame is expected. These issues are fixed
> in the HTMLVideoElement.
> 
> BUG= 694855 ,  756897 ,  809998 
> TEST=existing suspend tests no longer flake.
> 
> Change-Id: Ie6297df474c1f5da56ca6c0e81efa636fbc349bf
> Reviewed-on: https://chromium-review.googlesource.com/915081
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536487}

TBR=dalecurtis@chromium.org,mlamouri@chromium.org,liberato@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  694855 ,  756897 ,  809998 
Change-Id: I176b5116682851853ff2773e6e4a77a9ac2664b2
Reviewed-on: https://chromium-review.googlesource.com/920481
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536988}
[modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp
[modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d1d5207d97b36a4f46a62563860b5792735db554

commit d1d5207d97b36a4f46a62563860b5792735db554
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Feb 15 17:37:20 2018

Revert "Remove kHaveFutureData restriction from play state notifications."

This reverts commit b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=812423

Original change's description:
> Remove kHaveFutureData restriction from play state notifications.
> 
> This reland marks media-src-suspend-after-have-future-data and
> media-src-suspend-after-have-metadata as SlowTest(s) now that they no
> longer permanently time out. Each takes ~2ish seconds in release build
> and so meets the criteria for being a SlowTest.
> 
> This also changes WMPI to not try and get the current time value prior
> to the HaveCurrentData state (which Blink already respects).
> 
> ========= [ Original Description Follows ] ===============
> 
> HTMLMediaElement should tell WebMediaPlayer about play state changes
> as soon as the kHaveMetaData state is reached.
> 
> This will eventually allow us to delete a bunch of guessing-game type
> code for idle suspension and instead rely on actually getting a play()
> call from Blink to wake up when needed.
> 
> We can't stop WMPI from reaching the future data state quite yet though,
> because it has implications on the delivery of 'canplay' and 'canplaythrough'
> events that need to be part of a larger preload=metadata experiment. See old
> discussion on this here:
> 
> https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html
> 
> Note: This change exposed some bugs in how the ready state maximum is used
> and the states at which a video frame is expected. These issues are fixed
> in the HTMLVideoElement.
> 
> BUG= 694855 ,  756897 ,  809998 
> TEST=existing suspend tests no longer flake.
> 
> Change-Id: Ie6297df474c1f5da56ca6c0e81efa636fbc349bf
> Reviewed-on: https://chromium-review.googlesource.com/915081
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536487}

TBR=dalecurtis@chromium.org,mlamouri@chromium.org,liberato@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  694855 ,  756897 ,  809998 
Change-Id: I176b5116682851853ff2773e6e4a77a9ac2664b2
Reviewed-on: https://chromium-review.googlesource.com/920481
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#536988}(cherry picked from commit dc5d47c8ab0d958bdd967a834c472106f9697375)
Reviewed-on: https://chromium-review.googlesource.com/922301
Reviewed-by: Claude Masso <cmasso@chromium.org>
Cr-Commit-Position: refs/branch-heads/3348@{#3}
Cr-Branched-From: 62c55a88a1aa6b877054b25d06e3ab0ea505d253-refs/heads/master@{#536934}
[modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp
[modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6438cf1ff7f9e249d150b5dc22d0157193c09cac

commit 6438cf1ff7f9e249d150b5dc22d0157193c09cac
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Mar 29 02:34:01 2018

Simplify suspend/resume testing by having WMP suspend based on state.

This removes the need for stalled loading to force states on the WMPI
pipeline by instead having WMPI suspend once it reaches one of the
target states.

This provides equivalent coverage with much faster testing.

BUG= 819194 , 817705 , 756897 
TEST=no more flaky tests.

Change-Id: I1fa9e1ff6d165ed7921abf62127d3c2cd89f5707
Reviewed-on: https://chromium-review.googlesource.com/980802
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546704}
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/ASANExpectations
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/MSANExpectations
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/dae40d733338a3c54add2a9385674e9094cd04bb/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-future-data.html
[delete] https://crrev.com/dae40d733338a3c54add2a9385674e9094cd04bb/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-metadata.html
[delete] https://crrev.com/dae40d733338a3c54add2a9385674e9094cd04bb/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-before-have-metadata.html
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/media/media-src-suspend-after-have-enough-data.html
[add] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/media/media-src-suspend-after-have-future-data.html
[add] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/media/media-src-suspend-after-have-metadata.html
[add] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/media/media-src-suspend-before-have-metadata.html
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/media/suspend-util.js
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/Source/core/testing/Internals.idl
[modify] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/public/platform/WebMediaPlayer.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26561f43c8598bbc921e17b8ef93f0c0183106d3

commit 26561f43c8598bbc921e17b8ef93f0c0183106d3
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Apr 05 21:57:04 2018

Add suspend/resume tests for MSE.

This adds three tests: before metadata, after metadata, and after
have enough. It doesn't clone the have future data test that src=
has since has_future_data == has_enough_data when using MSE.

BUG= 756897 
TEST=all tests!

Change-Id: I99c2cff3668c471bcd3d3d4549e53690788b0813
Reviewed-on: https://chromium-review.googlesource.com/989073
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548575}
[modify] https://crrev.com/26561f43c8598bbc921e17b8ef93f0c0183106d3/media/blink/webmediaplayer_impl.h
[add] https://crrev.com/26561f43c8598bbc921e17b8ef93f0c0183106d3/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-suspend-after-have-enough-data.html
[add] https://crrev.com/26561f43c8598bbc921e17b8ef93f0c0183106d3/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-suspend-after-have-metadata.html
[add] https://crrev.com/26561f43c8598bbc921e17b8ef93f0c0183106d3/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-suspend-before-have-metadata.html
[modify] https://crrev.com/26561f43c8598bbc921e17b8ef93f0c0183106d3/third_party/WebKit/LayoutTests/media/suspend-util.js

 Issue 596191  has been merged into this issue.