Bug 74599

Summary: Enable the [Supplemental] IDL on AppleWebKit and AppleWin
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: WebCore Misc.Assignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, gns, gustavo.noronha, japhet, rakuco, sam, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 74481, 74837, 74841, 74900    
Bug Blocks: 72138    
Attachments:
Description Flags
WIP patch
none
WIP patch: patch conflict resolved
none
WIP patch to see if build succeeds
none
WIP patch to see if build succeeds
none
WIP patch to see if build succeeds
none
WIP patch to see if build succeeds none

Description Kentaro Hara 2011-12-15 03:16:38 PST
We have enabled the [Supplemental] IDL on Chromium and are planning to enable it on all platforms (Meta bug 72138). In this bug, we enable it on AppleWebKit.
Comment 1 Kentaro Hara 2011-12-17 10:26:28 PST
Created attachment 119730 [details]
WIP patch
Comment 2 Kentaro Hara 2011-12-17 10:26:56 PST
I uploaded a WIP patch. I am planning to land it in the following steps:

[1] Fix CodeGeneratorJS.pm for custom getters and setters with the [Supplemental] IDL.
[2] Move settingsForWindow(), webkitAudioContext() and webSocket() in JSDOMWindowCustom.cpp into JSDOMWindowWebAudioCustom.cpp and JSDOMWindowWebSocketCustom.cpp, for modularization.
[3] Make a change on Derivedsources.make.
Comment 3 Kentaro Hara 2011-12-17 10:33:34 PST
Comment on attachment 119730 [details]
WIP patch

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

> Source/WebCore/DerivedSources.make:923
> +	$(COMMAND_FOR_GENERATING_ALL_IDLS_TEMPFILE)

Unfortunately, this line takes 13 seconds in my Mac environment (although it is executed only when any of IDL files is updated).

As commented above, the line is equivalent to 'echo $(IDL_FILES_LIST) > $(ALL_IDLS_TEMPFILE)', but the line realizes it by executing "echo idl_file >> $(ALL_IDLS_TEMPFILE)" for *each* IDL file. This is because 'echo $(IDL_FILES_LIST) > $(ALL_IDLS_TEMPFILE)' exceeds the OS limit of command-line argument count, and 'echo "$(IDL_FILES_LIST)" > $(ALL_IDLS_TEMPFILE)' exceeds the OS limit of command-line argument bytes...

Any better idea?
Comment 4 Adam Barth 2011-12-17 10:36:39 PST
Comment on attachment 119730 [details]
WIP patch

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

>> Source/WebCore/DerivedSources.make:923
>> +	$(COMMAND_FOR_GENERATING_ALL_IDLS_TEMPFILE)
> 
> Unfortunately, this line takes 13 seconds in my Mac environment (although it is executed only when any of IDL files is updated).
> 
> As commented above, the line is equivalent to 'echo $(IDL_FILES_LIST) > $(ALL_IDLS_TEMPFILE)', but the line realizes it by executing "echo idl_file >> $(ALL_IDLS_TEMPFILE)" for *each* IDL file. This is because 'echo $(IDL_FILES_LIST) > $(ALL_IDLS_TEMPFILE)' exceeds the OS limit of command-line argument count, and 'echo "$(IDL_FILES_LIST)" > $(ALL_IDLS_TEMPFILE)' exceeds the OS limit of command-line argument bytes...
> 
> Any better idea?

