Bug 102763

Summary: [V8] Introduce constructorCallbackCustom()
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: WebCore JavaScriptAssignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 102770, 102806    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
patch for landing webkit.review.bot: commit-queue-

Description Kentaro Hara 2012-11-19 20:52:54 PST
Currently custom constructors have the following code:

  v8::Handle<v8::Value> V8XXX::constructorCallback(const v8::Arguments& args) {
    INC_STATS("DOM.XXX.Constructor");
    if (!args.IsConstructCall())
      return throwTypeError("DOM object constructor cannot be called as a function.", args.GetIsolate());
    if (ConstructorMode::current() == ConstructorMode::WrapExistingObject)
      return args.Holder();

    /* body of the constructor */;
  }

I would like to generate the common part in a code generator so that custom constructors can just write the body part. Specifically, I want to make the following change:

  // Generated automatically
  v8::Handle<v8::Value> V8XXX::constructorCallback(const v8::Arguments& args) {
    INC_STATS("DOM.XXX.Constructor");
    if (!args.IsConstructCall())
      return throwTypeError("DOM object constructor cannot be called as a function.", args.GetIsolate());
    if (ConstructorMode::current() == ConstructorMode::WrapExistingObject)
      return args.Holder();

    V8XXX::constructorCallbackCustom(args);
  }

  // Written manually
  v8::Handle<v8::Value> V8XXX::constructorCallback(const v8::Arguments& args) {
    /* body of the constructor */;
  }

By doing this, not only can we avoid duplicating the same logic in custom bindings, but also it enables us to insert the logic for [V8MeasureAs] into custom constructors.
Comment 1 Adam Barth 2012-11-19 23:39:36 PST
Sounds good to me.
Comment 2 Kentaro Hara 2012-11-20 16:48:22 PST
Created attachment 175301 [details]
Patch
Comment 3 Adam Barth 2012-11-20 16:52:06 PST
Comment on attachment 175301 [details]
Patch

Very nice.
Comment 4 Kentaro Hara 2012-11-20 16:54:07 PST
Created attachment 175306 [details]
patch for landing
Comment 5 WebKit Review Bot 2012-11-20 20:03:50 PST
Comment on attachment 175306 [details]
patch for landing

Rejecting attachment 175306 [details] from commit-queue.

New failing tests:
fast/dom/call-a-constructor-as-a-function.html
Full output: http://queues.webkit.org/results/14933559
Comment 6 Kentaro Hara 2012-11-20 21:03:39 PST
Committed r135346: <http://trac.webkit.org/changeset/135346>
Comment 7 Kentaro Hara 2012-11-20 21:06:33 PST
(In reply to comment #5)
> New failing tests:
> fast/dom/call-a-constructor-as-a-function.html
> Full output: http://queues.webkit.org/results/14933559

Updated the test result and landed the patch. It looks like that '.' was missing in an error message of DataView's custom constructors:) Now we have '.' for all custom constructors because the same error message is generated by the code generator.