Bug 213720

Summary:

Support MediaRecorder.onstart

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebRTC

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, darin, eric.carlson, esprehn+autocc, ews-watchlist, glenn, jer.noble, kondapallykalyan, philipj, sergio, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

85851

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for relanding	none
Patch for relanding	none

youenn fablet

Reported 2020-06-29 02:59:18 PDT

Support MediaRecorder.onstart

Attachments
Patch (7.30 KB, patch) 2020-06-29 03:01 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (7.32 KB, patch) 2020-06-29 05:46 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for relanding (7.80 KB, patch) 2020-07-03 06:16 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for relanding (7.79 KB, patch) 2020-07-03 07:26 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2020-06-29 03:01:36 PDT

Created attachment 403043 [details] Patch

youenn fablet

Comment 2 2020-06-29 05:46:01 PDT

Created attachment 403052 [details] Patch

Darin Adler

Comment 3 2020-06-29 10:04:13 PDT

Comment on attachment 403052 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403052&action=review > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:129 > +ExceptionOr<void> MediaRecorder::startRecording(Optional<unsigned> timeSlice) It’s not really great to change the type here, but then store the value in m_timeSlice, an Optional<int>. Basically it’s the same incorrect conversion of large values to negative numbers, just done at a slightly later time.

youenn fablet

Comment 4 2020-06-29 10:42:38 PDT

Comment on attachment 403052 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403052&action=review >> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:129 >> +ExceptionOr<void> MediaRecorder::startRecording(Optional<unsigned> timeSlice) > > It’s not really great to change the type here, but then store the value in m_timeSlice, an Optional<int>. Basically it’s the same incorrect conversion of large values to negative numbers, just done at a slightly later time. This patch also updates m_timeSlice to be an Optional<unsigned>.

Darin Adler

Comment 5 2020-06-29 10:49:36 PDT

Comment on attachment 403052 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403052&action=review >>> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:129 >>> +ExceptionOr<void> MediaRecorder::startRecording(Optional<unsigned> timeSlice) >> >> It’s not really great to change the type here, but then store the value in m_timeSlice, an Optional<int>. Basically it’s the same incorrect conversion of large values to negative numbers, just done at a slightly later time. > > This patch also updates m_timeSlice to be an Optional<unsigned>. Oops, how did I overlook that?

EWS

Comment 6 2020-06-29 11:32:15 PDT

Committed r263671: <https://trac.webkit.org/changeset/263671> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403052 [details].

Radar WebKit Bug Importer

Comment 7 2020-06-29 11:33:14 PDT

<rdar://problem/64899889>

Ryan Haddad

Comment 8 2020-07-02 12:35:08 PDT

Reverted r263633, r263651, and r263671 for reason: Still seeing MediaRecorder test crashes after re-landing r263633 Committed r263854: <https://trac.webkit.org/changeset/263854>

youenn fablet

Comment 9 2020-07-03 06:16:47 PDT

Created attachment 403455 [details] Patch for relanding

youenn fablet

Comment 10 2020-07-03 06:18:08 PDT

(In reply to youenn fablet from comment #9) > Created attachment 403455 [details] > Patch for relanding Relanding, with the modification of enqueuing a task to dispatch error/start events given the startLoading callback can be called synchronously.

EWS

Comment 11 2020-07-03 07:14:41 PDT

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

youenn fablet

Comment 12 2020-07-03 07:26:41 PDT

Created attachment 403457 [details] Patch for relanding

EWS

Comment 13 2020-07-03 07:55:17 PDT

Committed r263896: <https://trac.webkit.org/changeset/263896> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403457 [details].

Note You need to log in before you can comment on or make changes to this bug.