MSE: Notify Blink SourceBuffer on init segment received

MSE: Notify Blink SourceBuffer on init segment received

As the second of a three-sided set of change:
This change lays the groundwork for Blink to own the app-visible pieces
of the MSE initialization segment received algorithm. It lets
SourceState notify WebSourceBufferImpl upon parsing an initialization
segment. WSBI then notifies its WebSourceBufferClient.
Updated unit tests are included.

R=acolwell@chromium.org
TEST=No media_unittest regression
BUG=249428, 397243
Committed: https://crrev.com/37e3ddb2b29418584a3e6207dfed7a584712a037
Cr-Commit-Position: refs/heads/master@{#294722}

[acolwell GONE FROM CHROMIUM, 2014-09-09 22:36:37]

looks pretty good.

https://codereview.chromium.org/547223003/diff/1/media/filters/chunk_demuxer.cc
File media/filters/chunk_demuxer.cc (right):

https://codereview.chromium.org/547223003/diff/1/media/filters/chunk_demuxer....
media/filters/chunk_demuxer.cc:122: const ChunkDemuxer::InitSegmentReceivedCB&
init_segment_received_cb);
nit: typedef this above so you don't have to mention ChunkDemuxer in all the
other locations below?

[wolenetz, 2014-09-10 00:43:47]

PTAL @ PS3:

This is second of 3-sided blink->chromium->blink set of changes:
Part 1: https://codereview.chromium.org/552943002/
Part 2: https://codereview.chromium.org/547223003/
Part 3: https://codereview.chromium.org/562503002/

https://codereview.chromium.org/547223003/diff/1/media/filters/chunk_demuxer.cc
File media/filters/chunk_demuxer.cc (right):

https://codereview.chromium.org/547223003/diff/1/media/filters/chunk_demuxer....
media/filters/chunk_demuxer.cc:122: const ChunkDemuxer::InitSegmentReceivedCB&
init_segment_received_cb);
On 2014/09/09 22:36:37, acolwell wrote:
> nit: typedef this above so you don't have to mention ChunkDemuxer in all the
> other locations below?

Done.

[acolwell GONE FROM CHROMIUM, 2014-09-10 18:02:38]

lgtm % nits

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
File media/filters/chunk_demuxer.cc (right):

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
media/filters/chunk_demuxer.cc:212: InitSegmentReceivedCB
init_segment_received_cb_during_append_;
nit: move cb_ to the end like all the other callback member vars. Also consider
dropping the during_append.

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
File media/filters/chunk_demuxer_unittest.cc (right):

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
media/filters/chunk_demuxer_unittest.cc:2159: AppendInitSegment(HAS_AUDIO |
HAS_VIDEO, true);
nit: I'm not sure this bool param is helping all that much. WDYT about just
putting explicit expectations in front of the calls that you are passing true?

[damienv1, 2014-09-10 18:41:02]

damienv@chromium.org changed reviewers:
+ damienv@chromium.org

[damienv1, 2014-09-10 18:41:03]

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
File media/filters/chunk_demuxer.cc (right):

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
media/filters/chunk_demuxer.cc:688:
init_segment_received_cb_during_append_.Run();
Can you DCHECK(!init_segment_received_cb_during_append_.is_null()) ?

(so that we detect illegitimate uses of OnNewConfigs during abort...)

[wolenetz, 2014-09-11 22:22:27]

Patchset #4 (id:60001) has been deleted

[wolenetz, 2014-09-11 22:40:38]

Thanks acolwell@.

damienv@, I'm not sure I understand your comment:

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
File media/filters/chunk_demuxer.cc (right):

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
media/filters/chunk_demuxer.cc:212: InitSegmentReceivedCB
init_segment_received_cb_during_append_;
On 2014/09/10 18:02:38, acolwell_leaving_chromium_9-23 wrote:
> nit: move cb_ to the end like all the other callback member vars. Also
consider
> dropping the during_append.

Done.

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
media/filters/chunk_demuxer.cc:688:
init_segment_received_cb_during_append_.Run();
On 2014/09/10 18:41:03, damienv1 wrote:
> Can you DCHECK(!init_segment_received_cb_during_append_.is_null()) ?
> 
> (so that we detect illegitimate uses of OnNewConfigs during abort...)

