[webkit-reviews] review granted: [Bug 26495] Upstream V8Index : [Attachment 31451] patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Nate Chapin
<japhet at google.com>'s request for review:
Bug 26495: Upstream V8Index
https://bugs.webkit.org/show_bug.cgi?id=26495

Attachment 31451: patch
https://bugs.webkit.org/attachment.cgi?id=31451&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
This whole file is total sadness. But I'd rather land it so that we have them
all in one place.

> +// FIXME: Can we use a macro to include necessary headers by using
WRAPPER_TYPES?

Nah.

> +    class V8ClassIndex {
> +    public:
> +	   // Type must start at non-negative numbers. See ToInt, FromInt.
> +	   enum V8WrapperType {
> +	       INVALID_CLASS_INDEX = 0,
> +
> +#define DEFINE_ENUM(name, type) name, 
> +	       ALL_WRAPPER_TYPES(DEFINE_ENUM)
> +#undef DEFINE_ENUM
> +
> +	       CLASSINDEX_END,
> +	       WRAPPER_TYPE_COUNT = CLASSINDEX_END
> +	   };
> +
> +	   static int ToInt(V8WrapperType type) { return
static_cast<int>(type); }

Add a FIXME to convert to toInt. I realize it will be near-impossible to
refactor it before all of the bindings land in one place.

> +
> +	   static V8WrapperType FromInt(int v) {

Ditto.

> +	       ASSERT(INVALID_CLASS_INDEX <= v && v < CLASSINDEX_END);
> +	       return static_cast<V8WrapperType>(v);
> +	   }
> +
> +	   static FunctionTemplateFactory GetFactory(V8WrapperType type);

Ditto.

> +	   // Returns a field to be used as cache for the template for the
given type
> +	   static v8::Persistent<v8::FunctionTemplate>* GetCache(V8WrapperType
type);

Ditto.