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

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Darin Adler <darin 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 9864: Patch to fix WebKitPlugin exceptions
http://bugzilla.opendarwin.org/attachment.cgi?id=9864&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Is there a reason the ExecState function has been changed to no longer be
inlined? I ask because I'm concerned about how the change might affect
performance, and avoiding one extra level of function call in code that is used
in only a few places seems still worthwhile.

Formatting is incorrect in ExecState.cpp: indenting is inconsistent and we
normally indent the initializers rather than putting them flush left.

+    ExecState* execState() { return (m_execState); }

We don't put parentheses around return statements like these.

We'll need to do a performance test to check that this is OK.

Actually I have one big question. What guarantees that the context won't
outlast the execution state? I'm worried that the code in WebScriptObject.mm
might try to set the exception in a deallocated ExecState on the stack.

review- only because of that one last question. The other issues seem to be
minor ones.