Bug 203667

Summary: Integrate media code to use HTML5 event loop
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: MediaAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: calvaris, cdumez, dbates, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, gyuyoung.kim, jer.noble, jonlee, kangil.han, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 203714, 204335    
Bug Blocks: 202843, 164193    
Attachments:
Description Flags
WIP
none
WIP2
none
WIP3
none
WIP4
none
WIP5 none

Ryosuke Niwa
Reported 2019-10-31 00:40:16 PDT
WindowEventLoop and WorkerEventLoop has the nice property to preserve the order of events even when the documents go into a page cache. Make media code use WindowEventLoop instead of GenericEventQueue. This will also allow us to avoid triggering idle callback while there is a pending media events.
Attachments
WIP (10.03 KB, patch)
2019-10-31 01:01 PDT, Ryosuke Niwa
no flags
WIP2 (12.41 KB, patch)
2019-10-31 19:14 PDT, Ryosuke Niwa
no flags
WIP3 (48.03 KB, patch)
2019-11-08 00:28 PST, Ryosuke Niwa
no flags
WIP4 (75.81 KB, patch)
2019-11-09 00:18 PST, Ryosuke Niwa
no flags
WIP5 (76.31 KB, patch)
2019-11-09 00:42 PST, Ryosuke Niwa
no flags
Ryosuke Niwa
Comment 1 2019-10-31 01:01:55 PDT
Ryosuke Niwa
Comment 2 2019-10-31 01:03:23 PDT
Comment on attachment 382438 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=382438&action=review > Source/WebCore/html/track/TrackListBase.cpp:103 > + m_element->document().eventLoop().queueTaskToDispatchEvent(TaskSource::MediaElementEventTaskSource, *this, This is a bit verbose. I think we can add a helper function on ActiveDOMObject so that we can call it like this: queueTaskToDispatchEventOnThis(TaskSource::MediaElementEventTaskSource, TrackEvent::create(eventName, Event::CanBubble::No, Event::IsCancelable::No, WTFMove(track)));
Ryosuke Niwa
Comment 3 2019-10-31 19:14:49 PDT
Radar WebKit Bug Importer
Comment 4 2019-11-04 19:44:05 PST
Geoffrey Garen
Comment 5 2019-11-05 11:45:07 PST
Comment on attachment 382539 [details] WIP2 View in context: https://bugs.webkit.org/attachment.cgi?id=382539&action=review > Source/WebCore/dom/ActiveDOMObject.h:122 > + void queueTaskToDispatchEventOnThis(EventTargetType& target, TaskSource source, Ref<EventType>&& event, No need to say "OnThis", since you pass the target explicitly.
Ryosuke Niwa
Comment 6 2019-11-05 14:52:02 PST
(In reply to Geoffrey Garen from comment #5) > Comment on attachment 382539 [details] > WIP2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382539&action=review > > > Source/WebCore/dom/ActiveDOMObject.h:122 > > + void queueTaskToDispatchEventOnThis(EventTargetType& target, TaskSource source, Ref<EventType>&& event, > > No need to say "OnThis", since you pass the target explicitly. Oh yeah, that suffix got removed.
Ryosuke Niwa
Comment 7 2019-11-08 00:28:15 PST
Created attachment 383116 [details] WIP3 A few observations: * Media elements can move from one document to another so we need to support event loop's task changing its associated script execution context (otherwise media/adopt-node-crash.html would crash) * We need to make DeferrableTask work with EventLoop. * We need a way of grouping tasks together so that we can later cancel them all, or check if there is a pending task in the same group or not.
Ryosuke Niwa
Comment 8 2019-11-08 00:30:16 PST
I've started to think that we may want to make "EventLoopGroup" which can be suspended or stopped, which is implemented by ScriptExecutionContext. This way, the mechanism by which a group of tasks are suspended or stopped is shared between script execution context & other kinds of grouping. One complication is that we'd have to support a task moving from one group to another (so that media elements that move from one document to another would belong to the right group after the move).
Ryosuke Niwa
Comment 9 2019-11-08 00:34:38 PST
(In reply to Ryosuke Niwa from comment #8) > I've started to think that we may want to make "EventLoopGroup" which can be > suspended or stopped, which is implemented by ScriptExecutionContext. > > This way, the mechanism by which a group of tasks are suspended or stopped > is shared between script execution context & other kinds of grouping. > > One complication is that we'd have to support a task moving from one group > to another (so that media elements that move from one document to another > would belong to the right group after the move). What's interesting about this approach is that this will allow us to merge WorkerEventLoop and WindowEventLoop implementations as well; because in this world, what suspends, resumes, or stops will be a EventLoopGroup, not ScriptExecutionContext.
Ryosuke Niwa
Comment 10 2019-11-09 00:18:37 PST
Ryosuke Niwa
Comment 11 2019-11-09 00:42:22 PST
Ryosuke Niwa
Comment 12 2023-08-05 22:27:30 PDT
Chris has done this already.
Note You need to log in before you can comment on or make changes to this bug.