[Webkit-unassigned] [Bug 10114] Exceptions thrown by WebKitPlugins are ignored

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=10114





------- Comment #8 from ggaren at apple.com  2006-08-01 19:26 PDT -------
This looks pretty good.

+    ExecState(Interpreter* interp, Context* con);

In this prototype, and others, there's no need to provide names that give no
additional data. This would be better:

+    ExecState(Interpreter*, Context*);

+ExecState::ExecState(Interpreter* interp, Context* con)

In this implementation, and others, it would be clearer to use the full words
"interpreter" and "context."

+    void setExecState(ExecState *es) { m_execState = es; }

Our formatting guidelines say to use "Type* " rather than "Type *", even though
not all of our code follows that guideline yet. Also, it's an idiom throughout
JavaScriptCore to name ExecState variables "exec", rather than "es".

 - (void)setException:(NSString *)description
 {
-    if (const RootObject *root = [self _executionContext])
-        throwError(root->interpreter()->globalExec(), GeneralError,
description);
+    [[self class] throwException:description];
 }

This isn't quite right, as it won't necessarily set an exception in the target
object's execution context, which is what the API promises. You really want
logic that says, "Use root->interpreter()->context()->execState() if it exists,
otherwise root->interpreter()->globalExec()."

+    void setExecState(ExecState *es) { m_execState = es; }
+    ExecState* execState() { return (m_execState); }

I think it's impossible to create a Context without having an ExecState
available. So a clearer approach would be to require an ExecState as an
argument to the Context's constructor. Then you could change this code

+          if (state) {
+            throwError(state, GeneralError, exceptionMessage);
             return YES;
+          } else
+            fprintf(stderr, "No exec state for 0x%x\n", (unsigned
int)interp->context());
         }

into an assertion. Also, "state" should be "exec".


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.