Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(584)

Issue 1464313008: MediaRecorder testing of resume functionality and more start tests (Closed)

Created:
5 years ago by cpaulin (no longer in chrome)
Modified:
5 years ago
Reviewers:
phoglund_chromium
CC:
mcasas, chromium-reviews, jam, tnakamura+watch_chromium.org, phoglund+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

MediaRecorder testing of resume functionality and more start tests BUG=chromium:548914 Committed: https://crrev.com/3f39c25b888f3ccfb9a18e7d31ee20f0bcf89c58 Cr-Commit-Position: refs/heads/master@{#362288}

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+223 lines, -0 lines) Patch
M content/browser/media/webrtc_media_recorder_browsertest.cc View 1 chunk +31 lines, -0 lines 0 comments Download
M content/test/data/media/mediarecorder_test.html View 2 chunks +192 lines, -0 lines 4 comments Download

Messages

Total messages: 15 (7 generated)
cpaulin (no longer in chrome)
New CL ready for review
5 years ago (2015-11-30 18:23:16 UTC) #3
phoglund_chromium
lgtm https://codereview.chromium.org/1464313008/diff/1/content/test/data/media/mediarecorder_test.html File content/test/data/media/mediarecorder_test.html (right): https://codereview.chromium.org/1464313008/diff/1/content/test/data/media/mediarecorder_test.html#newcode194 content/test/data/media/mediarecorder_test.html:194: theRecorder.onstart = function(event) { While I think this ...
5 years ago (2015-11-30 20:42:56 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1464313008/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1464313008/1
5 years ago (2015-11-30 21:08:46 UTC) #6
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years ago (2015-11-30 23:45:57 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1464313008/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1464313008/1
5 years ago (2015-11-30 23:50:46 UTC) #10
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years ago (2015-12-01 00:14:30 UTC) #12
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/3f39c25b888f3ccfb9a18e7d31ee20f0bcf89c58 Cr-Commit-Position: refs/heads/master@{#362288}
5 years ago (2015-12-01 00:15:26 UTC) #14
cpaulin (no longer in chrome)
5 years ago (2015-12-02 00:05:54 UTC) #15
Message was sent while issue was closed.
closing drafts

https://codereview.chromium.org/1464313008/diff/1/content/test/data/media/med...
File content/test/data/media/mediarecorder_test.html (right):

https://codereview.chromium.org/1464313008/diff/1/content/test/data/media/med...
content/test/data/media/mediarecorder_test.html:194: theRecorder.onstart =
function(event) {
On 2015/11/30 20:42:56, phoglund_chrome wrote:
> While I think this is fine, it would have been cool if the media recorder
> returned promises. Then you could just do theRecorder.start().then(...)

Acknowledged.

https://codereview.chromium.org/1464313008/diff/1/content/test/data/media/med...
content/test/data/media/mediarecorder_test.html:202: return waitFor('Make sure
the start event was received',
On 2015/11/30 20:42:56, phoglund_chrome wrote:
> I realized I actually had to read up a bit to understand this, even though
I've
> seen it in your previous patch. So when a .then returns a value, it gets
passed
> on to the next then, except if it's a promise, in which case it gets worked
into
> the chain.

Acknowledged.

Powered by Google App Engine
This is Rietveld 408576698