Bug 32844

Summary: [V8] Generate header declaration for property accessor getters/setters
Product: WebKit Reporter: Nate Chapin <japhet>
Component: WebCore Misc.Assignee: Nate Chapin <japhet>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 32638    
Attachments:
Description Flags
Generate property accessor getter/setter declarations.
dglazkov: review+
patch2 - removed .idl modifications dglazkov: review+, japhet: commit-queue-

Description Nate Chapin 2009-12-21 16:20:57 PST
Part of https://bugs.webkit.org/show_bug.cgi?id=32638, the quest to remove all the manually declared custom bindings from V8CustomBinding.h.
Comment 1 Nate Chapin 2009-12-21 16:29:08 PST
Created attachment 45359 [details]
Generate property accessor getter/setter declarations.

I'm on my way out the door for the holidays, so this review is decidedly non-urgent.
Comment 2 WebKit Review Bot 2009-12-21 16:33:56 PST
style-queue ran check-webkit-style on attachment 45359 [details] without any errors.
Comment 3 Dimitri Glazkov (Google) 2009-12-29 09:08:29 PST
Comment on attachment 45359 [details]
Generate property accessor getter/setter declarations.

r=me with notes:

Gah! Why is this custom for V8 bindings? Need a FIXME and a bug for this.

> +        readonly attribute [V8CustomGetter] DOMString appVersion;

Ditto.

> +                 attribute [Replaceable, V8CustomGetter] WorkerContext self;

This special-casing bonanza needs FIXMEs and bugs. We should work to remove these.

> +    if ($dataNode->name eq "Event") {
> +        push(@headerContent, "  static v8::Handle<v8::Value> dataTransferAccessorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info);\n");
> +        push(@headerContent, "  static void valueAccessorSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info);\n");
> +    }
> +    if ($dataNode->name eq "Location") {
> +        push(@headerContent, "  static v8::Handle<v8::Value> assignAccessorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info);\n");
> +        push(@headerContent, "  static v8::Handle<v8::Value> reloadAccessorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info);\n");
> +        push(@headerContent, "  static v8::Handle<v8::Value> replaceAccessorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info);\n");
> +    }
>  }
Comment 4 Nate Chapin 2009-12-30 13:36:31 PST
Created attachment 45683 [details]
patch2 - removed .idl modifications

The .idl modifications were originally necessary because of some v8 only custom bindings that didn't really need to be custom.  Those bindings are now generated, which resulted in the removal of V8NavigatorCustom.cpp.
Comment 5 WebKit Review Bot 2009-12-30 13:36:56 PST
style-queue ran check-webkit-style on attachment 45683 [details] without any errors.
Comment 6 Dimitri Glazkov (Google) 2009-12-30 13:39:58 PST
Comment on attachment 45683 [details]
patch2 - removed .idl modifications

ROCK!
Comment 7 Nate Chapin 2009-12-30 14:14:29 PST
http://trac.webkit.org/changeset/52672