Bug 217316 – Make sure to fire the correct set of events in case MediaRecorder stream has track changes

• Component: WebRTC
• Status: RESOLVED FIXED

[youenn fablet]

WebKit MediaRecorderPrivate::startRecording callback should not set audio/video tracks if recorder is stopped

[youenn fablet]

Or maybe add a clean method for MediaRecorderPrivate and call it in MediaRecorder::fetchData fetchDataCallback

[youenn fablet]

Make sure to fire the correct set of events in case MediaRecorder stream has track changes

[youenn fablet]

Created attachment 410620 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[youenn fablet]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/26006

[youenn fablet]

Created attachment 410631 [details]
Patch

[Eric Carlson]

Comment on attachment 410631 [details]
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-error.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-error.html:60
> +            assert_equals(mediaRecorderErrorEvent.error.name, 'InvalidModificationError', 'the type of error should be UnknownError when track has been added or removed');

Ditto.

[youenn fablet]

(In reply to Eric Carlson from comment #7)
> Comment on attachment 410631 [details]
> 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.

I filed https://bugs.webkit.org/show_bug.cgi?id=217387 to keep track of this refactoring.

[youenn fablet]

Created attachment 410643 [details]
Patch for landing

[EWS]

Committed r268119: <https://trac.webkit.org/changeset/268119>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410643 [details].

[Radar WebKit Bug Importer]

<rdar://problem/70038607>