[webkit-reviews] review denied: [Bug 10114] Exceptions thrown by WebKitPlugins are ignored : [Attachment 9688] Patch to fix WebKitPlugin exceptions

[bugzilla-request-daemon at opendarwin.org]

Geoffrey Garen <ggaren at apple.com> has denied Geoffrey Garen
<ggaren at apple.com>'s request for review:
Bug 10114: Exceptions thrown by WebKitPlugins are ignored
http://bugzilla.opendarwin.org/show_bug.cgi?id=10114

Attachment 9688: Patch to fix WebKitPlugin exceptions
http://bugzilla.opendarwin.org/attachment.cgi?id=9688&action=edit

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
I don't think it's right for the WebScriptObject API to be responsible for
tracking the current ExecState. That's really a job for the JS engine. We
attempted a similar hack to this one in JavaScriptGlue, and bad things happened
(crashes, incompatibilities when the engine changed, mismatches between the
stored ExecState and the real one). There's also the threading issue you
mention, if we add support for ObjC plug-ins to the JS API.

The interpreter knows about the global ExecState and the most nested Context. I
think a better solution would be to use that model. Giving Context a
back-pointer to its sibling ExecState (ExecState already keeps a pointer to its
sibling Context) is probably easiest. Then you can write code like this:

throwError(interp->context()->execState(), GeneralError, exceptionMessage);

Since Context and ExecState should probably merge in the end, the code can
eventually become this:

throwError(interp->context(), GeneralError, exceptionMessage);

For now, a Context is transient and stack-allocated, so there aren't any
ownership issues to worry about.