Bug 128402 - Implement DataCue for metadata cues
Summary: Implement DataCue for metadata cues
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brendan Long
URL:
Keywords:
Depends on:
Blocks: 128412 122001
  Show dependency treegraph
 
Reported: 2014-02-07 14:46 PST by Brendan Long
Modified: 2014-02-12 12:34 PST (History)
15 users (show)

See Also:


Attachments
Patch (28.45 KB, patch)
2014-02-10 17:36 PST, Brendan Long
no flags Details | Formatted Diff | Diff
Patch (30.55 KB, patch)
2014-02-10 19:56 PST, Brendan Long
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (528.81 KB, application/zip)
2014-02-10 22:22 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (655.56 KB, application/zip)
2014-02-10 23:26 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (575.44 KB, application/zip)
2014-02-11 01:12 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (513.13 KB, application/zip)
2014-02-11 02:30 PST, Build Bot
no flags Details
Patch (39.36 KB, patch)
2014-02-11 09:00 PST, Brendan Long
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (508.08 KB, application/zip)
2014-02-11 10:36 PST, Build Bot
no flags Details
Patch (41.40 KB, patch)
2014-02-11 10:56 PST, Brendan Long
no flags Details | Formatted Diff | Diff
Patch (41.86 KB, patch)
2014-02-11 12:37 PST, Brendan Long
no flags Details | Formatted Diff | Diff
Patch (42.41 KB, patch)
2014-02-12 11:47 PST, Brendan Long
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan Long 2014-02-07 14:46:36 PST
See:

http://www.w3.org/html/wg/drafts/html/CR/embedded-content-0.html#datacue

This allows us to add binary metadata cues.
Comment 1 Brendan Long 2014-02-10 17:36:24 PST
Created attachment 223778 [details]
Patch
Comment 2 Brendan Long 2014-02-10 17:38:07 PST
The implementation of .text leaves something to be desired, but I have no idea how to do content sniffing. For in-band DataCues, we could use a constructor that takes a String instead of an ArrayBuffer, but I wasn't sure if that should be added now.
Comment 3 Brendan Long 2014-02-10 19:56:39 PST
Created attachment 223798 [details]
Patch

This one should make JSDataCue build correctly for Mac WebKit.
Comment 4 Build Bot 2014-02-10 22:22:52 PST
Comment on attachment 223798 [details]
Patch

Attachment 223798 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4829588383334400

New failing tests:
media/track/track-datacue.html
js/dom/global-constructors-attributes.html
Comment 5 Build Bot 2014-02-10 22:22:54 PST
Created attachment 223810 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2014-02-10 23:26:45 PST
Comment on attachment 223798 [details]
Patch

Attachment 223798 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5609549880885248

New failing tests:
media/track/track-datacue.html
js/dom/global-constructors-attributes.html
Comment 7 Build Bot 2014-02-10 23:26:50 PST
Created attachment 223814 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Build Bot 2014-02-11 01:12:06 PST
Comment on attachment 223798 [details]
Patch

Attachment 223798 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5744471312433152

New failing tests:
media/track/track-datacue.html
js/dom/global-constructors-attributes.html
Comment 9 Build Bot 2014-02-11 01:12:12 PST
Created attachment 223824 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 10 Build Bot 2014-02-11 02:30:47 PST
Comment on attachment 223798 [details]
Patch

Attachment 223798 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6546895086288896

New failing tests:
media/track/track-datacue.html
js/dom/global-constructors-attributes.html
Comment 11 Build Bot 2014-02-11 02:30:51 PST
Created attachment 223832 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 12 Brendan Long 2014-02-11 09:00:23 PST
Created attachment 223870 [details]
Patch

This should fix the test failures.
Comment 13 Eric Carlson 2014-02-11 10:32:23 PST
Comment on attachment 223870 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223870&action=review

> Source/WebCore/html/track/DataCue.cpp:55
> +    return const_cast<DataCue*>(toDataCue(const_cast<const TextTrackCue*>(cue)));

Ick, that is a mouthful. Is it really worth the double cast so you can call the other form of toDataCue and not duplicate the ASSERT here?

