The <track> element requires events to be dispatched asynchronously. Even though the HTMLMediaElement provides a particular mechanism for firing such events using a timer and a queue, a generic class should be in place that handles both events that are dispatched by the media element and the track element.
There's already a pretty nice EventQueue abstraction http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/dom/EventQueue.h&exact_package=chromium&q=EventQueue&type=cs&l=27. Might be worth taking a look.
Created attachment 123272 [details] Patch
Comment on attachment 123272 [details] Patch Attachment 123272 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11314006
Comment on attachment 123272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123272&action=review > Source/WebCore/html/HTMLMediaElement.h:581 > + MediaEventQueue m_asyncEventQueue; This shouldn't be inside of #if ENABLE(VIDEO_TRACK) as it is always used. This is causing the GTK and Qt bot failures.
Comment on attachment 123272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123272&action=review Marking r- only because it fails to compile on ports that don't define VIDEO_TRACK. > Source/WebCore/html/MediaEventQueue.cpp:8 > + Nit: This extra blank line is unnecessary. > Source/WebCore/html/MediaEventQueue.cpp:70 > + if (m_timer.isActive()) Nit: MediaEventQueue::close calls stop() without checking to see if the timer is active, to be consistent you should one or the other throughout the file. stop() is cheap so I would probably just call it but I don't have a strong preference.
Comment on attachment 123272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123272&action=review > Source/WebCore/html/MediaEventQueue.h:34 > +class MediaEventQueue : public EventQueue { > + > +public: > + MediaEventQueue(); > + virtual ~MediaEventQueue(); > + > + // EventQueue > + virtual bool enqueueEvent(PassRefPtr<Event>) OVERRIDE; > + virtual bool cancelEvent(Event*) OVERRIDE; > + virtual void close() OVERRIDE; > + > + void cancelAllEvents(); > + bool hasPendingEvents() const; > + > +private: > + void timerFired(Timer<MediaEventQueue>*); > + > + Vector<RefPtr<Event> > m_pendingEvents; > + Timer<MediaEventQueue> m_timer; > + > + bool m_isClosed; Why is this class called "MediaEventQueue"? We may end with a class that is specific to the media element at some point, but at this point it is just a generic event queue that could be used by other classes. I think this should a more generic name, GenericEventQueue or SimpleEventQueue perhaps, and it should be in WebCore/dom along with the abstract base class.
Created attachment 123407 [details] Updated patch. Renamed the class in GenericQueueEvent under dom/ and hopefully fixed build issues.
Created attachment 123416 [details] Updated the vcproj entry as it was not building on win-ews
Created attachment 123439 [details] Patch
Created attachment 123441 [details] Green patch Fixed build issues. Patch should be ready for review.
Comment on attachment 123441 [details] Green patch View in context: https://bugs.webkit.org/attachment.cgi?id=123441&action=review > Source/WebCore/html/HTMLMediaElement.cpp:-3181 > - // Return true when we have pending events so we can't fire events after the JS > - // object gets collected. Why remove this comment?
I tried building this before landing it, and this did not build for me on Mac. It looks like it needs symbols added to WebCore exports. Here is the tail of the build output: Ld /Volumes/HD2/WebKit2/WebKitBuild/Debug/WebCore.framework/Versions/A/WebCore normal x86_64 cd /Volumes/HD2/WebKit2/Source/WebCore setenv MACOSX_DEPLOYMENT_TARGET 10.7 /Developer/usr/bin/clang++ -arch x86_64 -dynamiclib -L/Volumes/HD2/WebKit2/WebKitBuild/Debug -F/Volumes/HD2/WebKit2/WebKitBuild/Debug -F/System/Library/Frameworks/Carbon.framework/Frameworks -F/System/Library/Frameworks/ApplicationServices.framework/Frameworks -F/System/Library/Frameworks/CoreServices.framework/Frameworks -F/System/Library/PrivateFrameworks -filelist /Volumes/HD2/WebKit2/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/x86_64/WebCore.LinkFileList -Xlinker --no-demangle -exported_symbols_list /Volumes/HD2/WebKit2/WebKitBuild/Debug/DerivedSources/WebCore/WebCore.LP64.exp -install_name /Volumes/HD2/WebKit2/WebKitBuild/Debug/WebCore.framework/Versions/A/WebCore -mmacosx-version-min=10.7 -lsqlite3 -lobjc -lANGLE -allowable_client WebCoreTestSupport -sub_library libobjc -umbrella WebKit -allowable_client WebKit2 -Xlinker -objc_gc_compaction -framework IOSurface -framework Accelerate -framework ApplicationServices -framework AudioToolbox -framework AudioUnit -framework Carbon -framework Cocoa -framework CoreAudio -framework IOKit -framework JavaScriptCore -licucore -lobjc -lxml2 -lz -framework OpenGL -framework QuartzCore -framework SystemConfiguration -single_module -compatibility_version 1 -current_version 535.19 -o /Volumes/HD2/WebKit2/WebKitBuild/Debug/WebCore.framework/Versions/A/WebCore Undefined symbols for architecture x86_64: "__ZN7WebCore17GenericEventQueueC1Ev", referenced from: __ZN7WebCore16HTMLMediaElementC2ERKNS_13QualifiedNameEPNS_8DocumentEb in HTMLMediaElement.o "__ZN7WebCore17GenericEventQueueD1Ev", referenced from: __ZN7WebCore16HTMLMediaElementD2Ev in HTMLMediaElement.o "__ZN7WebCore17GenericEventQueue15cancelAllEventsEv", referenced from: __ZN7WebCore16HTMLMediaElement31cancelPendingEventsAndCallbacksEv in HTMLMediaElement.o "__ZNK7WebCore17GenericEventQueue16hasPendingEventsEv", referenced from: __ZNK7WebCore16HTMLMediaElement18hasPendingActivityEv in HTMLMediaElement.o ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation) ** BUILD FAILED **
http://trac.webkit.org/changeset/106156