RESOLVED FIXED 213720
Support MediaRecorder.onstart 
https://bugs.webkit.org/show_bug.cgi?id=213720
Summary Support MediaRecorder.onstart
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
DetailsFormatted DiffDiff
Patch (7.32 KB, patch)
2020-06-29 05:46 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for relanding (7.80 KB, patch)
2020-07-03 06:16 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for relanding (7.79 KB, patch)
2020-07-03 07:26 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
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.