Is there a way in Make to break up IDL_FILES_LIST into chunks?
Comment 5 Kentaro Hara 2011-12-17 10:38:34 PST
(In reply to comment #4)
> (From update of attachment 119730 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=119730&action=review
> 
> >> Source/WebCore/DerivedSources.make:923
> >> +	$(COMMAND_FOR_GENERATING_ALL_IDLS_TEMPFILE)
> > 
> > Unfortunately, this line takes 13 seconds in my Mac environment (although it is executed only when any of IDL files is updated).
> > 
> > As commented above, the line is equivalent to 'echo $(IDL_FILES_LIST) > $(ALL_IDLS_TEMPFILE)', but the line realizes it by executing "echo idl_file >> $(ALL_IDLS_TEMPFILE)" for *each* IDL file. This is because 'echo $(IDL_FILES_LIST) > $(ALL_IDLS_TEMPFILE)' exceeds the OS limit of command-line argument count, and 'echo "$(IDL_FILES_LIST)" > $(ALL_IDLS_TEMPFILE)' exceeds the OS limit of command-line argument bytes...
> > 
> > Any better idea?
> 
> Is there a way in Make to break up IDL_FILES_LIST into chunks?

It is possible. I'll do it in the upcoming patch then.
Comment 6 Adam Barth 2011-12-17 10:38:45 PST
Should we store the list of IDL files in a file outside of DerivedSources.make?  Is there a way for the makefile to read them in?
Comment 7 Kentaro Hara 2011-12-17 10:46:26 PST
(In reply to comment #6)
> Should we store the list of IDL files in a file outside of DerivedSources.make?  Is there a way for the makefile to read them in?

It is also possible. Rather than splitting $(IDL_FILES_LIST) into chunks, preparing the file might be better.

### By the way, we will need to prepare the file that lists all IDL files for *each* platform.
Comment 8 Kentaro Hara 2011-12-17 10:49:27 PST
Created attachment 119731 [details]
WIP patch: patch conflict resolved
Comment 9 Early Warning System Bot 2011-12-17 11:00:17 PST
Comment on attachment 119731 [details]
WIP patch: patch conflict resolved

Attachment 119731 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10921175
Comment 10 Gustavo Noronha (kov) 2011-12-17 11:01:10 PST
Comment on attachment 119731 [details]
WIP patch: patch conflict resolved

Attachment 119731 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10935079
Comment 11 Gyuyoung Kim 2011-12-17 11:05:51 PST
Comment on attachment 119731 [details]
WIP patch: patch conflict resolved

Attachment 119731 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10932113
Comment 12 Adam Barth 2011-12-17 11:07:11 PST
> ### By the way, we will need to prepare the file that lists all IDL files for *each* platform.

Do you mean each build system?  Generally, the list of IDL files for every platform are the same.  Ideally we'd have one list that was used by all the build systems, but having a separate list for each build system is ok because that's what we have today.
Comment 13 Kentaro Hara 2011-12-17 17:21:52 PST
(In reply to comment #12)
> > ### By the way, we will need to prepare the file that lists all IDL files for *each* platform.
> 
> Do you mean each build system? 

Ah, yes, I meant it.

> Ideally we'd have one list that was used by all the build systems, but having a separate list for each build system is ok because that's what we have today.

I got it. Ideally we would have one file that lists all IDL files used by all the build systems, and an array that lists "exceptional" IDL files in each build script. Then in most cases we just need to touch the one file when we want to add a new IDL file. I'd like to do this refactoring after we enable the [Supplemental] IDL on all the build systems. Thanks.
Comment 14 Kentaro Hara 2011-12-18 04:38:48 PST
Created attachment 119758 [details]
WIP patch to see if build succeeds
Comment 15 Gyuyoung Kim 2011-12-18 04:52:15 PST
Comment on attachment 119758 [details]
WIP patch to see if build succeeds

Attachment 119758 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10935238
Comment 16 Kentaro Hara 2011-12-18 08:02:50 PST
Created attachment 119762 [details]
WIP patch to see if build succeeds
Comment 17 Gyuyoung Kim 2011-12-18 08:17:47 PST
Comment on attachment 119762 [details]
WIP patch to see if build succeeds

Attachment 119762 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10934273
Comment 18 Collabora GTK+ EWS bot 2011-12-18 08:33:10 PST
Comment on attachment 119762 [details]
WIP patch to see if build succeeds

Attachment 119762 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10929304
Comment 19 Kentaro Hara 2011-12-18 17:09:40 PST
Created attachment 119789 [details]
WIP patch to see if build succeeds
Comment 20 Kentaro Hara 2011-12-18 18:02:36 PST
Created attachment 119791 [details]
WIP patch to see if build succeeds