Bug 461917. Do a better job of reporting pending exceptions when compiling an event listener. r=mrbkap, sr=jst
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 03 Dec 2008 09:41:09 -0500
changeset 22253 9c8ba21dc1ccf5c337e6142106ce6bf06b5a66c6
parent 22252 d351bde7a804b9347a70a27402fe6ec5dd3bd6bd
child 22254 fdb12933052c4b060cce7afcef567d017233e39e
push idunknown
push userunknown
push dateunknown
reviewersmrbkap, jst
bugs461917
milestone1.9.2a1pre
Bug 461917. Do a better job of reporting pending exceptions when compiling an event listener. r=mrbkap, sr=jst
content/events/src/nsEventListenerManager.cpp
dom/public/nsIScriptContext.h
dom/src/base/nsJSEnvironment.cpp
--- a/content/events/src/nsEventListenerManager.cpp
+++ b/content/events/src/nsEventListenerManager.cpp
@@ -819,17 +819,16 @@ nsEventListenerManager::AddScriptEventLi
 
         rv = context->CompileEventHandler(aName, argCount, argNames,
                                           aBody,
                                           url.get(), lineNo,
                                           SCRIPTVERSION_DEFAULT, // for now?
                                           handler);
         if (rv == NS_ERROR_ILLEGAL_VALUE) {
           NS_WARNING("Probably a syntax error in the event handler!");
-          context->ReportPendingException();
           return NS_SUCCESS_LOSS_OF_INSIGNIFICANT_DATA;
         }
         NS_ENSURE_SUCCESS(rv, rv);
         // And bind it.
         rv = context->BindCompiledEventHandler(aObject, scope,
                                                aName, handler);
       }
       if (NS_FAILED(rv)) return rv;
--- a/dom/public/nsIScriptContext.h
+++ b/dom/public/nsIScriptContext.h
@@ -168,16 +168,21 @@ public:
    * Compile the event handler named by atom aName, with function body aBody
    * into a function object returned if ok via aHandler.  Does NOT bind the
    * function to anything - BindCompiledEventHandler() should be used for that
    * purpose.  Note that this event handler is always considered 'shared' and
    * hence is compiled without principals.  Never call the returned object
    * directly - it must be bound (and thereby cloned, and therefore have the 
    * correct principals) before use!
    *
+   * If the compilation sets a pending exception on the native context, it is
+   * this method's responsibility to report said exception immediately, without
+   * relying on callers to do so.
+   *
+   *
    * @param aName an nsIAtom pointer naming the function; it must be lowercase
    *        and ASCII, and should not be longer than 63 chars.  This bound on
    *        length is enforced only by assertions, so caveat caller!
    * @param aEventName the name that the event object should be bound to
    * @param aBody the event handler function's body
    * @param aURL the URL or filename for error messages
    * @param aLineNo the starting line number of the script for error messages
    * @param aVersion the script language version to use when executing
--- a/dom/src/base/nsJSEnvironment.cpp
+++ b/dom/src/base/nsJSEnvironment.cpp
@@ -418,20 +418,22 @@ NS_HandleScriptError(nsIScriptGlobalObje
 // it has not been done is that the code below only fills the error event
 // after it has a good nsPresContext - whereas using the above function
 // would involve always filling it.  Is that a concern?
 void
 NS_ScriptErrorReporter(JSContext *cx,
                        const char *message,
                        JSErrorReport *report)
 {
-  JSStackFrame * fp = nsnull;
-  while ((fp = JS_FrameIterator(cx, &fp))) {
-    if (!JS_IsNativeFrame(cx, fp)) {
-      return;
+  { // Scope for |fp|
+    JSStackFrame * fp = nsnull;
+    while ((fp = JS_FrameIterator(cx, &fp))) {
+      if (!JS_IsNativeFrame(cx, fp)) {
+        return;
+      }
     }
   }
 
   nsIXPConnect* xpc = nsContentUtils::XPConnect();
   if (xpc) {
     nsAXPCNativeCallContext *cc = nsnull;
     xpc->GetCurrentNativeCallContext(&cc);
     if (cc) {
@@ -1814,16 +1816,19 @@ nsJSContext::CompileEventHandler(nsIAtom
                                  const char** aArgNames,
                                  const nsAString& aBody,
                                  const char *aURL, PRUint32 aLineNo,
                                  PRUint32 aVersion,
                                  nsScriptObjectHolder &aHandler)
 {
   NS_ENSURE_TRUE(mIsInitialized, NS_ERROR_NOT_INITIALIZED);
 
+  NS_PRECONDITION(!::JS_IsExceptionPending(mContext),
+                  "Why are we being called with a pending exception?");
+
   if (!sSecurityManager) {
     NS_ERROR("Huh, we need a script security manager to compile "
              "an event handler!");
 
     return NS_ERROR_UNEXPECTED;
   }
 
   // Don't compile if aVersion is unknown.  Since the caller is responsible for
@@ -1844,16 +1849,21 @@ nsJSContext::CompileEventHandler(nsIAtom
       ::JS_CompileUCFunctionForPrincipals(mContext,
                                           nsnull, nsnull,
                                           charName, aArgCount, aArgNames,
                                           (jschar*)PromiseFlatString(aBody).get(),
                                           aBody.Length(),
                                           aURL, aLineNo);
 
   if (!fun) {
+    // Set aside the frame chain on cx while reporting, since it has
+    // nothing to do with the error we just hit.
+    JSStackFrame* frame = JS_SaveFrameChain(mContext);
+    ReportPendingException();
+    JS_RestoreFrameChain(mContext, frame);
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
   JSObject *handler = ::JS_GetFunctionObject(fun);
   NS_ASSERTION(aHandler.getScriptTypeID()==JAVASCRIPT,
                "Expecting JS script object holder");
   return aHandler.set((void *)handler);
 }