Bug 69460

Summary: Turn ENABLE_VIDEO_TRACK on for chromium.
Product: WebKit Reporter: Anna Cavender <annacc>
Component: MediaAssignee: Anna Cavender <annacc>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, jamesr, levin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 69573, 69602, 69603    
Bug Blocks: 43668    
Attachments:
Description Flags
Patch
none
fix for compiler () preferences
none
Patch for landing
none
Patch
none
Patch none

Anna Cavender
Reported 2011-10-05 13:43:50 PDT
Everything <track>-related should now be runtime enabled and off by default. Now we just need to turn ENABLE_VIDEO_TRACK on in order to enable it at runtime.
Attachments
Patch (1.14 KB, patch)
2011-10-05 14:45 PDT, Anna Cavender 		no flags
DetailsFormatted DiffDiff
fix for compiler () preferences (2.00 KB, patch)
2011-10-05 15:38 PDT, Anna Cavender 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.79 KB, patch)
2011-10-06 10:53 PDT, Anna Cavender 		no flags
DetailsFormatted DiffDiff
Patch (2.79 KB, patch)
2011-10-06 11:53 PDT, Anna Cavender 		no flags
DetailsFormatted DiffDiff
Patch (1.13 KB, patch)
2011-10-06 23:20 PDT, Anna Cavender 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Anna Cavender
Comment 1 2011-10-05 14:45:01 PDT
Created attachment 109866 [details] Patch
WebKit Review Bot
Comment 2 2011-10-05 15:22:16 PDT
Comment on attachment 109866 [details] Patch Attachment 109866 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9968043
Anna Cavender
Comment 3 2011-10-05 15:38:56 PDT
Created attachment 109875 [details] fix for compiler () preferences
David Levin
Comment 4 2011-10-05 17:08:18 PDT
Comment on attachment 109875 [details] fix for compiler () preferences View in context: https://bugs.webkit.org/attachment.cgi?id=109875&action=review > Source/WebCore/html/track/WebVTTParser.cpp:343 > + || (m_token.name().size() == 1 && m_token.name()[0] == 'v')) Since the "then" has multiple lines, it should have braces around it.
Anna Cavender
Comment 5 2011-10-06 10:53:45 PDT
Created attachment 109979 [details] Patch for landing
WebKit Review Bot
Comment 6 2011-10-06 11:40:29 PDT
Comment on attachment 109979 [details] Patch for landing Rejecting attachment 109979 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 Last 500 characters of output: 877f0e7382b9b559f111f171bba375520ef5248d r96838 = a273abdc68360bbc95238d521de6d2973adc9a65 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Updating chromium port dependencies using gclient... ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/9968314
Anna Cavender
Comment 7 2011-10-06 11:53:23 PDT
Created attachment 109989 [details] Patch
Anna Cavender
Comment 8 2011-10-06 11:55:48 PDT
Apparently 'webkit-patch land' doesn't work well with '--git-commit' because it changes the reviewer in the logs locally but not for the uploaded patch. Once the r+ was gone, there was nothing else I could do to land by hand. Requesting one more r+. Thanks!!
WebKit Review Bot
Comment 9 2011-10-06 14:43:38 PDT
Comment on attachment 109989 [details] Patch Clearing flags on attachment: 109989 Committed r96861: <http://trac.webkit.org/changeset/96861>
WebKit Review Bot
Comment 10 2011-10-06 14:43:43 PDT
All reviewed patches have been landed. Closing bug.
James Robinson
Comment 11 2011-10-06 18:21:09 PDT
Why would you title a patch that enables a new feature "Adding parens in WebVTTParser.cpp to appease compiler preferences."? That's extremely confusing to people looking at the commit log to try to figure out why they can't compile.
James Robinson
Comment 12 2011-10-06 18:23:05 PDT
I see - the WebKit/chromium/ChangeLog entry had the feature bit on top, but in the commit message it was buried under the WebCore/ChangeLog entry. A bit unfortunate - maybe it would have been better to land the compile fix on its own, then the feature flip?
Anna Cavender
Comment 13 2011-10-06 18:26:08 PDT
Sorry about that. I figured that's all that was happening under that ChangeLog, but you are right. I'll be sure to change it once I figure out why the Windows release didn't like it.
Anna Cavender
Comment 14 2011-10-06 22:46:12 PDT
We'll try this again after fixing some errors found.
Anna Cavender
Comment 15 2011-10-06 23:20:34 PDT
Created attachment 110094 [details] Patch
WebKit Review Bot
Comment 16 2011-10-06 23:43:04 PDT
Comment on attachment 110094 [details] Patch Attachment 110094 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10005126
WebKit Review Bot
Comment 17 2011-10-07 02:52:50 PDT
Comment on attachment 110094 [details] Patch Clearing flags on attachment: 110094 Committed r96923: <http://trac.webkit.org/changeset/96923>
WebKit Review Bot
Comment 18 2011-10-07 02:52:56 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.