Re: [webkit-dev] Inheritance in IDL files/JS bindings

2009-07-06 Thread Drew Wilson
BTW, I noticed that the CustomToJS IDL attribute in HTMLCollection.idl is
not implemented - instead, there's just a big hard-coded list of classes in
CodeGeneratorJS.pm::UsesManualToJSImplementation() that corresponds to that
functionality.
Any reason not to
get rid of UsesManualToJSImplementation() in favor of an explicit
CustomToJS attribute? I'm happy to put together a patach for this.

-atw



On Sat, Jul 4, 2009 at 3:02 PM, Maciej Stachowiak m...@apple.com wrote:


 On Jul 4, 2009, at 11:52 AM, Drew Wilson wrote:


 1) I don't ever actually want a raw JSAbstractWorker to be created - is
 there a way to disable the toJS() generation? It looks like maybe CustomToJS
 might've let me do this, but it still defines toJS() in the AbstractWorker.h
 file. I'd rather have this generate a compile error than have incorrect
 behavior at runtime.


 I think the right way to handle this is to make a custom toJS function that
 does a runtime check of the actual type, and makes the right kind of wrapper
 object. That's because any time you deal with an AbstractWorker pointer, you
 want toJS on it to return the canonical wrapper of the right concrete class.
 The toJS function for Node is a fairly elaborate example of this.


 2) What does GenerateNativeConverter do? I note that both Node and
 Element.idl define GenerateToJS, but none of their derived classes
 (HTMLElement.idl, HTMLOptionElement.idl) define GenerateToJS, which makes me
 think that just defining GenerateToJS on derived classes is not the correct
 fix.


 Correct - see above. toJS needs to do a dynamic check based on the runtime
 type, not a static check based on the declared type. We don't ever want to
 vend an Element wrapper for something that actually should be an
 HTMLDivElement, even if it got returned from a context that deals with
 generic elements.



 I don't know how useful the diff would be, but I uploaded one here - it's
 not ready for review yet as it has lots of odd debugging flotsam in it:
 https://bugs.webkit.org/attachment.cgi?id=32257action=review



 It sounds like you're on the right track to figuring this out. I believe 
 writing a custom toJS for AbstractWorker which checks what kind of object it 
 actually has will fix your problem.

 Regards,
 Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Inheritance in IDL files/JS bindings

2009-07-06 Thread Darin Adler

On Jul 6, 2009, at 2:52 PM, Drew Wilson wrote:

BTW, I noticed that the CustomToJS IDL attribute in  
HTMLCollection.idl is not implemented - instead, there's just a big  
hard-coded list of classes in  
CodeGeneratorJS.pm::UsesManualToJSImplementation() that corresponds  
to that functionality.


Any reason not to get rid of UsesManualToJSImplementation() in favor  
of an explicit CustomToJS attribute? I'm happy to put together a  
patch for this.


No reason I can think of. That would be fine to do.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Inheritance in IDL files/JS bindings

2009-07-06 Thread Drew Wilson
OK - I've uploaded a patch here if anyone has a chance to look at it - it's
only about 11 changed lines, so it's pretty small.
https://bugs.webkit.org/show_bug.cgi?id=27010

-atw

On Mon, Jul 6, 2009 at 3:00 PM, Darin Adler da...@apple.com wrote:

 On Jul 6, 2009, at 2:52 PM, Drew Wilson wrote:

  BTW, I noticed that the CustomToJS IDL attribute in HTMLCollection.idl is
 not implemented - instead, there's just a big hard-coded list of classes in
 CodeGeneratorJS.pm::UsesManualToJSImplementation() that corresponds to that
 functionality.

 Any reason not to get rid of UsesManualToJSImplementation() in favor of an
 explicit CustomToJS attribute? I'm happy to put together a patch for this.


 No reason I can think of. That would be fine to do.

-- Darin


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Inheritance in IDL files/JS bindings

2009-07-04 Thread Drew Wilson
Thanks for the insights.
I stepped through the worker constructor code. The problem was at the very
last line, where we create a JS wrapper object around the core
WebCore::Worker object:

return asObject(toJS(exec, worker.release()));

Before I changed Worker.idl to have Worker derive from the AbstractWorker
interface, the generator was creating a toJS() function in JSWorker. Once I
changed Worker to derive from AbstractWorker, the generator stopped putting
toJS() in JSWorker, so the code was falling through to call the base-class
code and returning an AbstractWorker object. I guess by default the
generator only creates toJS() functions for base classes?

In this case, adding GenerateToJS to Worker.idl fixed the problem by telling
the generator to create a toJS() function for JSWorker as well. Is that the
right fix?

I still have a couple of questions:

1) I don't ever actually want a raw JSAbstractWorker to be created - is
there a way to disable the toJS() generation? It looks like maybe CustomToJS
might've let me do this, but it still defines toJS() in the AbstractWorker.h
file. I'd rather have this generate a compile error than have incorrect
behavior at runtime.

