MediaRecorder: update to spec (3/3)

MediaRecorder: update to spec (3/3)

- removed MediaRecorderErrorEvent, using error Event instead. [1]
- removed default value of mimeType in the ctor options dictionary,
since the options themselves are optional [2]
- minor include + DEPS cleanup

BUG=262211, 571107
[1] https://github.com/w3c/mediacapture-record/issues/31
[2] https://github.com/w3c/mediacapture-record/issues/40

Committed: https://crrev.com/c6c912b245543da98a0f4e7b25d27af992fb2190
Cr-Commit-Position: refs/heads/master@{#367310}

[mcasas, 2015-12-21 18:15:26]

Description was changed from

==========
MediaRecorder: update to spec (3/3)

- removed MediaRecorderErrorEvent, using error Event instead. [1]
- removed default value of mimeType in the ctor options dictionary,
since the options themselves are optional [2]
- minor include + DEPS cleanup

BUG=262211

[1] https://github.com/w3c/mediacapture-record/issues/31
[2] https://github.com/w3c/mediacapture-record/issues/40
==========

to

==========
MediaRecorder: update to spec (3/3)

- removed MediaRecorderErrorEvent, using error Event instead. [1]
- removed default value of mimeType in the ctor options dictionary,
since the options themselves are optional [2]
- minor include + DEPS cleanup

BUG=262211, 571107

[1] https://github.com/w3c/mediacapture-record/issues/31
[2] https://github.com/w3c/mediacapture-record/issues/40
==========

[mcasas, 2015-12-21 18:20:31]

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

[mcasas, 2015-12-21 18:20:31]

peter@ PTAL (mostly removals, changes are in MediaRecorder.cpp)

[Peter Beverloo, 2015-12-22 11:59:35]

Code changes lg modulo the three questions below.

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp (right):

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:225: void
MediaRecorder::failOutOfMemory(const WebString& message)
A few thoughts:

  (1) I think it's unfortunate that we're losing the error messages, this
reduces clarity. But if this improves alignment with the other media drafts, ok.
  (2) The MediaRecorder::fail*() methods actually aren't being called by our
implementation. Do you plan to call them?
  (3) The three MediaRecorder::fail*() methods are now identical. Can we unify
them into a single implementation?

I'm confused about ErrorEvent - there seem to be two? Let me reply to the GitHub
issue on that note.

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl
(right):

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:8:
DOMString mimeType;  // Encoding mimeType.
micro nit, and sorry for not spotting this earlier - four space indent.

[mcasas, 2015-12-22 19:56:46]

Patchset #2 (id:20001) has been deleted

[mcasas, 2015-12-22 19:57:08]

Patchset #2 (id:40001) has been deleted

[mcasas, 2015-12-22 20:24:40]

peter@ PTAL

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp (right):

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:225: void
MediaRecorder::failOutOfMemory(const WebString& message)
On 2015/12/22 11:59:35, Peter Beverloo wrote:
> A few thoughts:
> 
>   (1) I think it's unfortunate that we're losing the error messages, this
> reduces clarity. But if this improves alignment with the other media drafts,
ok.

It is, I filed corresponding issues to track this situation:
https://github.com/w3c/mediacapture-record/issues/42
and
https://github.com/w3c/mediacapture-main/issues/298

>   (2) The MediaRecorder::fail*() methods actually aren't being called by our
> implementation. Do you plan to call them?

Yes, there's a parallel CL calling fallingIllegalStreamModification()
when there's a Track added or removed, see https://crrev.com/1543673002

>   (3) The three MediaRecorder::fail*() methods are now identical. Can we unify
> them into a single implementation?

We should! Let me do that.

> 
> I'm confused about ErrorEvent - there seem to be two? Let me reply to the
GitHub
> issue on that note.

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl
(right):

https://codereview.chromium.org/1540303003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:8:
DOMString mimeType;  // Encoding mimeType.
On 2015/12/22 11:59:35, Peter Beverloo wrote:
> micro nit, and sorry for not spotting this earlier - four space indent.

Done.

[mcasas, 2015-12-22 20:25:22]

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

[mcasas, 2015-12-22 20:25:23]

esprehn@  Blink's public/platform/WebMediaRecorderHandlerClient.h
-- no TBR this time ;)  --

[esprehn, 2015-12-22 21:42:25]

Lgtm, thanks!

[Peter Beverloo, 2015-12-23 13:13:46]

lgtm, thanks!

[mcasas, 2015-12-23 22:04:40]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2015-12-23 22:05:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1540303003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1540303003/60001

[commit-bot: I haz the power, 2015-12-23 23:41:41]

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

[commit-bot: I haz the power, 2015-12-23 23:41:42]

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

[mcasas, 2016-01-04 14:38:11]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2016-01-04 14:38:11]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1540303003/#ps80001
(title: "reverted DEPS to master")

[commit-bot: I haz the power, 2016-01-04 14:38:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1540303003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1540303003/80001

[commit-bot: I haz the power, 2016-01-04 16:04:21]

Description was changed from

==========
MediaRecorder: update to spec (3/3)

- removed MediaRecorderErrorEvent, using error Event instead. [1]
- removed default value of mimeType in the ctor options dictionary,
since the options themselves are optional [2]
- minor include + DEPS cleanup

BUG=262211, 571107

[1] https://github.com/w3c/mediacapture-record/issues/31
[2] https://github.com/w3c/mediacapture-record/issues/40
==========

to

==========
MediaRecorder: update to spec (3/3)

- removed MediaRecorderErrorEvent, using error Event instead. [1]
- removed default value of mimeType in the ctor options dictionary,
since the options themselves are optional [2]
- minor include + DEPS cleanup

BUG=262211, 571107

[1] https://github.com/w3c/mediacapture-record/issues/31
[2] https://github.com/w3c/mediacapture-record/issues/40
==========

[commit-bot: I haz the power, 2016-01-04 16:04:21]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-01-04 16:05:30]

Description was changed from

==========
MediaRecorder: update to spec (3/3)

- removed MediaRecorderErrorEvent, using error Event instead. [1]
- removed default value of mimeType in the ctor options dictionary,
since the options themselves are optional [2]
- minor include + DEPS cleanup

BUG=262211, 571107

[1] https://github.com/w3c/mediacapture-record/issues/31
[2] https://github.com/w3c/mediacapture-record/issues/40
==========

to

==========
MediaRecorder: update to spec (3/3)

- removed MediaRecorderErrorEvent, using error Event instead. [1]
- removed default value of mimeType in the ctor options dictionary,
since the options themselves are optional [2]
- minor include + DEPS cleanup

BUG=262211, 571107

[1] https://github.com/w3c/mediacapture-record/issues/31
[2] https://github.com/w3c/mediacapture-record/issues/40

Committed: https://crrev.com/c6c912b245543da98a0f4e7b25d27af992fb2190
Cr-Commit-Position: refs/heads/master@{#367310}
==========

[commit-bot: I haz the power, 2016-01-04 16:05:31]

Patchset 3 (id:??) landed as
https://crrev.com/c6c912b245543da98a0f4e7b25d27af992fb2190
Cr-Commit-Position: refs/heads/master@{#367310}