Hi again!
- 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?
I would suggest testing the NPAPI with this feature enabled and
disabled, and seeing if you can suss out any difference. We have a
test Netscape plug-in in WebKitTools/DumpRenderTree that you can
modify to suite your needs. If you have some theories about why the
code is written as it is, I'd be happy to entertain them :).
- 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?
If you have reproducible steps for causing a crash, that's probably
worth its own bug. Otherwise, I think the existing bug is sufficient.
- 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?
Sure.
- 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?
A bug report is always better than an email. In theory, each instance
of this mistake produces a different buggy behavior, so separate
reports for each would probably be best.
- 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?
Technically, all of these except for getOwnPropertySlot are probably
bugs. (Whether [[get]] should throw an exception when it encounters a
bad value is up to the class doing the getting.)
However, it's hard to imagine a situation where things would go
wrong. If I called 'new Date(valueThatDoesNotConvertToPrimitive)',
DateObjectImp::construct() would not return an error object, but the
exception would be set in the ExecState, and the Node tree would
catch and report it as a part of evaluation. (Remember
KJS_CHECKEXCPETION*.) Whether a function wants to return immediately
when an exception is thrown is, I guess, up to the function.
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.
Ah! I missed that case. Yes, I think it's correct to make
PropertyIsEnumerable virtual.
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?
I hesitate to make prototype access virtual. It's a part of property
lookup, which is our hottest code.
I'm not sure I understand why you think RuntimeObjectImp::prototype()
returns the wrong prototype. A RuntimeObjectImp is a JavaScript
object that communicates with an object in another language runtime
and, as a JavaScript object, it has as its prototype the default
object prototype.
Your case is unique, since you want a JavaScript object that not only
to communicates with, but embodies, a JavaScript object in another
language runtime, prototype and all. To accomplish that, I think you
need to do a one-time fix-up of the object's prototype chain at
create time. This approach is not ideal, but the cost it incurs is
about equal to the cost of a single property lookup, and it happens
during a much more expensive object allocation, anyway.
Geoff
_______________________________________________
webkit-dev mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-dev