I'm confused. Running an is_null() callback should cause error. SS:Abort()
neither sets nor clears this callback; only SS::Append() does. Are you perhaps
asking for an inverse of this DCHECK to be added within SS::Abort()?

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
File media/filters/chunk_demuxer_unittest.cc (right):

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
media/filters/chunk_demuxer_unittest.cc:2159: AppendInitSegment(HAS_AUDIO |
HAS_VIDEO, true);
On 2014/09/10 18:02:38, acolwell_leaving_chromium_9-23 wrote:
> nit: I'm not sure this bool param is helping all that much. WDYT about just
> putting explicit expectations in front of the calls that you are passing true?

Done.

[wolenetz, 2014-09-13 00:28:56]

On 2014/09/11 22:40:38, wolenetz wrote:
> Thanks acolwell@.
> 
> damienv@, I'm not sure I understand your comment:
> 
>
https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
> File media/filters/chunk_demuxer.cc (right):
> 
>
https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
> media/filters/chunk_demuxer.cc:212: InitSegmentReceivedCB
> init_segment_received_cb_during_append_;
> On 2014/09/10 18:02:38, acolwell_leaving_chromium_9-23 wrote:
> > nit: move cb_ to the end like all the other callback member vars. Also
> consider
> > dropping the during_append.
> 
> Done.
> 
>
https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
> media/filters/chunk_demuxer.cc:688:
> init_segment_received_cb_during_append_.Run();
> On 2014/09/10 18:41:03, damienv1 wrote:
> > Can you DCHECK(!init_segment_received_cb_during_append_.is_null()) ?
> > 
> > (so that we detect illegitimate uses of OnNewConfigs during abort...)
> 
> I'm confused. Running an is_null() callback should cause error. SS:Abort()
> neither sets nor clears this callback; only SS::Append() does. Are you perhaps
> asking for an inverse of this DCHECK to be added within SS::Abort()?
> 
>
https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
> File media/filters/chunk_demuxer_unittest.cc (right):
> 
>
https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
> media/filters/chunk_demuxer_unittest.cc:2159: AppendInitSegment(HAS_AUDIO |
> HAS_VIDEO, true);
> On 2014/09/10 18:02:38, acolwell_leaving_chromium_9-23 wrote:
> > nit: I'm not sure this bool param is helping all that much. WDYT about just
> > putting explicit expectations in front of the calls that you are passing
true?
> 
> Done.

I clarified
https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
with damienv@ offline and will have another PS posted shortly.

[wolenetz, 2014-09-13 00:41:26]

damienv@, ptal PS5
thanks!

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
File media/filters/chunk_demuxer.cc (right):

https://codereview.chromium.org/547223003/diff/40001/media/filters/chunk_demu...
media/filters/chunk_demuxer.cc:688:
init_segment_received_cb_during_append_.Run();
On 2014/09/11 22:40:37, wolenetz wrote:
> On 2014/09/10 18:41:03, damienv1 wrote:
> > Can you DCHECK(!init_segment_received_cb_during_append_.is_null()) ?
> > 
> > (so that we detect illegitimate uses of OnNewConfigs during abort...)
> 
> I'm confused. Running an is_null() callback should cause error. SS:Abort()
> neither sets nor clears this callback; only SS::Append() does. Are you perhaps
> asking for an inverse of this DCHECK to be added within SS::Abort()?

Done.

[damienv1, 2014-09-13 00:44:33]

lgtm

[wolenetz, 2014-09-13 00:51:43]

The CQ bit was checked by wolenetz@chromium.org

[commit-bot: I haz the power, 2014-09-13 00:53:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/547223003/100001

[commit-bot: I haz the power, 2014-09-13 01:40:47]

Committed patchset #5 (id:100001) as 6e284912b804fb1a327b0166436d2241202c18b4

[commit-bot: I haz the power, 2014-09-13 01:42:43]

Patchset 5 (id:??) landed as
https://crrev.com/37e3ddb2b29418584a3e6207dfed7a584712a037
Cr-Commit-Position: refs/heads/master@{#294722}