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

Issue 1497883002: MediaRecorder: update to spec (1/3) (Closed)

Created:
5 years ago by mcasas
Modified:
5 years ago
Reviewers:
Peter Beverloo
CC:
chromium-reviews, blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

MediaRecorder: update to spec (1/3) Inspired by CL: https://crrev.com/1490403003/ ;) - TreatUndefinedAs=missing is going to be removed from the spec [1] and is already not present in Gecko implementation [2] - Also, "BlobEvent.data is non-nullable, so the dict argument should be required, and the data argument should be required and non-nullable." [3] - Removing suffix 'Enum' from RecordingStateEnum [4]. - BlobEvent has a constructor now [5], so remove the TODO. BUG=262211 [1] https://github.com/w3c/mediacapture-record/issues/7 [2] https://github.com/mozilla/gecko-dev/blob/master/dom/webidl/MediaRecorder.webidl [3] https://groups.google.com/a/chromium.org/d/msg/blink-dev/76HB0BIxk_o/9ZK0yXirBwAJ [4] https://github.com/w3c/mediacapture-record/issues/8 [5] https://github.com/w3c/mediacapture-record/issues/11 Committed: https://crrev.com/918bd3b360632a246a14008876a6cedd56333469 Cr-Commit-Position: refs/heads/master@{#364518}

Patch Set 1 #

Patch Set 2 : s/eventInit/eventInitDict/ #

Total comments: 8

Patch Set 3 : peter@s comments: add tests for BlobEvent ctor and reordered MediaRecorder.idl to match spec #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -11 lines) Patch
M third_party/WebKit/LayoutTests/fast/mediarecorder/BlobEvent-basic.html View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/mediarecorder/BlobEvent.cpp View 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/mediarecorder/BlobEvent.idl View 1 1 chunk +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.idl View 1 2 2 chunks +4 lines, -5 lines 0 comments Download

Messages

Total messages: 27 (19 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1497883002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1497883002/40001
5 years ago (2015-12-08 01:17:07 UTC) #6
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/98311) linux_chromium_compile_dbg_ng on ...
5 years ago (2015-12-08 01:56:45 UTC) #8
mcasas
peter@ PTAL (minor changes)
5 years ago (2015-12-09 19:07:36 UTC) #15
Peter Beverloo
lgtm \o/ https://codereview.chromium.org/1497883002/diff/100001/third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl File third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl (right): https://codereview.chromium.org/1497883002/diff/100001/third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl#newcode6 third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl:6: required Blob data; nit: it might be ...
5 years ago (2015-12-09 23:11:29 UTC) #19
mcasas
https://codereview.chromium.org/1497883002/diff/100001/third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl File third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl (right): https://codereview.chromium.org/1497883002/diff/100001/third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl#newcode6 third_party/WebKit/Source/modules/mediarecorder/BlobEventInit.idl:6: required Blob data; On 2015/12/09 23:11:29, Peter Beverloo wrote: ...
5 years ago (2015-12-10 02:16:59 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1497883002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1497883002/120001
5 years ago (2015-12-10 22:15:36 UTC) #23
commit-bot: I haz the power
Committed patchset #3 (id:120001)
5 years ago (2015-12-10 23:21:54 UTC) #25
commit-bot: I haz the power
5 years ago (2015-12-10 23:23:10 UTC) #27
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/918bd3b360632a246a14008876a6cedd56333469
Cr-Commit-Position: refs/heads/master@{#364518}

Powered by Google App Engine
This is Rietveld 408576698