> Source/WebCore/html/track/TextTrack.cpp:283
>  
> +    if (cue->cueType() == TextTrackCue::Data && kind() != metadataKeyword()) {

Nit: I think it would be helpful to include the spec text here as a comment.
Comment 14 Build Bot 2014-02-11 10:36:18 PST
Comment on attachment 223870 [details]
Patch

Attachment 223870 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5286323795525632

New failing tests:
js/dom/global-constructors-attributes.html
Comment 15 Build Bot 2014-02-11 10:36:22 PST
Created attachment 223878 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 16 Brendan Long 2014-02-11 10:56:49 PST
Created attachment 223881 [details]
Patch

Added spec text to addCue(), duplicated the ASSERT in toDataCue() instead of doing two const_casts, and added DataCue to one more test -expected.txt file.
Comment 17 Brendan Long 2014-02-11 12:06:26 PST
Here's the windows build failure:

     1>WebCore.lib(JSBindingsAllInOne.obj) : error LNK2019: unresolved external symbol "public: static class JSC::JSObject * __cdecl WebCore::JSDataCue::createPrototype(class JSC::VM &,class JSC::JSGlobalObject *)" (?createPrototype@JSDataCue@WebCore@@SAPAVJSObject@JSC@@AAVVM@4@PAVJSGlobalObject@4@@Z) referenced in function "class JSC::Structure * __cdecl WebCore::getDOMStructure<class WebCore::JSDataCue>(class JSC::VM &,class WebCore::JSDOMGlobalObject *)" (??$getDOMStructure@VJSDataCue@WebCore@@@WebCore@@YAPAVStructure@JSC@@AAVVM@2@PAVJSDOMGlobalObject@0@@Z)
     1>WebCore.lib(JSBindingsAllInOne.obj) : error LNK2019: unresolved external symbol "protected: __thiscall WebCore::JSDataCue::JSDataCue(class JSC::Structure *,class WebCore::JSDOMGlobalObject *,class WTF::PassRefPtr<class WebCore::DataCue>)" (??0JSDataCue@WebCore@@IAE@PAVStructure@JSC@@PAVJSDOMGlobalObject@1@V?$PassRefPtr@VDataCue@WebCore@@@WTF@@@Z) referenced in function "public: static class WebCore::JSDataCue * __cdecl WebCore::JSDataCue::create(class JSC::Structure *,class WebCore::JSDOMGlobalObject *,class WTF::PassRefPtr<class WebCore::DataCue>)" (?create@JSDataCue@WebCore@@SAPAV12@PAVStructure@JSC@@PAVJSDOMGlobalObject@2@V?$PassRefPtr@VDataCue@WebCore@@@WTF@@@Z)
     1>WebCore.lib(JSBindingsAllInOne.obj) : error LNK2019: unresolved external symbol "protected: void __thiscall WebCore::JSDataCue::finishCreation(class JSC::VM &)" (?finishCreation@JSDataCue@WebCore@@IAEXAAVVM@JSC@@@Z) referenced in function "public: static class WebCore::JSDataCue * __cdecl WebCore::JSDataCue::create(class JSC::Structure *,class WebCore::JSDOMGlobalObject *,class WTF::PassRefPtr<class WebCore::DataCue>)" (?create@JSDataCue@WebCore@@SAPAV12@PAVStructure@JSC@@PAVJSDOMGlobalObject@2@V?$PassRefPtr@VDataCue@WebCore@@@WTF@@@Z)
     1>WebCore.lib(JSBindingsAllInOne.obj) : error LNK2001: unresolved external symbol "protected: static struct JSC::ClassInfo const WebCore::JSDataCue::s_info" (?s_info@JSDataCue@WebCore@@1UClassInfo@JSC@@B)
     1>WebCore.lib(DerivedSources.obj) : error LNK2019: unresolved external symbol "public: static class JSC::JSValue __cdecl WebCore::JSDataCue::getConstructor(class JSC::VM &,class JSC::JSGlobalObject *)" (?getConstructor@JSDataCue@WebCore@@SA?AVJSValue@JSC@@AAVVM@4@PAVJSGlobalObject@4@@Z) referenced in function "public: static bool __cdecl WebCore::JSDOMWindowConstructor::getOwnPropertySlot(class JSC::JSObject *,class JSC::ExecState *,class JSC::PropertyName,class JSC::PropertySlot &)" (?getOwnPropertySlot@JSDOMWindowConstructor@WebCore@@SA_NPAVJSObject@JSC@@PAVExecState@4@VPropertyName@4@AAVPropertySlot@4@@Z)
     1>C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\bin32\WebKit.dll : fatal error LNK1120: 5 unresolved externals
     1>Done Building Project "C:\cygwin\home\buildbot\WebKit\Source\WebKit\WebKit.vcxproj\WebKit\WebKit.vcxproj" (Build target(s)) -- FAILED.

I'm not sure where to add JSDataCue or DataCue.idl. "git grep git grep TextTrackCue Source/WebCore/WebCore.vcxproj/" didn't show any JS* or *.idl files.
Comment 18 Brendan Long 2014-02-11 12:37:25 PST
Created attachment 223890 [details]
Patch

This might fix the Windows build? I added DataCue.h to another part of the project file.
Comment 19 Brendan Long 2014-02-12 11:47:34 PST
Created attachment 223988 [details]
Patch

Added JSDataCue.cpp to DerivedSources.cpp
Comment 20 WebKit Commit Bot 2014-02-12 12:10:24 PST
Comment on attachment 223988 [details]
Patch

Rejecting attachment 223988 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 223988, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/6610797220331520
Comment 21 WebKit Commit Bot 2014-02-12 12:34:24 PST
Comment on attachment 223988 [details]
Patch

Clearing flags on attachment: 223988

Committed r163974: <http://trac.webkit.org/changeset/163974>
Comment 22 WebKit Commit Bot 2014-02-12 12:34:28 PST
All reviewed patches have been landed.  Closing bug.