Bug 70464 - Add a ConstructorMode check to all existing custom V8 constructors
Summary: Add a ConstructorMode check to all existing custom V8 constructors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-19 18:18 PDT by Kentaro Hara
Modified: 2011-10-19 20:52 PDT (History)
4 users (show)

See Also:


Attachments
Patch (13.98 KB, patch)
2011-10-19 18:56 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2011-10-19 18:18:27 PDT
We should add the following two lines to all existing custom V8 constructors:

    if (ConstructorMode::current() == ConstructorMode::WrapExistingObject)
        return args.Holder();

This checks whether a programmer is trying to allocate an object via "new X", or C++ is trying to allocate an object via the function template and wrap the object with a JS flavor. In the latter case, a constructor callback should not be executed (i.e. should return immediately).

As for existing custom V8 constructors, no bugs have been occurring without this check for now. However, without the check, the problems that I described in the bug 70015 can happen in the future if someone changes code. In addition, if the check does not exist in existing custom constructors, people will add a new custom constructor without the check without considering the possibility of the problems, which may result in ugly bugs.
Comment 1 Kentaro Hara 2011-10-19 18:56:58 PDT
Created attachment 111708 [details]
Patch
Comment 2 Adam Barth 2011-10-19 19:00:49 PDT
Comment on attachment 111708 [details]
Patch

I suspect a bunch of these cases can't actually occur, but I support adding this check everywhere as a model.
Comment 3 WebKit Review Bot 2011-10-19 20:52:11 PDT
Comment on attachment 111708 [details]
Patch

Clearing flags on attachment: 111708

Committed r97929: <http://trac.webkit.org/changeset/97929>
Comment 4 WebKit Review Bot 2011-10-19 20:52:16 PDT
All reviewed patches have been landed.  Closing bug.