Digging further through the errors, it seems that the generated code for the
base class itself is littered with references to the V8ClassIndex::<type>
value. For example:
static v8::Handle<v8::Value> locationAttrGetter(v8::Local<v8::String>
name, const v8::AccessorInfo& info) {
INC_STATS("DOM.WorkerContext.location._get");
v8::Handle<v8::Object> holder = info.Holder();
WorkerContext* imp = V8DOMWrapper::convertToNativeObject<WorkerContext>(
*V8ClassIndex::WORKERCONTEXT*, holder);
I'd note that V8DOMWrapper::convertToNativeObject() is basically just a
wrapper around convertDOMWrapperToNative() which itself just pulls a pointer
value out of kDOMWrapperObjectIndex (the passed-in type is essentially
ignored except for some runtime debugging) so we could probably just
generate calls to convertDOMWrapperToNative() instead.
-atw
On Wed, Jul 22, 2009 at 12:59 PM, Drew Wilson <[email protected]> wrote:
> I think this may not be true, as I got a compilation error in the generated
> bindings when I removed the type enum for the base class.
> Note that the code generated for my derived class contains an explicit
> reference to the base class:
> static v8::Persistent<v8::FunctionTemplate>
> ConfigureV8DedicatedWorkerContextTemplate(v8::Persistent<v8::FunctionTemplate>
> desc) {
>
> ...initialization of attributes...
>
> desc->Inherit(V8DOMWrapper::getTemplate(*V8ClassIndex::WORKERCONTEXT*));
> desc->SetClassName(v8::String::New("DedicatedWorkerContext"));
> return desc;
> }
>
> So it appears that we *do* need to define the base type in this case, and
> in general we need to be able to generate a template for every item in the
> class hierarchy, even if that class should not be directly instantiable.
>
> Is this correct, or am I missing something?
>
> -atw
>
> On Tue, Jul 21, 2009 at 10:42 PM, Mads Sig Ager <[email protected]> wrote:
>
>> If you don't need the base 'type' in the binding layer code, you don't
>> have to specify it in the V8Index file. Prototype chains and
>> instanceof operations are all handled by V8 based on the code
>> generated from the IDL files and it is independent of the 'type'
>> declarations in the V8Index file.
>>
>> Cheers, -- Mads
>>
>> On Tue, Jul 21, 2009 at 6:13 PM, Drew Wilson<[email protected]>
>> wrote:
>> > The other unanswered question is whether it's useful to define the base
>> > "type" in V8Index.h.
>> > If a wrapper of the base type (WRAPPERCONTEXT) is never instantiated, do
>> I
>> > still need to define it for the purposes of things like "instanceof" and
>> > prototype chains? Or is it *only* used to specify the type of wrapper
>> > instances?
>> > -atw
>> >
>> > On Tue, Jul 21, 2009 at 10:58 AM, Adam Barth <[email protected]>
>> wrote:
>> >>
>> >> I think the way this works in general is that you create the wrapper
>> >> for the derived class. You can see all the switch statements in
>> >> V8DOMWrapper.cpp that try to do this for Nodes, etc.
>> >>
>> >> Adam
>> >>
>> >>
>> >> On Tue, Jul 21, 2009 at 10:32 AM, Jeremy Orlow<[email protected]>
>> wrote:
>> >> > On Tue, Jul 21, 2009 at 10:19 AM, Drew Wilson <[email protected]>
>> >> > wrote:
>> >> >>
>> >> >> It seems like that would have some undesirable side-effects, aside
>> from
>> >> >> the fact that WebKit frowns on using virtual functions
>> unnecessarily.
>> >> >> So, let's imagine that I have two derived classes,
>> SharedWorkerContext
>> >> >> and
>> >> >> DedicatedWorkerContext. DedicatedWorkerContext wants to expose
>> >> >> postMessage()
>> >> >> as a public callable function, but SharedWorkerContext would not.
>> >> >> If we only have a single V8 "class" for both of these
>> (WORKERCONTEXT)
>> >> >> then
>> >> >> that implies:
>> >> >> 1) I have to define postMessage() as a virtual function on the base
>> >> >> WebCore class (WorkerContext). In fact, WorkerContext ends up
>> >> >> containing the
>> >> >> union of all exposed APIs for every future derived class, which
>> seems
>> >> >> ugly.
>> >> >> 2) From javascript, if I have a SharedWorkerContext, and I do this
>> >> >> "typeof
>> >> >> postMessage", it should return "undefined" (since
>> SharedWorkerContext
>> >> >> does
>> >> >> not define this attribute) - however, since SharedWorkerContext is
>> >> >> actually
>> >> >> just a vanilla WORKERCONTEXT behind the scenes, it would return
>> >> >> "function",
>> >> >> which violates the spec.
>> >> >> It seems like the right way to do this is to actually have separate
>> V8
>> >> >> items. The alternative is to have just a single WORKERCONTEXT, but
>> >> >> instead
>> >> >> of using polymorphism have custom getters/setters for every
>> attribute
>> >> >> that
>> >> >> check the type of the impl class and do the appropriate thing. But
>> it
>> >> >> seems
>> >> >> like the whole point of having the V8ClassIndex enum is to avoid
>> this
>> >> >> kind
>> >> >> of manual polymorphism.
>> >> >
>> >> > Good points.
>> >> > Are there other use cases for building polymorphism into V8? Is
>> there
>> >> > any
>> >> > notion of polymorphism in the IDL files? Maybe the best answer is to
>> >> > just
>> >> > make them two completely different classes. It kind of seems like
>> doing
>> >> > this elegantly is going to cost us performance wise one way or
>> another.
>> >> > J
>> >> >
>> >> >> On Mon, Jul 20, 2009 at 8:22 PM, Jeremy Orlow <[email protected]>
>> >> >> wrote:
>> >> >>>
>> >> >>> In other words, make all workers appear the same to V8 (i.e. as a
>> >> >>> WORKERCONTEXT) and then implement polymorphism in the
>> implementations
>> >> >>> being
>> >> >>> wrapped by V8.
>> >> >>> On Mon, Jul 20, 2009 at 8:19 PM, Jeremy Orlow <[email protected]
>> >
>> >> >>> wrote:
>> >> >>>>
>> >> >>>> Sorry if this is a dumb question, but why woudn't you simply have
>> a
>> >> >>>> WORKERCONTEXT and let virtual dispatch do its job for the rest?
>> >> >>>> Shared
>> >> >>>> methods can be implemented on the base class and the rest can be
>> >> >>>> purely
>> >> >>>> virtual with implementations in the sub classes.
>> >> >>>> J
>> >> >>>>
>> >> >>>> On Mon, Jul 20, 2009 at 3:21 PM, Drew Wilson <
>> [email protected]>
>> >> >>>> wrote:
>> >> >>>>>
>> >> >>>>> Following up on this. Let's imagine that I don't define a
>> >> >>>>> V8ClassIndex
>> >> >>>>> enum for the common base class (so, in my case, I get rid of
>> >> >>>>> WORKERCONTEXT,
>> >> >>>>> and have only DEDICATEDWORKERCONTEXT and SHAREDWORKERCONTEXT (the
>> >> >>>>> derived
>> >> >>>>> classes).
>> >> >>>>> Now, let's say I'm defining an ACCESSOR_GETTER on the base class
>> -
>> >> >>>>> the
>> >> >>>>> first thing I want to do is get a pointer to the native object. I
>> >> >>>>> don't know
>> >> >>>>> which concrete class the native object is (DedicatedWorkerContext
>> or
>> >> >>>>> SharedWorkerContext), but I do know that it's guaranteed to be an
>> >> >>>>> instance
>> >> >>>>> of the base native class (WorkerContext).
>> >> >>>>> Currently I'm doing this:
>> >> >>>>> WorkerContext* workerContext =
>> >> >>>>>
>> >> >>>>>
>> V8DOMWrapper::convertToNativeObject<WorkerContext>(V8ClassIndex::WORKERCONTEXT,
>> >> >>>>> info.Holder());
>> >> >>>>> If I remove V8ClassIndex::WORKERCONTEXT, then I can't pass that
>> in
>> >> >>>>> to
>> >> >>>>> convertToNativeObject. It looks like the passed-in enum is only
>> used
>> >> >>>>> for
>> >> >>>>> error checking currently, so I guess what I should use instead is
>> >> >>>>> convertDOMWrapperToNative()?:
>> >> >>>>> WorkerContext* workerContext =
>> >> >>>>>
>> >> >>>>>
>> V8DOMWrapper::convertDOMWrapperToNative<WorkerContext>(info.Holder());
>> >> >>>>> Is that the general pattern people use for cases like this?
>> >> >>>>> -atw
>> >> >>>>> On Mon, Jul 20, 2009 at 3:21 PM, Drew Wilson <
>> [email protected]>
>> >> >>>>> wrote:
>> >> >>>>>>
>> >> >>>>>> <resending from correct acct>:
>> >> >>>>>> Currently, Web Workers have a class (WorkerContext) which
>> >> >>>>>> represents
>> >> >>>>>> the global scope for a worker. The custom V8 bindings for this
>> >> >>>>>> class are
>> >> >>>>>> defined in V8WorkerContextCustom, and we also define
>> >> >>>>>> V8ClassIndex::WORKERCONTEXT for the wrapper object.
>> >> >>>>>> I'm refactoring the WebCore impl class, so WorkerContext becomes
>> >> >>>>>> (essentially) an abstract base class, and the actual worker
>> context
>> >> >>>>>> will be
>> >> >>>>>> one of two derived classes: DedicatedWorkerContext or
>> >> >>>>>> SharedWorkerContext.
>> >> >>>>>> In my refactoring, I've only implemented a single derived class
>> >> >>>>>> (DedicatedWorkerContext) for now.
>> >> >>>>>> I'm trying to figure out the correct way to structure the V8
>> >> >>>>>> bindings
>> >> >>>>>> - I've already made a pass at it that passes all of the unit
>> tests:
>> >> >>>>>> https://bugs.webkit.org/show_bug.cgi?id=27420
>> >> >>>>>> I've defined V8ClassIndex::DEDICATEDWORKERCONTEXT, but I'm not
>> >> >>>>>> certain
>> >> >>>>>> if I need to remove V8ClassIndex::WORKERCONTEXT or not, since
>> there
>> >> >>>>>> shouldn't ever be a wrapper object created for that base class.
>> Are
>> >> >>>>>> the
>> >> >>>>>> wrapper types defined in V8Index.h only for actually
>> instantiable
>> >> >>>>>> wrapper
>> >> >>>>>> objects (in which case I should only define them for the
>> "leaves"
>> >> >>>>>> of the
>> >> >>>>>> tree), or is it used for other things like instanceof?
>> >> >>>>>> -atw
>> >> >>>>>
>> >> >>>>>
>> >> >>>>>
>> >> >>>>
>> >> >>>
>> >> >>
>> >> >
>> >> >
>> >> > >> >> >
>> >> >
>> >
>> >
>>
>
>
--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: [email protected]
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---
