Ah, yes of course. The generated code for the base class itself needs it. I had also forgotten that we use the ID to get the superclass template to inherit from.
Don't listen to me. :-) -- Mads On Wed, Jul 22, 2009 at 10:15 PM, Drew Wilson<[email protected]> wrote: > 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 -~----------~----~----~----~------~----~------~--~---
