MSE: Experimental support for new abort and duration behavior

MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

BUG=623729, 398130
TEST=Updated many of http/tests/media/media-source/* and added SBSTest.GetHighestPresentationTimestamp
R=chcunningham@chromium.org, dcheng@chromium.org, holte@chromium.org, tkent@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/663209d1ecce638d0fc8bde35daa7a471a899f65

[wolenetz, 2016-06-28 22:55:21]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org,mlamouri@chromium.org,dcheng@chromium.org
BUG=623729
TEST=TODO
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org,mlamouri@chromium.org,dcheng@chromium.org
BUG=623729
TEST=TODO
==========

[wolenetz, 2016-06-28 22:55:21]

wolenetz@chromium.org changed reviewers:
- dcheng@chromium.org, mlamouri@chromium.org

[wolenetz, 2016-06-28 22:55:40]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org,mlamouri@chromium.org,dcheng@chromium.org
BUG=623729
TEST=TODO
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729
TEST=TODO
==========

[wolenetz, 2016-06-28 23:31:31]

Patch set 1 is the basic approach. It will need further testing for the *new*
behavior, including adjusting existing tests that fail to verify the old
behavior.

Also, when running with --disable-blink-features=MediaSourceNewAbortAndDuration,
the following two tests crash:
 http/tests/media/media-source/mediasource-htmlmediaelement-lifetime.html [
Crash ]
 http/tests/media/media-source/mediasource-remove.html [ Crash ]

chcunningham@: This is an early patch set. Take a look if you want to
familiarize yourself with the approach.

[wolenetz, 2016-06-29 21:09:02]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729
TEST=TODO
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729
TEST=TODO
==========

[chcunningham, 2016-06-29 21:19:24]

Looks pretty good. Awaiting test changes for final sign off

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/UseCounter.h (right):

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/UseCounter.h:1228: MediaSourceAbortRemove =
1421,
friendly reminder to be careful on rebase

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediasource/MediaSource.cpp (right):

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediasource/MediaSource.cpp:444: // See also
deprecated remove(new duration, old duration) behavior below.
given that they aren't related, this is maybe more confusing than helpful (or
i'm missing something)

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediasource/MediaSource.cpp:455: if
(!RuntimeEnabledFeatures::mediaSourceNewAbortAndDurationEnabled() && newDuration
< oldDuration) {
Maybe you can save some cycles and only run this remove call if newDuration <
highestPresTime. IIUC, running whenever highestPresTime < newDuration <
oldDuration is a no-op (other than firing a few events... which is perhaps
problematic for my proposal)

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp (right):

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp:542: SBLOG <<
__FUNCTION__ << " this=" << this;
Do you need both SBLOGs? (here and 2 lines down)

[wolenetz, 2016-06-29 21:44:12]

On 2016/06/28 23:31:31, wolenetz wrote:
> Patch set 1 is the basic approach. It will need further testing for the *new*
> behavior, including adjusting existing tests that fail to verify the old
> behavior.
> 
> Also, when running with
--disable-blink-features=MediaSourceNewAbortAndDuration,
> the following two tests crash:
>  http/tests/media/media-source/mediasource-htmlmediaelement-lifetime.html [
> Crash ]
>  http/tests/media/media-source/mediasource-remove.html [ Crash ]
> 
> chcunningham@: This is an early patch set. Take a look if you want to
> familiarize yourself with the approach.

Patch set 2 appears to both:
* not regress old behavior (when the new flag is disabled in layout test runner
using
--additional-drt-flag="--disable-blink-features=MediaSourceNewAbortAndDuration"),
the old layout tests pass as expected), and
* emit the new deprecation warnings to devtools console (when the new flag is
disabled (by default with chrome), a local test page that attempts to do a
truncating duration reduction, and separately an abort() of an in-progress
remove(). For example (from shell console):
[1286:1286:0629/143414:INFO:CONSOLE(73)] "audio: 1 [0.015333 - 15.012666]",
source: http://localhost:8000/test.html (73)
[1286:1286:0629/143414:INFO:CONSOLE(44)] "video: 1 [0 - 15]", source:
http://localhost:8000/test.html (44)
[1286:1286:0629/143415:INFO:CONSOLE(81)] "duration=Infinity, about to set to
10", source: http://localhost:8000/test.html (81)
[1286:1286:0629/143415:INFO:CONSOLE(82)] "Setting MediaSource.duration below the
highest presentation timestamp of any buffered coded frames is deprecated due to
specification change. Support for implicit removal of truncated buffered media
will be removed in the future. You should instead perform explicit
remove(newDuration, oldDuration) on all sourceBuffers, where newDuration <
oldDuration. See https://www.chromestatus.com/features/6107495151960064 for more
details.", source: http://localhost:8000/test.html (82)
...
[1286:1286:0629/143423:INFO:CONSOLE(88)] "About to synchronously try to
remove(0,duration) and then abort() it", source: http://localhost:8000/test.html
(88)
[1286:1286:0629/143423:INFO:CONSOLE(90)] "Using SourceBuffer.abort() to abort
remove()'s asynchronous range removal is deprecated due to specification change.
Support will be removed in the future. You should instead await 'updateend'.
abort() is intended to only abort asynchronous media appends. See
https://www.chromestatus.com/features/6107495151960064 for more details.",
source: http://localhost:8000/test.html (90)

Work remaining to be done in next patch set(s):
* Adjust existing layout tests to verify the *new* behavior.
* Add at least some unit test of the HBPT calculation.

Note, due to ongoing PTS/DTS conflation (bug 398130), this change actually uses
highest buffered DTS, not highest buffered PTS.

[wolenetz, 2016-06-29 21:46:17]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729
TEST=TODO
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729,398130
TEST=TODO
==========

[wolenetz, 2016-06-30 00:46:56]

The CQ bit was checked by wolenetz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 00:47:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[wolenetz, 2016-06-30 01:17:49]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729,398130
TEST=TODO
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729,398130
TEST=Updated many of http/tests/media/media-source/* and added
SBSTest.GetHighestPresentationTimestamp
==========

[wolenetz, 2016-06-30 01:25:22]

The CQ bit was checked by wolenetz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 01:25:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[wolenetz, 2016-06-30 01:25:56]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

R=chcunningham@chromium.org
BUG=623729,398130
TEST=Updated many of http/tests/media/media-source/* and added
SBSTest.GetHighestPresentationTimestamp
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

BUG=623729,398130
TEST=Updated many of http/tests/media/media-source/* and added
SBSTest.GetHighestPresentationTimestamp
==========

[wolenetz, 2016-06-30 01:35:48]

wolenetz@chromium.org changed reviewers:
+ dcheng@chromium.org, holte@chromium.org, tkent@chromium.org

[wolenetz, 2016-06-30 01:35:50]

Please take a look at patch set 8. Apart from a tiny rebase to update the
UseCounter enum (and histogram), it should all be ready to go. Your support on
the intent to implement thread ([3] in CL description) would also be helpful.
I'd like to land this ASAP.

chcunningham@: Everything, especially media/* .../Source/modules/*
.../LayoutTests/*
tkent@: OWNERS review for .../WebKit/public/* .../Source/platform/*
dcheng@: OWNERS review for .../Source/core/frame/*
holte@: OWNERS review for tools/metrics/*

Thank you all!

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/UseCounter.h (right):

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/UseCounter.h:1228: MediaSourceAbortRemove =
1421,
On 2016/06/29 21:19:24, chcunningham wrote:
> friendly reminder to be careful on rebase

Acknowledged.

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediasource/MediaSource.cpp (right):

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediasource/MediaSource.cpp:444: // See also
deprecated remove(new duration, old duration) behavior below.
On 2016/06/29 21:19:24, chcunningham wrote:
> given that they aren't related, this is maybe more confusing than helpful (or
> i'm missing something)

They are tightly related. This is referring to line 456. The new behavior
requires enforcement here, so deprecation warning is done here. The old behavior
(which is deprecated) is at line 456.

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediasource/MediaSource.cpp:455: if
(!RuntimeEnabledFeatures::mediaSourceNewAbortAndDurationEnabled() && newDuration
< oldDuration) {
On 2016/06/29 21:19:24, chcunningham wrote:
> Maybe you can save some cycles and only run this remove call if newDuration <
> highestPresTime. IIUC, running whenever highestPresTime < newDuration <
> oldDuration is a no-op (other than firing a few events... which is perhaps
> problematic for my proposal)

There are events and an async period of time when updating is held true.
Otherwise, you'd be correct. But we still need to do the old behavior (including
events and async scheduling of a no-op removal where highestPresTime <
newDuration < oldDuration during which time updating becomes true).

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp (right):

https://codereview.chromium.org/2102323002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp:542: SBLOG <<
__FUNCTION__ << " this=" << this;
On 2016/06/29 21:19:24, chcunningham wrote:
> Do you need both SBLOGs? (here and 2 lines down)

Nope. I was being extra-verbose. I'll keep the latter.

https://codereview.chromium.org/2102323002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html
(right):

https://codereview.chromium.org/2102323002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html:48:
// Explicitly remove buffered beyond the new reduced duration media prior to
reducing duration.
Awkward word order will be fixed in next patch set.

https://codereview.chromium.org/2102323002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-seek-beyond-duration.html
(left):

https://codereview.chromium.org/2102323002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-seek-beyond-duration.html:88:
seekToSpecifiedTimeSetEOSAndVerifyDone(test, mediaElement, mediaSource,
mediaSource.duration, mediaSource.duration + 0.1);
aside: The extra parameter here meant we weren't previously testing what we
thought we were :/. Fixed.

https://codereview.chromium.org/2102323002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.h (right):

https://codereview.chromium.org/2102323002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.h:1233: MediaSourceAbortRemove =
1427,
note: I'll likely need to rebase and rerun the histogram update tool before
eventually landing this CL.

[tkent, 2016-06-30 03:25:24]

> tkent@: OWNERS review for .../WebKit/public/* .../Source/platform/*

lgtm

[commit-bot: I haz the power, 2016-06-30 04:25:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 04:25:42]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-06-30 07:03:16]

rs lgtm for //third_party/WebKit/Source/core/frame changes.

[Steven Holte, 2016-06-30 18:27:47]

On 2016/06/30 07:03:16, dcheng wrote:
> rs lgtm for //third_party/WebKit/Source/core/frame changes.

histograms lgtm

[wolenetz, 2016-06-30 20:26:41]

The CQ bit was checked by wolenetz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 20:27:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chcunningham, 2016-06-30 20:27:42]

lgtm

https://codereview.chromium.org/2102323002/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html
(right):

https://codereview.chromium.org/2102323002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html:144:
'mediaElement reducedDuration after seek to it');
nit: can you make these more readable sentences? i feel like you're missing a
word

[wolenetz, 2016-06-30 20:28:57]

Thanks for reviews.  I've rebased since the usecounter enum mapping is
continually changing.

Dry-running CQ optimistically in anticipation of l*tm of patch set 9 final l*tm
from chcunningham@.

Note, if there's at most test-only or non-production code changes resulting from
chcunningham@'s review, I'll want to land fixes for those later so that this CL
can land ASAP.

[commit-bot: I haz the power, 2016-06-30 20:37:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 20:37:06]

Dry run: Try jobs failed on following builders:
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)

[wolenetz, 2016-06-30 20:40:35]

On 2016/06/30 20:28:57, wolenetz wrote:
> Thanks for reviews.  I've rebased since the usecounter enum mapping is
> continually changing.
> 
> Dry-running CQ optimistically in anticipation of l*tm of patch set 9 final
l*tm
> from chcunningham@.
> 
> Note, if there's at most test-only or non-production code changes resulting
from
> chcunningham@'s review, I'll want to land fixes for those later so that this
CL
> can land ASAP.

Thanks Chris. I'll update that test and rerebase + CQ it.

[wolenetz, 2016-06-30 20:47:03]

Thanks again. Sending to CQ.

https://codereview.chromium.org/2102323002/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html
(right):

https://codereview.chromium.org/2102323002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html:144:
'mediaElement reducedDuration after seek to it');
On 2016/06/30 20:27:42, chcunningham wrote:
> nit: can you make these more readable sentences? i feel like you're missing a
> word

Done.

[wolenetz, 2016-06-30 20:47:10]

The CQ bit was checked by wolenetz@chromium.org

[wolenetz, 2016-06-30 20:47:11]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
holte@chromium.org, dcheng@chromium.org, chcunningham@chromium.org
Link to the patchset: https://codereview.chromium.org/2102323002/#ps180001
(title: "Fixed mediasource-duration nit")

[commit-bot: I haz the power, 2016-06-30 20:48:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-30 22:14:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 22:14:39]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wolenetz, 2016-06-30 23:00:41]

https://codereview.chromium.org/2102323002/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html
(right):

https://codereview.chromium.org/2102323002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html:51:
test.expectEvent(sourceBuffer, 'sourceBuffer range removal completed');
Ugh. This typo regressed the test. I fixed it, rebased to ToT, verified locally
all the mediasource layout tests pass on linux, and will git cl land PS12 since
PS10 otherwise passed CQ and UseCounters.h hasn't churned.

[wolenetz, 2016-06-30 23:10:27]

The CQ bit was checked by wolenetz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 23:11:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-30 23:13:38]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

BUG=623729,398130
TEST=Updated many of http/tests/media/media-source/* and added
SBSTest.GetHighestPresentationTimestamp
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

BUG=623729,398130
TEST=Updated many of http/tests/media/media-source/* and added
SBSTest.GetHighestPresentationTimestamp
R=chcunningham@chromium.org, dcheng@chromium.org, holte@chromium.org,
tkent@chromium.org

Committed: https://crrev.com/663209d1ecce638d0fc8bde35daa7a471a899f65
Cr-Commit-Position: refs/heads/master@{#403327}
==========

[commit-bot: I haz the power, 2016-06-30 23:13:40]

Patchset 12 (id:??) landed as
https://crrev.com/663209d1ecce638d0fc8bde35daa7a471a899f65
Cr-Commit-Position: refs/heads/master@{#403327}

[wolenetz, 2016-06-30 23:15:00]

Description was changed from

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

BUG=623729,398130
TEST=Updated many of http/tests/media/media-source/* and added
SBSTest.GetHighestPresentationTimestamp
R=chcunningham@chromium.org, dcheng@chromium.org, holte@chromium.org,
tkent@chromium.org

Committed: https://crrev.com/663209d1ecce638d0fc8bde35daa7a471a899f65
Cr-Commit-Position: refs/heads/master@{#403327}
==========

to

==========
MSE: Experimental support for new abort and duration behavior

Introduces a new RuntimeEnabledFeature "MediaSourceNewAbortAndDuration"
which enables new behavior corresponding to the recent MSE spec change
[1], fixing cross-platform spec ambiguities. New behavior:
* abort() gives error if there is a remove() in progress. abort() is
  intended to only abort in-progress appends, not removals.
* setting MediaSource.duration to a value which would truncate currently
  buffered media will now result in error. Apps can use explicit
  remove() to perform the desired buffered media truncation prior to
  lowering the duration.
Note that both of these behavior changes are intended to fix
previously ambiguous spec text related to asynchronous range
removals. There is no other strong reason why these two behavior
changes are included in the same CL (other than to reduce some
process redundancy.)
If "MediaSourceNewAbortAndDuration" is not enabled, then the old
behavior remains along with emission of new deprecation messages.
This change only enables that flag in 'experimental' status (see [2]).

Intent to implement thread is at [3].

[1] https://github.com/w3c/media-source/pull/65
[2] http://www.chromium.org/blink/runtime-enabled-features
[3] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/oboI-rINUt0

BUG=623729,398130
TEST=Updated many of http/tests/media/media-source/* and added
SBSTest.GetHighestPresentationTimestamp
R=chcunningham@chromium.org, dcheng@chromium.org, holte@chromium.org,
tkent@chromium.org

Committed:
https://chromium.googlesource.com/chromium/src/+/663209d1ecce638d0fc8bde35daa...
==========

[wolenetz, 2016-06-30 23:15:02]

Committed patchset #12 (id:220001) manually as
663209d1ecce638d0fc8bde35daa7a471a899f65 (presubmit successful).

[foolip, 2016-11-24 09:31:45]

wolenetz@, is this ready for removal now? Lately we've been trying to avoid
deprecation messages with no removal milestone, so I'm looking through the ones
that we have and trying to understand how to move them forward.

[wolenetz, 2016-12-01 02:31:57]

On 2016/11/24 09:31:45, foolip wrote:
> wolenetz@, is this ready for removal now? Lately we've been trying to avoid
> deprecation messages with no removal milestone, so I'm looking through the
ones
> that we have and trying to understand how to move them forward.

I'd like to first add RAPPOR and hopefully merge that addition to M-56 to be
more confident about that removal. This is on my hotlist of things to do after I
complete all the collateral fixes related to my recent roll of FFmpeg for M56
(https://crbug.com/591845).

[wolenetz, 2017-02-15 01:16:44]

On 2016/12/01 02:31:57, wolenetz wrote:
> On 2016/11/24 09:31:45, foolip wrote:
> > wolenetz@, is this ready for removal now? Lately we've been trying to avoid
> > deprecation messages with no removal milestone, so I'm looking through the
> ones
> > that we have and trying to understand how to move them forward.
> 
> I'd like to first add RAPPOR and hopefully merge that addition to M-56 to be
> more confident about that removal. This is on my hotlist of things to do after
I
> complete all the collateral fixes related to my recent roll of FFmpeg for M56
> (https://crbug.com/591845).

foolip@, can you help confirm my new understanding from internal page
https://goto.google.com/sxbxi that MediaSourceAbortRemove and
MediaSourceDurationTruncatingBuffered use counters are almost 4 orders of
magnitude smaller than the CreateObjectURLMediaSource use counter?