After start running the MedisRecorder, if any of the track is added or removed, the 'onerror' event will be called on javascript, and the state of MediaRecorder will be inactive.
Created attachment 351991 [details] Patch
Wendy, could you make any new MediaRecorder bug (this one and bug 190291) as blocking bug 85851? That will help bug 85851 observers to see progress in the implementation.
Comment on attachment 351991 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351991&action=review > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:79 > + dispatchEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No)); It should probably be asynchronous and dispatch a MediaRecorderErrorEvent. Spec is unclear though. I filed https://github.com/w3c/mediacapture-record/issues/151. In the meantime, dispatching Unknown exception might be good enough. > Source/WebCore/Modules/mediarecorder/MediaRecorder.h:36 > +class MediaRecorder final : public ActiveDOMObject, public RefCounted<MediaRecorder>, public EventTargetWithInlineData, public MediaStream::Observer { Can you make it private MediaStream::Observer? > Source/WebCore/Modules/mediarecorder/MediaRecorder.h:74 > + void didAddOrRemoveTrack(); final
Comment on attachment 351991 [details] Patch Attachment 351991 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/9523081 New failing tests: fast/mediacapturefromelement/CanvasCaptureMediaStream-imagebitmaprenderingcontext.html fast/mediacapturefromelement/CanvasCaptureMediaStream-framerate-0.html fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-element.html
Created attachment 351997 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 351991 [details] Patch Attachment 351991 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9523703 New failing tests: fast/mediacapturefromelement/CanvasCaptureMediaStream-imagebitmaprenderingcontext.html fast/mediacapturefromelement/CanvasCaptureMediaStream-framerate-0.html fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-element.html
Created attachment 351999 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 351991 [details] Patch Attachment 351991 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/9523717 New failing tests: fast/mediacapturefromelement/CanvasCaptureMediaStream-imagebitmaprenderingcontext.html fast/mediacapturefromelement/CanvasCaptureMediaStream-framerate-0.html fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-element.html
Created attachment 352001 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 351991 [details] Patch Attachment 351991 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/9525318 New failing tests: fast/mediacapturefromelement/CanvasCaptureMediaStream-imagebitmaprenderingcontext.html fast/mediacapturefromelement/CanvasCaptureMediaStream-framerate-0.html fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-element.html
Created attachment 352009 [details] Archive of layout-test-results from ews112 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 351991 [details] Patch Attachment 351991 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9525559 New failing tests: fast/mediacapturefromelement/CanvasCaptureMediaStream-imagebitmaprenderingcontext.html fast/mediacapturefromelement/CanvasCaptureMediaStream-framerate-0.html fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-element.html
Created attachment 352012 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 351991 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351991&action=review > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:48 > + m_stream->addObserver(this); Isn't it concerning that MediaRecorder adds itself as an observer but never unregisters itself? I would have expected a matching unregisterObserver() call in MediaRecorder's destructor. > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:64 > + return Exception { InvalidStateError }; Please provide a useful error message (second parameter to Exception constructor). Also, your test does not seem to cover this case. > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:66 > + timeslice = std::numeric_limits<int>::max(); unused? > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:74 > + if (scriptExecutionContext()->activeDOMObjectsAreSuspended() || scriptExecutionContext()->activeDOMObjectsAreStopped()) What guarantees that scriptExecutionContext() does not return nullptr? >> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:79 >> + dispatchEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No)); > > It should probably be asynchronous and dispatch a MediaRecorderErrorEvent. > Spec is unclear though. I filed https://github.com/w3c/mediacapture-record/issues/151. > In the meantime, dispatching Unknown exception might be good enough. I agree this should fire a MediaRecorderErrorEvent: https://w3c.github.io/mediacapture-record/#error-handling-principles I also agree firing events asynchronously is better practice. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:2 > +<title>MediaRecorder Error</title> Could please add appropriate <html> <head> <body> elements? > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:11 > + ac = new AudioContext(); Why is this global? > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:19 > + canvas = document.getElementById("canvas"); Why is this global? > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:24 > + callError = false; Bad naming: Should be something like "wasErrorHandlerCalled" > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:25 > + video = createVideoStream(); Why is this global? > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:26 > + audio = createAudioStream(); ditto. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:27 > + recorder = new MediaRecorder(video); ditto. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:29 > + callError = true; Ideally, you should do some basic checks on the error event as well. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:35 > + assert_true(callError, "onerror should be fired"); "onerror should be fired" does not make much sense given that onerror is an event handler, not an event. It should either be: - onerror should be called or my preferred one - error event should be fired > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:36 > + }, "MediaRecorder will stop recording when any of track is added and onerror event will be fired"); onerror -> error > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:39 > + callError = false; Bad naming: Should be something like "wasErrorHandlerCalled" > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:40 > + video = createVideoStream(); Why is this global? > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:41 > + recorder = new MediaRecorder(video); ditto. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:43 > + callError = true; Ideally, you should do some basic checks on the error event as well. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:49 > + assert_true(callError, "onerror should be fired"); onerror -> error > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:50 > + }, "MediaRecorder will stop recording when any of track is removed and onerror event will be fired"); onerror -> error
Comment on attachment 351991 [details] Patch Attachment 351991 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9527389 New failing tests: fast/mediacapturefromelement/CanvasCaptureMediaStream-imagebitmaprenderingcontext.html fast/mediacapturefromelement/CanvasCaptureMediaStream-framerate-0.html fast/mediacapturefromelement/CanvasCaptureMediaStream-capture-out-of-DOM-element.html
Created attachment 352023 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 352079 [details] Patch
Created attachment 352083 [details] Patch
Comment on attachment 352079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352079&action=review > Source/WebCore/ChangeLog:6 > + Reviewed by Youenn Fablet and Chris Dumez (OOPS!). You are only supposed to update this once you got a r+, did you get a r+ from Youenn? I know I did not :) Also, you're supposed to drop the (OOPS!). > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:57 > + m_state = RecordingState::Inactive; There is no need to reset the state in the destructor. > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:85 > + if (state() != RecordingState::Inactive) { Should likely be an early return as per coding style: if (state() == RecordingState::Inactive) return; > Source/WebCore/Modules/mediarecorder/MediaRecorder.h:77 > + void didAddOrRemoveTrack() final; Please do not put methods in between the data members. Data members should always come last. > Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp:48 > + , m_domError(init.error) init.error.releaseNonNull() > Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h:37 > + DOMException* error() const { return m_domError.get(); } We should probably move this one after the create() factories for clarity. Also, it can return a DOMException& since I do not think the error can be null. > Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h:44 > + static Ref<MediaRecorderErrorEvent> create(const AtomicString&, CanBubble, IsCancelable, Exception&&); I think we can omit the CanBubble, IsCancelable parameters and use the values defined in the spec inside the constructor. > Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h:47 > + EventInterface eventInterface() const override; Can this be private? > Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h:53 > + RefPtr<DOMException> m_domError; Can probably be a Ref<>, I do not think it can be null. > LayoutTests/ChangeLog:6 > + Reviewed by Youenn Fablet and Chris Dumez (OOPS!). Same comment as earlier. > LayoutTests/imported/w3c/ChangeLog:6 > + Reviewed by Youenn Fablet and Chris Dumez (OOPS!). Same comment as earlier.
Created attachment 352084 [details] Patch
Created attachment 352095 [details] Patch
Comment on attachment 352084 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352084&action=review > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:83 > + return; MediaRecorder is an ActiveDOMObject. We should probably implement MediaRecorder ::stop() and store the fact that media recorder is stopped. Inside stop(), we would do the same stuff as done in MediaRecorder::~MediaRecorder. That will remove the need for this specific check, which might not always be safe as scriptExecutionContext() might sometimes be null. > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:87 > + auto event = MediaRecorderErrorEvent::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No, Exception { UnknownError, "Track cannot be added to or removed from the MediaStream while recording is happening"_s }); As Chris said, Event::CanBubble::No, Event::IsCancelable::No should probably not be needed here. > Source/WebCore/Modules/mediarecorder/MediaRecorder.h:30 > +#include "GenericEventQueue.h" We probably can forward declare GenericEventQueue. > Source/WebCore/Modules/mediarecorder/MediaRecorder.h:48 > + virtual ~MediaRecorder(); s/virtual// > LayoutTests/ChangeLog:8 > + Since dataavailable event has not been implemented, skip three MediaRecorder-related tests now, s/now/now./ > LayoutTests/ChangeLog:9 > + and need to reenable those tests once implementing the dataavailable event for MediaRecorder. /and need to/We will / > LayoutTests/ChangeLog:13 > + * platform/mac/TestExpectations: It might be simpler to skip them in LayoutTests/TestExpectations. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:30 > + recorder.onerror = t.step_func(function (mediaRecorderErrorEvent) { You can also use arrow function which preserves 'this' and is shorter than function. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:41 > + }, 500); 500 milliseconds might be too small for some very slow bots. Let's bump it to 2 seconds. Ditto below. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:61 > + async_test(t => { It does not need to be an async test. A regular test will be good enough.
Created attachment 352108 [details] Patch
Comment on attachment 352108 [details] Patch This patch is almost ready. Can you verify whether the state should be changed asynchronously or not? If so, can you update the patch accordingly? View in context: https://bugs.webkit.org/attachment.cgi?id=352108&action=review > Source/WebCore/ChangeLog:22 > + * Modules/mediarecorder/MediaRecorderErrorEvent.cpp: Copied from Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp. s/Copied from.../Added/ > Source/WebCore/ChangeLog:27 > + * Modules/mediarecorder/MediaRecorderErrorEvent.idl: Copied from Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp. s/Copied from.../Added/ > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:91 > + m_state = RecordingState::Inactive; m_state should probable be modified asynchronously as per spec. Maybe this call to use a deferred task directly to do both state change and event dispatch. > Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp:48 > + , m_domError(init.error.releaseNonNull()) init is being moved but domError is also using it. While it might work, it seems fragile. I would change the caller (MediaRecorderErrorEvent::create) so as to pass Exception&&. Something like auto domError = init.error.releaseNonNull(); ... new MediaRecorderErrorEvent(type, WTFMove(init), WTFMove(domError), isTrusted); Also do we need isTrusted? Can we just use IsTrusted::No and not pass this parameter? > Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.idl:32 > + Exposed=Window This should be: EnabledAtRuntime=MediaRecorder, > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:38 > + assert_equals(recorder.state, "inactive", "MediaRecorder has been stopped after adding a track to stream"); This is probably wrong, this should be checked in recorder.onerror since state should be changed asynchronously. > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:67 > + recorder.start(); Nice!
Comment on attachment 352108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352108&action=review >> Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp:48 >> + , m_domError(init.error.releaseNonNull()) > > init is being moved but domError is also using it. > While it might work, it seems fragile. > I would change the caller (MediaRecorderErrorEvent::create) so as to pass Exception&&. > Something like > auto domError = init.error.releaseNonNull(); > ... > new MediaRecorderErrorEvent(type, WTFMove(init), WTFMove(domError), isTrusted); > > Also do we need isTrusted? Can we just use IsTrusted::No and not pass this parameter? It is an event fired by the user-agent so I think it should be *Trusted*. Untrusted ones are fired by JS.
(In reply to Chris Dumez from comment #25) > Comment on attachment 352108 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=352108&action=review > > >> Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp:48 > >> + , m_domError(init.error.releaseNonNull()) > > > > init is being moved but domError is also using it. > > While it might work, it seems fragile. > > I would change the caller (MediaRecorderErrorEvent::create) so as to pass Exception&&. > > Something like > > auto domError = init.error.releaseNonNull(); > > ... > > new MediaRecorderErrorEvent(type, WTFMove(init), WTFMove(domError), isTrusted); > > > > Also do we need isTrusted? Can we just use IsTrusted::No and not pass this parameter? > > It is an event fired by the user-agent so I think it should be *Trusted*. > Untrusted ones are fired by JS. So, to be clear, I think we should keep the IsTrustedParameter (set it to No by default for when it is created by JS), but pass IsTrusted::Yes from your C++ call site that fires the event.
Comment on attachment 352108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352108&action=review > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:32 > + assert_equals(mediaRecorderErrorEvent.error.name, 'UnknownError', 'the type of error should be UnknownError when track has been added or removed'); Let's add a check to ensure the event is trusted. Discussing with Chris, it is not clear whether the error event being created is or not.
Comment on attachment 352108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352108&action=review > LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html:32 > + assert_equals(mediaRecorderErrorEvent.error.name, 'UnknownError', 'the type of error should be UnknownError when track has been added or removed'); It would be good to check that mediaRecorderErrorEvent.isTrusted is true as well.
Created attachment 352195 [details] Patch
Comment on attachment 352195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352195&action=review > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:89 > + scheduleDeferredTask([this] { You need to check for (!m_isActive || state() == RecordingState::Inactive) here as it might have changed.
(In reply to youenn fablet from comment #31) > Comment on attachment 352195 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=352195&action=review > > > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:89 > > + scheduleDeferredTask([this] { > > You need to check for (!m_isActive || state() == RecordingState::Inactive) > here as it might have changed. The test should check that the state is active after changing the track and inactive in on error event handler.
Comment on attachment 352195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352195&action=review > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:90 > + auto event = MediaRecorderErrorEvent::create(eventNames().errorEvent, Exception { UnknownError, "Track cannot be added to or removed from the MediaStream while recording is happening"_s }); What if we're no longer active by then? > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:104 > + callOnMainThread([weakThis = makeWeakPtr(*this), function = WTFMove(function)] { Should probably be a ScriptExecutionContext::postTask()
Comment on attachment 352195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352195&action=review > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:54 > + if (m_isActive) These 2 lines basically reset m_isActive to false, which is not needed in a destructor.
Created attachment 352215 [details] Patch
Comment on attachment 352215 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352215&action=review LGTM but I have a suggestion. > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:59 > + m_isActive = false; Should we remove ourselves as an observer here as well? It seems like it'd be good to do it as early as possible. Then I think we would not need the m_isActive flag as didAddOrRemoveTrack() could not get called after we're no longer active.
Comment on attachment 352215 [details] Patch LGTM too :) View in context: https://bugs.webkit.org/attachment.cgi?id=352215&action=review >> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:59 >> + m_isActive = false; > > Should we remove ourselves as an observer here as well? It seems like it'd be good to do it as early as possible. > > Then I think we would not need the m_isActive flag as didAddOrRemoveTrack() could not get called after we're no longer active. We discussed this with Wendy yesterday and thought that this might not be worth. Contrary to observer being call for every frame, this observer will never be called. But we will in most cases try to remove the observer from the observer map twice.
(In reply to youenn fablet from comment #37) > Comment on attachment 352215 [details] > Patch > > LGTM too :) > > View in context: > https://bugs.webkit.org/attachment.cgi?id=352215&action=review > > >> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:59 > >> + m_isActive = false; > > > > Should we remove ourselves as an observer here as well? It seems like it'd be good to do it as early as possible. > > > > Then I think we would not need the m_isActive flag as didAddOrRemoveTrack() could not get called after we're no longer active. > > We discussed this with Wendy yesterday and thought that this might not be > worth. > Contrary to observer being call for every frame, this observer will never be > called. > But we will in most cases try to remove the observer from the observer map > twice. Ok
Comment on attachment 352215 [details] Patch Rejecting attachment 352215 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 352215, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=352215&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=190438&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Updating working directory Processing patch 352215 from bug 190438. Fetching: https://bugs.webkit.org/attachment.cgi?id=352215 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.idl A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error-expected.txt A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html M LayoutTests/ChangeLog M LayoutTests/TestExpectations M LayoutTests/imported/w3c/ChangeLog M Source/WebCore/CMakeLists.txt M Source/WebCore/ChangeLog M Source/WebCore/DerivedSources.make M Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp M Source/WebCore/Modules/mediarecorder/MediaRecorder.h M Source/WebCore/Modules/mediarecorder/MediaRecorder.idl M Source/WebCore/Sources.txt M Source/WebCore/WebCore.xcodeproj/project.pbxproj M Source/WebCore/bindings/js/WebCoreBuiltinNames.h M Source/WebCore/dom/EventNames.in M Source/WebCore/dom/EventTargetFactory.in ERROR from SVN: A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output: Commits are currently disabled while we update infrastructure. W: b86ad68395fa839c9ff66a6dd4e0396c139fd4af and refs/remotes/origin/master differ, using rebase: :040000 040000 77f97ebb1e4138c5024d28dea7700f347c8ba6bc 92eda367eb67787361b8f4da2a96959390505aa7 M LayoutTests :040000 040000 98a323ac310b04dd2019f6cfa11ecebf675dfe26 db349c7d53434e63104b36e19511df8a3253ad7d M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.idl A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error-expected.txt A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html M LayoutTests/ChangeLog M LayoutTests/TestExpectations M LayoutTests/imported/w3c/ChangeLog M Source/WebCore/CMakeLists.txt M Source/WebCore/ChangeLog M Source/WebCore/DerivedSources.make M Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp M Source/WebCore/Modules/mediarecorder/MediaRecorder.h M Source/WebCore/Modules/mediarecorder/MediaRecorder.idl M Source/WebCore/Sources.txt M Source/WebCore/WebCore.xcodeproj/project.pbxproj M Source/WebCore/bindings/js/WebCoreBuiltinNames.h M Source/WebCore/dom/EventNames.in M Source/WebCore/dom/EventTargetFactory.in ERROR from SVN: A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output: Commits are currently disabled while we update infrastructure. W: b86ad68395fa839c9ff66a6dd4e0396c139fd4af and refs/remotes/origin/master differ, using rebase: :040000 040000 77f97ebb1e4138c5024d28dea7700f347c8ba6bc 92eda367eb67787361b8f4da2a96959390505aa7 M LayoutTests :040000 040000 98a323ac310b04dd2019f6cfa11ecebf675dfe26 db349c7d53434e63104b36e19511df8a3253ad7d M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource Current branch master is up to date. Total errors found: 0 in 1 files Full output: https://webkit-queues.webkit.org/results/9559633
Comment on attachment 352215 [details] Patch Rejecting attachment 352215 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 352215, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=352215&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=190438&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Updating working directory Processing patch 352215 from bug 190438. Fetching: https://bugs.webkit.org/attachment.cgi?id=352215 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.idl A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error-expected.txt A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html M LayoutTests/ChangeLog M LayoutTests/TestExpectations M LayoutTests/imported/w3c/ChangeLog M Source/WebCore/CMakeLists.txt M Source/WebCore/ChangeLog M Source/WebCore/DerivedSources.make M Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp M Source/WebCore/Modules/mediarecorder/MediaRecorder.h M Source/WebCore/Modules/mediarecorder/MediaRecorder.idl M Source/WebCore/Sources.txt M Source/WebCore/WebCore.xcodeproj/project.pbxproj M Source/WebCore/bindings/js/WebCoreBuiltinNames.h M Source/WebCore/dom/EventNames.in M Source/WebCore/dom/EventTargetFactory.in ERROR from SVN: A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output: Commits are currently disabled while we update infrastructure. W: da2e2722190129d719e49bf2de56959ad68c7650 and refs/remotes/origin/master differ, using rebase: :040000 040000 77f97ebb1e4138c5024d28dea7700f347c8ba6bc 92eda367eb67787361b8f4da2a96959390505aa7 M LayoutTests :040000 040000 98a323ac310b04dd2019f6cfa11ecebf675dfe26 db349c7d53434e63104b36e19511df8a3253ad7d M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.cpp C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.h C Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp => Source/WebCore/Modules/mediarecorder/MediaRecorderErrorEvent.idl A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error-expected.txt A LayoutTests/imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-error.html M LayoutTests/ChangeLog M LayoutTests/TestExpectations M LayoutTests/imported/w3c/ChangeLog M Source/WebCore/CMakeLists.txt M Source/WebCore/ChangeLog M Source/WebCore/DerivedSources.make M Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp M Source/WebCore/Modules/mediarecorder/MediaRecorder.h M Source/WebCore/Modules/mediarecorder/MediaRecorder.idl M Source/WebCore/Sources.txt M Source/WebCore/WebCore.xcodeproj/project.pbxproj M Source/WebCore/bindings/js/WebCoreBuiltinNames.h M Source/WebCore/dom/EventNames.in M Source/WebCore/dom/EventTargetFactory.in ERROR from SVN: A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output: Commits are currently disabled while we update infrastructure. W: da2e2722190129d719e49bf2de56959ad68c7650 and refs/remotes/origin/master differ, using rebase: :040000 040000 77f97ebb1e4138c5024d28dea7700f347c8ba6bc 92eda367eb67787361b8f4da2a96959390505aa7 M LayoutTests :040000 040000 98a323ac310b04dd2019f6cfa11ecebf675dfe26 db349c7d53434e63104b36e19511df8a3253ad7d M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource Current branch master is up to date. Total errors found: 0 in 1 files Full output: https://webkit-queues.webkit.org/results/9559987
Comment on attachment 352215 [details] Patch Rejecting attachment 352215 [details] from commit-queue. yuhan_wu@apple.com does not have committer permissions according to https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
svn is down for maintenance. When up again, it will be cq+able again, probably by tomorrow or Sunday.
Comment on attachment 352215 [details] Patch Clearing flags on attachment: 352215 Committed r237106: <https://trac.webkit.org/changeset/237106>
All reviewed patches have been landed. Closing bug.
<rdar://problem/45271123>