2) What does GenerateNativeConverter do? I note that both Node and
Element.idl define GenerateToJS, but none of their derived classes
(HTMLElement.idl, HTMLOptionElement.idl) define GenerateToJS, which makes me
think that just defining GenerateToJS on derived classes is not the correct
fix.


I don't know how useful the diff would be, but I uploaded one here - it's
not ready for review yet as it has lots of odd debugging flotsam in it:
https://bugs.webkit.org/attachment.cgi?id=32257action=review

-atw


On Fri, Jul 3, 2009 at 3:23 PM, Sam Weinig sam.wei...@gmail.com wrote:



 On Fri, Jul 3, 2009 at 2:42 PM, Drew Wilson atwil...@google.com wrote:

 I'm working on refactoring some code out of WebCore::Worker into a common
 base class to be shared between SharedWorker and Worker.
 So I've defined a new AbstractWorker.idl and its associated JS bindings.
 AbstractWorker is not intended to be instantiable, so I don't have a
 constructor generated for it.

 interface [CustomMarkFunction, Conditional=WORKERS] AbstractWorker {
 ...
 }


 GenerateConstructor is a slight misnomer unfortunately.  Event
 non-instatiatable interfaces should have one so that they can be exposed on
 the global object for use with instanceof and accessing the prototype.



 I then changed Worker.idl to make the Worker interface derive from the
 base AbstractWorker interface, like so:

 interface [CustomMarkFunction, Conditional=WORKERS] Worker :
 AbstractWorker {
...
 }

 Everything compiles just fine, except that at runtime when I instantiate a
 Worker object from Javascript, I seem to get an instance of AbstractWorker
 instead, and referencing things that should be defined on the Worker
 prototype (like Worker::postMessage) yields undefined. I thought at first
 that perhaps I was doing something wrong with the prototype (setup in
 JSWorkerConstructor.cpp) so I wrote some JS tests, which produced the
 expected results:

 log(Worker.toString());   = [object WorkerConstructor]
 log(Worker.prototype.toString());= [object WorkerPrototype]
 log(Worker.prototype.postMessage.toString());   = function
 postMessage() { [native code] }

 And yet when I actually instantiate a Worker object:

 var worker = new Worker(resources/worker-common.js);
 log (worker.toString());= [object *AbstractWorker*]
 log (typeof worker.postMessage);= *undefined*

 Is there anything special I need to do when implementing inheritance in
 IDL files and JS bindings? I *think* I'm correctly following the example of
 other places where we use inheritance (example HTMLElement - Element -
 Node), although there are some options defined in the .idl file like
 GenerateNativeConverter and GenerateToJS that might be relevant but
 which I couldn't find any documentation for.

 Anyhow, I'm digging into this further, but I figured people might be able
 to shed some light on possible causes of this behavior.


 It is hard to say without a full diff.  Nothing you have described seems
 too far off.

 -Sam


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Inheritance in IDL files/JS bindings

2009-07-04 Thread Maciej Stachowiak


On Jul 4, 2009, at 11:52 AM, Drew Wilson wrote:



1) I don't ever actually want a raw JSAbstractWorker to be created -  
is there a way to disable the toJS() generation? It looks like maybe  
CustomToJS might've let me do this, but it still defines toJS() in  
the AbstractWorker.h file. I'd rather have this generate a compile  
error than have incorrect behavior at runtime.


I think the right way to handle this is to make a custom toJS function  
that does a runtime check of the actual type, and makes the right kind  
of wrapper object. That's because any time you deal with an  
AbstractWorker pointer, you want toJS on it to return the canonical  
wrapper of the right concrete class. The toJS function for Node is a  
fairly elaborate example of this.




2) What does GenerateNativeConverter do? I note that both Node and  
Element.idl define GenerateToJS, but none of their derived classes  
(HTMLElement.idl, HTMLOptionElement.idl) define GenerateToJS, which  
makes me think that just defining GenerateToJS on derived classes is  
not the correct fix.


Correct - see above. toJS needs to do a dynamic check based on the  
runtime type, not a static check based on the declared type. We don't  
ever want to vend an Element wrapper for something that actually  
should be an HTMLDivElement, even if it got returned from a context  
that deals with generic elements.





I don't know how useful the diff would be, but I uploaded one here -  
it's not ready for review yet as it has lots of odd debugging  
flotsam in it: https://bugs.webkit.org/attachment.cgi?id=32257action=review


It sounds like you're on the right track to figuring this out. I  
believe writing a custom toJS for AbstractWorker which checks what  
kind of object it actually has will fix your problem.


Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Inheritance in IDL files/JS bindings

