Changeset 167142 in webkit for trunk/Source/JavaScriptCore/bindings/ScriptFunctionCall.cpp

Timestamp:

Apr 11, 2014, 1:24:56 PM (11 years ago)

Author:

[email protected]

Message:

JSMainThreadExecState::call() should clear exceptions before returning.
<​https://webkit.org/b/131530>

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Added a version of JSC::call() that return any uncaught exception instead
of leaving it pending in the VM.

As part of this change, I updated various parts of the code base to use the
new API as needed.

• bindings/ScriptFunctionCall.cpp:

(Deprecated::ScriptFunctionCall::call):

• ScriptFunctionCall::call() is only used by the inspector to inject scripts. The injected scripts that will include Inspector scripts that should catch and handle any exceptions that were thrown. We should not be seeing any exceptions returned from this call. However, we do have checks for exceptions in case there are bugs in the Inspector scripts which allowed the exception to leak through. Hence, it is proper to clear the exception here, and only record the fact that an exception was seen (if present).

• bindings/ScriptFunctionCall.h:
• inspector/InspectorEnvironment.h:
• runtime/CallData.cpp:

(JSC::call):

• runtime/CallData.h:

Source/WebCore:

Test: fast/dom/regress-131530.html

Previously, JSMainThreadExecState::call() did not clear any pending
exceptions in the VM before returning. On returning, the
JSMainThreadExecState destructor may re-enter the VM to notify
MutationObservers. This may result in a crash because the VM expects
exceptions to be cleared at entry.

We now change JSMainThreadExecState::call() to return the exception
(if present) via an argument, and clear it from the VM before returning.

As part of this change, I updated various parts of the code base to use the
new API as needed.

• bindings/js/JSCallbackData.cpp:

(WebCore::JSCallbackData::invokeCallback):

• bindings/js/JSCustomXPathNSResolver.cpp:

(WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):

• bindings/js/JSDOMGlobalObjectTask.cpp:
• Assert that there's no unhandled exception after the Microtask returns. See comment for WebCore::JSMainThreadExecState::runTask below for more details.

• bindings/js/JSErrorHandler.cpp:

(WebCore::JSErrorHandler::handleEvent):

• bindings/js/JSEventListener.cpp:

(WebCore::JSEventListener::handleEvent):

• bindings/js/JSHTMLDocumentCustom.cpp:

(WebCore::JSHTMLDocument::open):

• Document.open() cannot be the first function on the JS stack. Hence, there is no need to use JSMainThreadExecState to call into the VM, as this is only needed to catch the event of returning from the first function for the purpose of notifying MutationObservers. Change to call JSC::call() directly.

• bindings/js/JSMainThreadExecState.cpp:

(WebCore::functionCallHandlerFromAnyThread):

• bindings/js/JSMainThreadExecState.h:

(WebCore::JSMainThreadExecState::call):
(WebCore::JSMainThreadExecState::evaluate):

• Remove the explicitly acquisition of the JSLock here because we now acquire the JSLock as part of the JSMainThreadExecState instance.

(WebCore::JSMainThreadExecState::runTask):

• Added an assert to verify that the task does not return with an unhandled exception. Currently, the only Microtask in use is for the Promise implementation, which will eat the exception before returning. This assertion is added here to verify that this contract does not inadvertantly change in the future.

(WebCore::JSMainThreadExecState::JSMainThreadExecState):

• Now acquires the JSLock as well since by definition, we're only instantiating the JSMainThreadExecState because we're about to enter the VM.

• bindings/js/JSMutationCallback.cpp:

(WebCore::JSMutationCallback::call):

• bindings/js/JSNodeFilterCondition.cpp:

(WebCore::JSNodeFilterCondition::acceptNode):

• acceptNode() is only used in the TreeWalker and NodeIterator APIs which cannot be the first function on the JS stack. Hence, we should call JSC::call() directly instead of going through JSMainThreadExecState.

• bindings/js/ScheduledAction.cpp:

(WebCore::ScheduledAction::executeFunctionInContext):

• bindings/objc/WebScriptObject.mm:

(WebCore::addExceptionToConsole):
(-[WebScriptObject callWebScriptMethod:withArguments:]):

LayoutTests:

• fast/dom/regress-131530-expected.txt: Added.
• fast/dom/regress-131530.html: Added.

File:

• trunk/Source/JavaScriptCore/bindings/ScriptFunctionCall.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/bindings/ScriptFunctionCall.cpp

@@ -134 +134 @@
 
     JSValue result;
+    JSValue exception;
     if (m_callHandler)
-        result = m_callHandler(m_exec, function, callType, callData, thisObject, m_arguments);
+        result = m_callHandler(m_exec, function, callType, callData, thisObject, m_arguments, &exception);
     else
-        result = JSC::call(m_exec, function, callType, callData, thisObject, m_arguments);
+        result = JSC::call(m_exec, function, callType, callData, thisObject, m_arguments, &exception);
 
-    if (m_exec->hadException()) {
+    if (exception) {
         hadException = true;
         return Deprecated::ScriptValue();