MediaRecorder: update to spec (2/3)

MediaRecorder: update to spec (2/3)

- s/DOMString canRecordMimeType/bool isTypeSupported/, see [1]
- s/mimeType/options dictionary/, related to [2]

Also, updated tests everywhere.

BUG=262211
[1] https://github.com/w3c/mediacapture-record/issues/10
[2] https://github.com/w3c/mediacapture-record/issues/19

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

[mcasas, 2015-12-09 00:10:30]

Patchset #2 (id:20001) has been deleted

[mcasas, 2015-12-09 00:10:41]

Patchset #1 (id:1) has been deleted

[mcasas, 2015-12-09 01:30:43]

Description was changed from

==========
[Experimental] MediaRecorder: s/mimeType/options dictionary/,
s/canRecordMimeType/isTypeSupported/

Updated tests everywhere.

BUG=262211
==========

to

==========
MediaRecorder update to spec 2: s/mimeType/options dictionary/,
s/canRecordMimeType/isTypeSupported/

Also, updated tests everywhere.

BUG=262211
==========

[mcasas, 2015-12-09 19:06:02]

Description was changed from

==========
MediaRecorder update to spec 2: s/mimeType/options dictionary/,
s/canRecordMimeType/isTypeSupported/

Also, updated tests everywhere.

BUG=262211
==========

to

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

- s/DOMString canRecordMimeType/bool isTypeSupported/, see [1]
- s/mimeType/options dictionary/, related to [2]

Also, updated tests everywhere.

BUG=262211


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

[mcasas, 2015-12-09 19:09:04]

Patchset #1 (id:40001) has been deleted

[mcasas, 2015-12-10 19:56:06]

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

[mcasas, 2015-12-10 19:56:06]

peter@ PTAL 

More cleanup

[Peter Beverloo, 2015-12-10 20:28:53]

lgtm % comments - thanks!

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp (right):

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:165: // [1]
http://w3c.github.io/mediacapture-record/MediaRecorder.html#methods
nit: the [1] reference is gone

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl
(right):

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:5: //
https://w3c.github.io/mediacapture-record/MediaRecorder.html
append #MediaRecorderOptions, even though the patch hasn't made gh-pages yet?

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:8:
GarbageCollected,
[GC] doesn't do anything for dictionaries, they're completely auto-generated.

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:12:
unsigned long audioBitsPerSecond;
I think it would be best if we don't mention these types in the IDL yet until we
support them.

[mcasas, 2015-12-10 21:40:40]

Also reviewed spec links.

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp (right):

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:165: // [1]
http://w3c.github.io/mediacapture-record/MediaRecorder.html#methods
On 2015/12/10 20:28:53, Peter Beverloo wrote:
> nit: the [1] reference is gone

I was probably messing up with the github, I just
confirmed is OK.

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl
(right):

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:5: //
https://w3c.github.io/mediacapture-record/MediaRecorder.html
On 2015/12/10 20:28:53, Peter Beverloo wrote:
> append #MediaRecorderOptions, even though the patch hasn't made gh-pages yet?

Just made it!
http://w3c.github.io/mediacapture-record/MediaRecorder.html
has all the extra corrections of this patch and the previous
https://codereview.chromium.org/1497883002/.

Will update.

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:8:
GarbageCollected,
On 2015/12/10 20:28:53, Peter Beverloo wrote:
> [GC] doesn't do anything for dictionaries, they're completely auto-generated.

Done.

https://codereview.chromium.org/1507183002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorderOptions.idl:12:
unsigned long audioBitsPerSecond;
On 2015/12/10 20:28:53, Peter Beverloo wrote:
> I think it would be best if we don't mention these types in the IDL yet until
we
> support them.

Done.

[mcasas, 2015-12-10 23:40:47]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2015-12-10 23:40:48]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/1507183002/#ps100001
(title: "Rebased to http://crrev.com/1497883002 (just landed)")

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

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

[commit-bot: I haz the power, 2015-12-11 01:39:11]

Description was changed from

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

- s/DOMString canRecordMimeType/bool isTypeSupported/, see [1]
- s/mimeType/options dictionary/, related to [2]

Also, updated tests everywhere.

BUG=262211


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

to

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

- s/DOMString canRecordMimeType/bool isTypeSupported/, see [1]
- s/mimeType/options dictionary/, related to [2]

Also, updated tests everywhere.

BUG=262211


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

[commit-bot: I haz the power, 2015-12-11 01:39:12]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2015-12-11 01:40:13]

Description was changed from

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

- s/DOMString canRecordMimeType/bool isTypeSupported/, see [1]
- s/mimeType/options dictionary/, related to [2]

Also, updated tests everywhere.

BUG=262211


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

to

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

- s/DOMString canRecordMimeType/bool isTypeSupported/, see [1]
- s/mimeType/options dictionary/, related to [2]

Also, updated tests everywhere.

BUG=262211

[1] https://github.com/w3c/mediacapture-record/issues/10
[2] https://github.com/w3c/mediacapture-record/issues/19

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

[commit-bot: I haz the power, 2015-12-11 01:40:13]

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

[philipj_slow, 2015-12-11 08:29:03]

philipj@opera.com changed reviewers:
+ philipj@opera.com

[philipj_slow, 2015-12-11 08:29:03]

https://codereview.chromium.org/1507183002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp (right):

https://codereview.chromium.org/1507183002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:162: bool
MediaRecorder::isTypeSupported(const String& type)
Can you use this in the constructor, where the "the type provided ... is
unsupported" exception is thrown? That should cause an exception to be thrown by
default, highlighting the issue of having "" as the default value of mimeType.

If there needs to be any divergence between when
MediaRecorder.isTypeSupported(x) returns false and when new MediaRecorder(ms, {
mimeType: x }) throws an exception, can you document them clearly? My guess is
that there need be no such differences.

[philipj_slow, 2015-12-18 20:18:10]

https://codereview.chromium.org/1507183002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp (right):

https://codereview.chromium.org/1507183002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp:162: bool
MediaRecorder::isTypeSupported(const String& type)
On 2015/12/11 08:29:03, philipj wrote:
> Can you use this in the constructor, where the "the type provided ... is
> unsupported" exception is thrown? That should cause an exception to be thrown
by
> default, highlighting the issue of having "" as the default value of mimeType.
> 
> If there needs to be any divergence between when
> MediaRecorder.isTypeSupported(x) returns false and when new MediaRecorder(ms,
{
> mimeType: x }) throws an exception, can you document them clearly? My guess is
> that there need be no such differences.

I've filed https://code.google.com/p/chromium/issues/detail?id=571107