2009-07-03 Thread Drew Wilson
I'm working on refactoring some code out of WebCore::Worker into a common
base class to be shared between SharedWorker and Worker.
So I've defined a new AbstractWorker.idl and its associated JS bindings.
AbstractWorker is not intended to be instantiable, so I don't have a
constructor generated for it.

interface [CustomMarkFunction, Conditional=WORKERS] AbstractWorker {
...
}

I then changed Worker.idl to make the Worker interface derive from the base
AbstractWorker interface, like so:

interface [CustomMarkFunction, Conditional=WORKERS] Worker :
AbstractWorker {
   ...
}

Everything compiles just fine, except that at runtime when I instantiate a
Worker object from Javascript, I seem to get an instance of AbstractWorker
instead, and referencing things that should be defined on the Worker
prototype (like Worker::postMessage) yields undefined. I thought at first
that perhaps I was doing something wrong with the prototype (setup in
JSWorkerConstructor.cpp) so I wrote some JS tests, which produced the
expected results:

log(Worker.toString());   = [object WorkerConstructor]
log(Worker.prototype.toString());= [object WorkerPrototype]
log(Worker.prototype.postMessage.toString());   = function
postMessage() { [native code] }

And yet when I actually instantiate a Worker object:

var worker = new Worker(resources/worker-common.js);
log (worker.toString());= [object *AbstractWorker*]
log (typeof worker.postMessage);= *undefined*

Is there anything special I need to do when implementing inheritance in IDL
files and JS bindings? I *think* I'm correctly following the example of
other places where we use inheritance (example HTMLElement - Element -
Node), although there are some options defined in the .idl file like
GenerateNativeConverter and GenerateToJS that might be relevant but
which I couldn't find any documentation for.

Anyhow, I'm digging into this further, but I figured people might be able to
shed some light on possible causes of this behavior.

-atw
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Inheritance in IDL files/JS bindings

2009-07-03 Thread Maciej Stachowiak


On Jul 3, 2009, at 2:42 PM, Drew Wilson wrote:



Is there anything special I need to do when implementing inheritance  
in IDL files and JS bindings? I *think* I'm correctly following the  
example of other places where we use inheritance (example  
HTMLElement - Element - Node), although there are some options  
defined in the .idl file like GenerateNativeConverter and  
GenerateToJS that might be relevant but which I couldn't find any  
documentation for.


Anyhow, I'm digging into this further, but I figured people might be  
able to shed some light on possible causes of this behavior.


Hard to tell without seeing your diff, but based on this I'd guess the  
bug is in the Worker constructor. Is there any chance it could be  
accidentally instantiating the wrong class, or setting the wrong  
prototype?


 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Inheritance in IDL files/JS bindings

2009-07-03 Thread Sam Weinig
On Fri, Jul 3, 2009 at 2:42 PM, Drew Wilson atwil...@google.com wrote:

 I'm working on refactoring some code out of WebCore::Worker into a common
 base class to be shared between SharedWorker and Worker.
 So I've defined a new AbstractWorker.idl and its associated JS bindings.
 AbstractWorker is not intended to be instantiable, so I don't have a
 constructor generated for it.

 interface [CustomMarkFunction, Conditional=WORKERS] AbstractWorker {
 ...
 }


GenerateConstructor is a slight misnomer unfortunately.  Event
non-instatiatable interfaces should have one so that they can be exposed on
the global object for use with instanceof and accessing the prototype.



 I then changed Worker.idl to make the Worker interface derive from the base
 AbstractWorker interface, like so:

 interface [CustomMarkFunction, Conditional=WORKERS] Worker :
 AbstractWorker {
...
 }

 Everything compiles just fine, except that at runtime when I instantiate a
 Worker object from Javascript, I seem to get an instance of AbstractWorker
 instead, and referencing things that should be defined on the Worker
 prototype (like Worker::postMessage) yields undefined. I thought at first
 that perhaps I was doing something wrong with the prototype (setup in
 JSWorkerConstructor.cpp) so I wrote some JS tests, which produced the
 expected results:

 log(Worker.toString());   = [object WorkerConstructor]
 log(Worker.prototype.toString());= [object WorkerPrototype]
 log(Worker.prototype.postMessage.toString());   = function
 postMessage() { [native code] }

 And yet when I actually instantiate a Worker object:

 var worker = new Worker(resources/worker-common.js);
 log (worker.toString());= [object *AbstractWorker*]
 log (typeof worker.postMessage);= *undefined*

 Is there anything special I need to do when implementing inheritance in IDL
 files and JS bindings? I *think* I'm correctly following the example of
 other places where we use inheritance (example HTMLElement - Element -
 Node), although there are some options defined in the .idl file like
 GenerateNativeConverter and GenerateToJS that might be relevant but
 which I couldn't find any documentation for.

 Anyhow, I'm digging into this further, but I figured people might be able
 to shed some light on possible causes of this behavior.


It is hard to say without a full diff.  Nothing you have described seems too
far off.

-Sam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev