[webkit-dev] JavascriptCore questions

[Edward Rowe]

Thanks for the responses Geoff - this has helped alot. Please see some follow-ups inline below. I'm new to this thing, so apologies in advance for any naïve questions.
 

-----Original Message-----
From: Geoffrey Garen [mailto:ggaren at apple.com] 
Subject: Re: [webkit-dev] JavascriptCore questions


Hi Ed.

>> Interpreters, ExecStates, Binding
>> ---------------------------------
>> - ConvertValueToNPVariant() and convertValueToObjcValue(), which are 
>> used in the plugin interface binding code, both contain special logic 
>> that basically says "if wrapping up a window object, store the 
>> RootObject associated with the window object in the wrapper rather 
>> than the current RootObject". Can you tell me the purpose of this 
>> logic? I have theories, but nothing definitive.
>
>I'm not sure.

Can you give me any advice on how to find out? 


>> - The JavaScriptObject struct contains pointers to RootObjects. I may 
>> be missing something, but it appears to me that a JavaScriptObject 
>> struct can outlive the RootObjects that it points at, which could lead 
>> to the use of freed memory. Can this happen?
>
>Yes, I think so. That's a bug. We're tracking some bugs along those lines.

Should I enter a bug for this, or is it already covered?



>> - When a plugin is unloaded, what happens if there are existing 
>> RuntimeObjectImp instances that reference objects provided by the 
>> plugin? Is there logic to prevent a plugin from being unloaded if 
>> there are extant objects that it has provided? I haven't been able to 
>> find any code that ensures this.
>
>No. I believe this is a bug.

Should I enter a bug?



>> - There are a variety of places where
>> exec->dynamicInterpreter()->globalObject() is called. For example,
>> this
>> happens in FunctionProtoFunc::callAsFunction() to handle the apply 
>> case when the this arg is NULL. In such cases, is it correct to use 
>> the global obj from the current ExecState's Interpreter as opposed to 
>> the global object at the root of the scope chain? ECMA 262 section
>> 15.3.4.4
>> seems somewhat ambiguous on this subject, but my reading leads me to 
>> think that it would be more correct to pull from the scope chain.
>
> Filed as http://bugs.webkit.org/show_bug.cgi?id=11884.

I'm aware of several other places that may also be similar problems. Would you like me to update the bug you entered? Enter separate bugs for each? Just send my findings to this list?




>> - The default implementation of JSObject::defaultValue() and
>> JSObject::toPrimitive() can return an Error object if an exception 
>> occurs. This would seem to violate ECMA 262 sections 8.6.2.6 and 9.1 
>> which state that defaultvalue() and toPrimitive() return non-Object 
>> types. Looking at some of the callers of toPrimitive(), it appears 
>> that some are not prepared to handle getting an object back. I'll file 
>> this as a bug if you guys like.
>
>defaultValue() and toPrimitive() must be able to throw exceptions because they call arbitrary functions. When the >spec says, "The above specification of [[DefaultValue]] for native objects can return only primitive values," I think it's just attempting to explain that the purpose of defaultValue() is to return a primitive value, although it can throw an exception as well. If there's code that might fail because an exception is thrown, I think that warrants a bug.

DateObjectImp::construct(), equal(), relation(), add() and UserObjectImp::getOwnPropertySlot() all call toPrimitive() and do not appear to be prepared to handle the case where an Error object is returned/an exception is set in the exec. Bugs?



>> - Neither JSObject::propertyIsEnumerable() nor
>> JSObject::getPropertyAttributes() are virtual, making it impossible 
>> for a subclass to override the default behavior. Any objection to 
>> making
>> propertyIsEnumerable() virtual, and making getPropertyAttributes() 
>> private?
>
>If you want to override those functions, you're probably (although not definitely) doing things wrong. Object has default behavior, allowing you to add and remove properties with attributes. Subclasses can implement custom properties, overriding get, put, getPropertyNames, etc. So there's no need to muck around in Object's implementation of its default behavior, when you can override that behavior, instead. In other words, the functions you mention should really be private, instead of virtual.

I'd prefer to make them both private, but it looks like JSObject::propertyIsEnumerable() is called by ObjectProtoFunc::callAsFunction() in the "PropertyIsEnumerable" case. Since a caller can use Function.call to set the "this" object used by ObjectProtoFunc::callAsFunction(), and thus we can't assume this obj is any particular subclass of JSObject, it would appear that PropertyIsEnumerable needs to remain public. If you concur with this analysis, any objection to making JSObject::PropertyIsEnumerable() virtual? As it is only called by ObjectProtoFunc::callAsFunction()'s PropertyIsEnumerable handler, it seems unlikely that there will be any interesting perf hit from doing so.

Along the same lines, JSObject::prototype() is not virtual, and this prevents me from overriding it to return the correct prototype (the proto for the object in the other engine) in the case I'm dealing with. Instead, a caller gets the local prototype (the prototype for the wrapper) which is incorrect. Note that RuntimeObjectImp (in the binding code) also suffers from this affliction. Any objection to making JSObject::prototype() virtual, or suggestions for other approaches?


Thanks
Ed