MediaRecorder: handle libwebm::Segment::AddFrame() errors and wire up to Blink

MediaRecorder: handle libwebm::Segment::AddFrame() errors and wire up to Blink

libwebm::Segment::AddFrame() can and will return false on error 
conditions, but the return value is currently, sadly, ignored.  
This CL handles the return value and wires it to Blink's 
onError() callback.

Unittests are beefed up, in particular:

- Added WebmMuxer::ForceOneLibWebmErrorForTesting(), and using it 
in WebmMuxerTest to force errors and then 
EXPECT_FALSE(...OnEncodedVideo/OnEncodedAudio)

- Thanks to scheib@ insistence in testing, I found an unreachable 
code and a bug (!) in WebmMuxer:

-- the dumping of |encoded_frames_queue_| [1] is not reachable 
because the said deque is emptied in OnEncodedAudio() (left a 
DCHECK() instead).

-- the bug, introduced in this very CL, was the use-after-free 
during emptying the same deque, when retrying emptying it after an 
error. 

- MediaRecorderHandlerTest adds a test case to verify that a
muxer error causes an onError() to be bubbled up to Blink.


All in all, testing FTW!


[1] https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?q=webm_muxer.cc&sq=package:chromium&dr&l=162


BUG=675521
Review-Url: https://codereview.chromium.org/2633623002
Cr-Commit-Position: refs/heads/master@{#443780}
Committed: https://chromium.googlesource.com/chromium/src/+/ed7c87b8816f37e73f640e3b7c5c93fad0dbd29a

[mcasas, 2017-01-13 02:33:12]

Patchset #1 (id:1) has been deleted

[mcasas, 2017-01-13 02:37:58]

mcasas@chromium.org changed reviewers:
+ scheib@chromium.org

[mcasas, 2017-01-13 02:37:59]

scheib@ ptal

fresh pair of eyes for an easy CL

[mcasas, 2017-01-13 07:33:44]

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

[commit-bot: I haz the power, 2017-01-13 07:34:00]

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, 2017-01-13 08:35:56]

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

[commit-bot: I haz the power, 2017-01-13 08:35:57]

Dry run: This issue passed the CQ dry run.

[scheib, 2017-01-13 18:07:20]

Can we add any tests for scenarios expected to fail?

[mcasas, 2017-01-14 00:27:22]

Description was changed from

==========
MediaRecorder: handle libwebm::Segment::AddFrame() errors and wire up to Blink

libwebm::Segment::AddFrame() can and will return false on error conditions,
but the return value is currently, sadly, ignored.  This CL handles the
return value and wires it to Blink's onError() callback.

BUG=675521
==========

to

==========
MediaRecorder: handle libwebm::Segment::AddFrame() errors and wire up to Blink

libwebm::Segment::AddFrame() can and will return false on error 
conditions, but the return value is currently, sadly, ignored.  
This CL handles the return value and wires it to Blink's 
onError() callback.

Unittests are beefed up, in particular:

- Added WebmMuxer::ForceOneLibWebmErrorForTesting(), and using it 
in WebmMuxerTest to force errors and then 
EXPECT_FALSE(...OnEncodedVideo/OnEncodedAudio)

- Thanks to scheib@ insistence in testing, I found an unreachable 
code and a bug (!) in WebmMuxer:

-- the dumping of |encoded_frames_queue_| [1] is not reachable 
because the said deque is emptied in OnEncodedAudio() (left a 
DCHECK() instead).

-- the bug, introduced in this very CL, was the use-after-free 
during emptying the same deque, when retrying emptying it after an 
error. 

- MediaRecorderHandlerTest adds a test case to verify that a
muxer error causes an onError() to be bubbled up to Blink.


All in all, testing FTW!


[1]
https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?q=webm_muxer....


BUG=675521
==========

[mcasas, 2017-01-14 00:27:37]

Patchset #2 (id:40001) has been deleted

[mcasas, 2017-01-14 00:58:06]

Patchset #2 (id:60001) has been deleted

[mcasas, 2017-01-14 01:01:57]

Patchset #2 (id:80001) has been deleted

[mcasas, 2017-01-14 01:46:32]

ptal

[scheib, 2017-01-14 02:36:20]

LGTM

[mcasas, 2017-01-14 02:51:47]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2017-01-14 02:52:05]

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, 2017-01-14 03:39:50]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484362307630430,
"parent_rev": "7bbe43192ccb51e01ae956f268a9db3732fd5dcd", "commit_rev":
"ed7c87b8816f37e73f640e3b7c5c93fad0dbd29a"}

[commit-bot: I haz the power, 2017-01-14 03:40:22]

Description was changed from

==========
MediaRecorder: handle libwebm::Segment::AddFrame() errors and wire up to Blink

libwebm::Segment::AddFrame() can and will return false on error 
conditions, but the return value is currently, sadly, ignored.  
This CL handles the return value and wires it to Blink's 
onError() callback.

Unittests are beefed up, in particular:

- Added WebmMuxer::ForceOneLibWebmErrorForTesting(), and using it 
in WebmMuxerTest to force errors and then 
EXPECT_FALSE(...OnEncodedVideo/OnEncodedAudio)

- Thanks to scheib@ insistence in testing, I found an unreachable 
code and a bug (!) in WebmMuxer:

-- the dumping of |encoded_frames_queue_| [1] is not reachable 
because the said deque is emptied in OnEncodedAudio() (left a 
DCHECK() instead).

-- the bug, introduced in this very CL, was the use-after-free 
during emptying the same deque, when retrying emptying it after an 
error. 

- MediaRecorderHandlerTest adds a test case to verify that a
muxer error causes an onError() to be bubbled up to Blink.


All in all, testing FTW!


[1]
https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?q=webm_muxer....


BUG=675521
==========

to

==========
MediaRecorder: handle libwebm::Segment::AddFrame() errors and wire up to Blink

libwebm::Segment::AddFrame() can and will return false on error 
conditions, but the return value is currently, sadly, ignored.  
This CL handles the return value and wires it to Blink's 
onError() callback.

Unittests are beefed up, in particular:

- Added WebmMuxer::ForceOneLibWebmErrorForTesting(), and using it 
in WebmMuxerTest to force errors and then 
EXPECT_FALSE(...OnEncodedVideo/OnEncodedAudio)

- Thanks to scheib@ insistence in testing, I found an unreachable 
code and a bug (!) in WebmMuxer:

-- the dumping of |encoded_frames_queue_| [1] is not reachable 
because the said deque is emptied in OnEncodedAudio() (left a 
DCHECK() instead).

-- the bug, introduced in this very CL, was the use-after-free 
during emptying the same deque, when retrying emptying it after an 
error. 

- MediaRecorderHandlerTest adds a test case to verify that a
muxer error causes an onError() to be bubbled up to Blink.


All in all, testing FTW!


[1]
https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?q=webm_muxer....


BUG=675521

Review-Url: https://codereview.chromium.org/2633623002
Cr-Commit-Position: refs/heads/master@{#443780}
Committed:
https://chromium.googlesource.com/chromium/src/+/ed7c87b8816f37e73f640e3b7c5c...
==========

[commit-bot: I haz the power, 2017-01-14 03:40:25]

Committed patchset #2 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/ed7c87b8816f37e73f640e3b7c5c...