[webkit-reviews] review granted: [Bug 217316] Make sure to fire the correct set of events in case MediaRecorder stream has track changes : [Attachment 410631] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 217316: Make sure to fire the correct set of events in case MediaRecorder
stream has track changes
https://bugs.webkit.org/show_bug.cgi?id=217316

Attachment 410631: Patch

https://bugs.webkit.org/attachment.cgi?id=410631&action=review




--- Comment #7 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 410631
  --> https://bugs.webkit.org/attachment.cgi?id=410631
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=410631&action=review

> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:71
>      if (result.hasException())
>	   return result.releaseException();
>  
> +    result.releaseReturnValue()->stop();
> +

As we discussed, I think it would be helpful to have a comment explaining why
the private recorder is allocated and abandoned immediately.

> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:283
> +	   dispatchError(Exception { InvalidModificationError, "Track cannot be
added to or removed from the MediaStream while recording is happening"_s });

s/while recording is happening/while recording/

>
LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-e
rror.html:41
> +	       assert_equals(mediaRecorderErrorEvent.error.name,
'InvalidModificationError', 'the type of error should be UnknownError when
track has been added or removed');

s/should be UnknownError when/should be InvalidModificationError when/

>
LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-e
rror.html:60
> +	       assert_equals(mediaRecorderErrorEvent.error.name,
'InvalidModificationError', 'the type of error should be UnknownError when
track has been added or removed');

Ditto.