Bug 62904

Summary: Remove LegacyDefaultOptionalArguments flag from IDL files where it would not change behavior
Product: WebKit Reporter: Mark Pilgrim (Google) <pilgrim>
Component: WebCore JavaScriptAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, fishd, ossy, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 62925    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
warning fix
none
Patch none

Description Mark Pilgrim (Google) 2011-06-17 13:01:07 PDT
After bug 62750, there are many IDL files that contain the new LegacyDefaultOptionalArguments flag that don't actually need it. Some examples:

- the IDL file contains no functions
- the IDL file contains only functions with no arguments
- the IDL file contains functions, *all* of which use the [RequiresAllArguments=raise] extended attribute

This patch simplifies such IDL files by removing the LegacyDefaultOptionalArguments flag and (if needed) removing the [RequiresAllArguments=raise] extended attribute from each function declaration. This patch does not make any required arguments optional or any optional arguments required. It changes no behavior at all.
Comment 1 Mark Pilgrim (Google) 2011-06-17 13:53:08 PDT
Created attachment 97648 [details]
Patch
Comment 2 Darin Adler 2011-06-17 17:42:31 PDT
Comment on attachment 97648 [details]
Patch

=
Comment 3 WebKit Review Bot 2011-06-17 23:31:09 PDT
Comment on attachment 97648 [details]
Patch

Clearing flags on attachment: 97648

Committed r89189: <http://trac.webkit.org/changeset/89189>
Comment 4 WebKit Review Bot 2011-06-17 23:31:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Csaba Osztrogonác 2011-06-18 01:31:08 PDT
Reopen, because it broke Qt build and was rolled out by http://trac.webkit.org/changeset/89190

cc1plus: warnings being treated as errors
../../WebCore/generated/JSDOMCoreException.cpp: In function ‘JSC::EncodedJSValue WebCore::jsDOMCoreExceptionPrototypeFunctionToString(JSC::ExecState*)’:
../../WebCore/generated/JSDOMCoreException.cpp:265: error: comparison of unsigned expression < 0 is always false
Comment 6 Csaba Osztrogonác 2011-06-18 01:39:06 PDT
It broke other bots too, not only the Qt.

Please respect contributing rules try to keep the tree green:
http://www.webkit.org/coding/contributing.html

"Your change must at least compile on all platforms."
Comment 7 Adam Barth 2011-06-18 04:34:54 PDT
Thanks Ossy.  I wonder why the qt ews bot didn't warn us. :(
Comment 8 Darin Adler 2011-06-18 13:13:35 PDT
(In reply to comment #7)
> Thanks Ossy.  I wonder why the qt ews bot didn't warn us. :(

What about the commit bot? Is this happening because the commit bot now uses V8 instead of JSC?
Comment 9 Adam Barth 2011-06-18 22:05:43 PDT
> What about the commit bot? Is this happening because the commit bot now uses V8 instead of JSC?

As the patch built fine on gtk and win, it seems more likely to be a compiler difference than a JSC/V8 difference.
Comment 10 Darin Adler 2011-06-19 11:32:57 PDT
(In reply to comment #9)
> > What about the commit bot? Is this happening because the commit bot now uses V8 instead of JSC?
> 
> As the patch built fine on gtk and win, it seems more likely to be a compiler difference than a JSC/V8 difference.

It’s a combination of both. A compiler warning in JSC-generated code.
Comment 11 Csaba Osztrogonác 2011-06-20 03:53:37 PDT
Created attachment 97776 [details]
warning fix

"if (exec->argumentCount() < 0" was incorrect, because it is always false.
With the attached fix non-V8 builds will be happy.
Comment 12 Mark Pilgrim (Google) 2011-06-20 11:55:31 PDT
Created attachment 97832 [details]
Patch
Comment 13 Mark Pilgrim (Google) 2011-06-20 11:57:48 PDT
Combined patch with warning fix. Thanks Ossy for debugging.
Comment 14 Adam Barth 2011-06-20 11:57:57 PDT
Comment on attachment 97832 [details]
Patch

Ok.  Let's give this version a try.
Comment 15 WebKit Review Bot 2011-06-20 12:22:47 PDT
Comment on attachment 97832 [details]
Patch

Clearing flags on attachment: 97832

Committed r89269: <http://trac.webkit.org/changeset/89269>
Comment 16 WebKit Review Bot 2011-06-20 12:22:53 PDT
All reviewed patches have been landed.  Closing bug.