Bug 66893

Summary: [WebAudio] Undeclared dependency to VIDEO
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: Web AudioAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: crogers, dglazkov, donggwan.kim, eric, kbr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 67468    
Bug Blocks:    
Attachments:
Description Flags
proposed patch
webkit.review.bot: commit-queue-
proposed patch
webkit.review.bot: commit-queue-
proposed patch
webkit.review.bot: commit-queue-
proposed patch
webkit.review.bot: commit-queue-
proposed patch
webkit.review.bot: commit-queue-
updated patch none

Philippe Normand
Reported Wednesday, August 24, 2011 10:40:28 PM UTC
The createMediaElementSource() of AudioContext introduces a dependency on the VIDEO feature but it's not explicit, so this breaks: build-webkit --no-video --web-audio I tried to use [Conditional=VIDEO] as a prefix to the new method in the idl file but it seems this works only for attributes.
Attachments
proposed patch (7.15 KB, patch)
2011-08-25 00:18 PDT, Philippe Normand
webkit.review.bot: commit-queue-
proposed patch (8.94 KB, patch)
2011-08-25 04:38 PDT, Philippe Normand
webkit.review.bot: commit-queue-
proposed patch (9.00 KB, patch)
2011-08-29 01:36 PDT, Philippe Normand
webkit.review.bot: commit-queue-
proposed patch (8.98 KB, patch)
2011-08-29 07:42 PDT, Philippe Normand
webkit.review.bot: commit-queue-
proposed patch (9.10 KB, patch)
2011-08-29 07:59 PDT, Philippe Normand
webkit.review.bot: commit-queue-
updated patch (5.71 KB, patch)
2011-08-30 01:03 PDT, Philippe Normand
no flags
Philippe Normand
Comment 1 Thursday, August 25, 2011 8:18:27 AM UTC
Created attachment 105138 [details] proposed patch
WebKit Review Bot
Comment 2 Thursday, August 25, 2011 8:30:18 AM UTC
Comment on attachment 105138 [details] proposed patch Attachment 105138 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9511260
WebKit Review Bot
Comment 3 Thursday, August 25, 2011 9:21:20 AM UTC
Comment on attachment 105138 [details] proposed patch Attachment 105138 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9511284
WebKit Review Bot
Comment 4 Thursday, August 25, 2011 9:51:26 AM UTC
Comment on attachment 105138 [details] proposed patch Attachment 105138 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9510309
Philippe Normand
Comment 5 Thursday, August 25, 2011 12:38:56 PM UTC
Created attachment 105160 [details] proposed patch
Philippe Normand
Comment 6 Thursday, August 25, 2011 12:39:56 PM UTC
(In reply to comment #5) > Created an attachment (id=105160) [details] > proposed patch This one should make the Chromium EWS happy. About the mac ews, not sure, it might need a clobber build.
WebKit Review Bot
Comment 7 Thursday, August 25, 2011 1:46:06 PM UTC
Comment on attachment 105160 [details] proposed patch Attachment 105160 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9510377
Philippe Normand
Comment 8 Monday, August 29, 2011 9:36:11 AM UTC
WebKit Review Bot
Comment 9 Monday, August 29, 2011 9:41:39 AM UTC
Comment on attachment 105475 [details] proposed patch Attachment 105475 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9567016
WebKit Review Bot
Comment 10 Monday, August 29, 2011 9:44:01 AM UTC
Comment on attachment 105475 [details] proposed patch Attachment 105475 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9563114
Eric Seidel (no email)
Comment 11 Monday, August 29, 2011 1:59:59 PM UTC
Comment on attachment 105475 [details] proposed patch Attachment 105475 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9564151
Philippe Normand
Comment 12 Monday, August 29, 2011 3:42:07 PM UTC
Created attachment 105489 [details] proposed patch
WebKit Review Bot
Comment 13 Monday, August 29, 2011 3:48:48 PM UTC
Comment on attachment 105489 [details] proposed patch Attachment 105489 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9558318
WebKit Review Bot
Comment 14 Monday, August 29, 2011 3:50:01 PM UTC
Comment on attachment 105489 [details] proposed patch Attachment 105489 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9564176
Eric Seidel (no email)
Comment 15 Monday, August 29, 2011 3:50:53 PM UTC
Comment on attachment 105489 [details] proposed patch Attachment 105489 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9565154
Philippe Normand
Comment 16 Monday, August 29, 2011 3:59:33 PM UTC
Created attachment 105492 [details] proposed patch
WebKit Review Bot
Comment 17 Monday, August 29, 2011 4:11:33 PM UTC
Comment on attachment 105492 [details] proposed patch Attachment 105492 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9565160
Chris Rogers
Comment 18 Monday, August 29, 2011 10:56:32 PM UTC
Comment on attachment 105492 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=105492&action=review Hi Philippe, can you try my suggestion to avoid custom bindings? Thanks, Chris > Source/WebCore/webaudio/AudioContext.idl:62 > + [Custom] MediaElementAudioSourceNode createMediaElementSource(in HTMLMediaElement mediaElement) Can't we avoid using the Custom bindings and instead have something like this: #if defined(ENABLE_VIDEO) && ENABLE_VIDEO MediaElementAudioSourceNode createMediaElementSource(in HTMLMediaElement mediaElement) raises(DOMException); #endif
Philippe Normand
Comment 19 Tuesday, August 30, 2011 9:03:55 AM UTC
Created attachment 105591 [details] updated patch
Philippe Normand
Comment 20 Tuesday, August 30, 2011 9:04:54 AM UTC
(In reply to comment #18) > (From update of attachment 105492 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=105492&action=review > > Hi Philippe, can you try my suggestion to avoid custom bindings? > Good idea indeed! Thanks for the feedback.
WebKit Review Bot
Comment 21 Tuesday, August 30, 2011 12:47:25 PM UTC
Comment on attachment 105591 [details] updated patch Attachment 105591 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9565412
Philippe Normand
Comment 22 Thursday, September 1, 2011 4:20:05 PM UTC
Would you mind review this Kenneth? Chris did a first pass.
Kenneth Russell
Comment 23 Thursday, September 1, 2011 8:04:23 PM UTC
Comment on attachment 105591 [details] updated patch Looks fine to me.
Philippe Normand
Comment 24 Friday, September 2, 2011 8:34:42 AM UTC
Philippe Normand
Comment 25 Friday, September 2, 2011 8:58:22 AM UTC
I'll have a look at the mac build
Philippe Normand
Comment 26 Friday, September 2, 2011 10:53:45 AM UTC
Build fix is to use Conditional=WEB_AUDIO&VIDEO in MediaElementAudioSourceNode.idl instead of 2 separate Conditional lines. I'll reland the patch, I verified the fix on my mac :)
Philippe Normand
Comment 27 Friday, September 2, 2011 10:58:19 AM UTC
Note You need to log in before you can comment on or make changes to this bug.