Bug 64973 - Add [Optional=CallWithDefaultValue] flag and use it in initXXXEvent declarations
Summary: Add [Optional=CallWithDefaultValue] flag and use it in initXXXEvent declarations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-21 13:07 PDT by Mark Pilgrim (Google)
Modified: 2011-07-22 16:27 PDT (History)
3 users (show)

See Also:


Attachments
Patch (48.10 KB, patch)
2011-07-21 13:10 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (49.05 KB, patch)
2011-07-22 12:55 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2011-07-21 13:07:53 PDT
As discussed in IRC, we are doing away with the interface-level LegacyDefaultOptionalArguments flag. To handle some cases of the old behavior, we need to define a new (argument-level) flag so that functions can define optional arguments but not have to declare every single combination of arguments in C++. The old code generator handled this by calling a single C++ function with all arguments, but with default values for the "optional" arguments. Thus, this patch invents a new argument-level flag called [Optional=CallWithDefaultValue] that emulates the old behavior.

This patch also fixes all the initXXXEvent declarations to remove the LegacyDefaultOptionalArguments flag and use the new [Optional=CallWithDefaultValue] flag on arguments that require it for backward compatibility.
Comment 1 Mark Pilgrim (Google) 2011-07-21 13:10:54 PDT
Created attachment 101639 [details]
Patch
Comment 2 Adam Barth 2011-07-21 14:24:23 PDT
Comment on attachment 101639 [details]
Patch

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

> Source/WebCore/dom/MessageEvent.idl:40
> -        [Custom] void initMessageEvent(in DOMString typeArg, in boolean canBubbleArg, in boolean cancelableArg, in SerializedScriptValue dataArg, in DOMString originArg, in DOMString lastEventIdArg, in DOMWindow sourceArg, in Array messagePorts);
> +        [Custom] void initMessageEvent(in [Optional=CallWithDefaultValue] DOMString typeArg, in [Optional=CallWithDefaultValue] boolean canBubbleArg, in [Optional=CallWithDefaultValue] boolean cancelableArg, in [Optional=CallWithDefaultValue] SerializedScriptValue dataArg, in [Optional=CallWithDefaultValue] DOMString originArg, in [Optional=CallWithDefaultValue] DOMString lastEventIdArg, in [Optional=CallWithDefaultValue] DOMWindow sourceArg, in [Optional=CallWithDefaultValue] Array messagePorts);

I'd reformat this onto multiple lines.

> Source/WebCore/dom/MessageEvent.idl:45
> -        void initMessageEvent(in DOMString typeArg, in boolean canBubbleArg, in boolean cancelableArg, in SerializedScriptValue dataArg, in DOMString originArg, in DOMString lastEventIdArg, in DOMWindow sourceArg, in MessagePort messagePort);
> +        void initMessageEvent(in [Optional=CallWithDefaultValue] DOMString typeArg, in [Optional=CallWithDefaultValue] boolean canBubbleArg, in [Optional=CallWithDefaultValue] boolean cancelableArg, in [Optional=CallWithDefaultValue] SerializedScriptValue dataArg, in [Optional=CallWithDefaultValue] DOMString originArg, in [Optional=CallWithDefaultValue] DOMString lastEventIdArg, in [Optional=CallWithDefaultValue] DOMWindow sourceArg, in [Optional=CallWithDefaultValue] MessagePort messagePort);

This too.
Comment 3 Mark Pilgrim (Google) 2011-07-21 14:25:59 PDT
Also just realized that I'm using the != operator instead of "ne" to compare strings in Perl (CodeGenerator*.pm). Leaving this comment as a reminder to myself since I might not be able to re-patch this until Monday.
Comment 4 WebKit Review Bot 2011-07-21 17:32:18 PDT
Comment on attachment 101639 [details]
Patch

Attachment 101639 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9200874

New failing tests:
fast/events/document-elementFromPoint.html
fast/events/offsetX-offsetY.html
fast/events/init-events.html
fast/dom/Window/window-xy-properties.html
Comment 5 Mark Pilgrim (Google) 2011-07-22 12:55:58 PDT
Created attachment 101756 [details]
Patch
Comment 6 Mark Pilgrim (Google) 2011-07-22 12:57:06 PDT
Fixed !=/ne confusion. Also fixed failing tests -- there was one more place in the V8 code generator (only) that needed to distinguish between [Optional] and [Optional=CallWithDefaultValue], when checking whether it needed to build a custom signature.
Comment 7 WebKit Review Bot 2011-07-22 16:27:08 PDT
Comment on attachment 101756 [details]
Patch

Clearing flags on attachment: 101756

Committed r91617: <http://trac.webkit.org/changeset/91617>
Comment 8 WebKit Review Bot 2011-07-22 16:27:13 PDT
All reviewed patches have been landed.  Closing bug.