WebCore:
authorjianli@chromium.org <jianli@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Jul 2009 17:45:04 +0000 (17:45 +0000)
committerjianli@chromium.org <jianli@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Jul 2009 17:45:04 +0000 (17:45 +0000)
2009-07-27  Jian Li  <jianli@chromium.org>

        Reviewed by David Levin.

        Fix error handling in dedicated worker and worker context.
        https://bugs.webkit.org/show_bug.cgi?id=27525

        The following problems have been fixed:
        1) The uncaught runtime script error is not reported using the
        WorkerGlobalScope object's onerror attribute.
        2) If the error is still not handled afterwards (onerror attribute
        is not defined as a function or it returns true), the error should
        be reported back to the associated Worker object by firing an
        ErrorEvent.
        3) If the error is still not handled by the associated Worker
        object, the error should be reported to the user.

        Test: fast/workers/worker-script-error.html

        * bindings/js/JSEventListener.cpp:
        (WebCore::JSEventListener::reportError):
        * bindings/js/JSEventListener.h:
        * dom/EventListener.h:
        (WebCore::EventListener::reportError): adds a function to call
        EventListener as a function with 3 arguments to report an error.
        * workers/AbstractWorker.cpp:
        (WebCore::AbstractWorker::dispatchScriptErrorEvent):
        * workers/AbstractWorker.h:
        * workers/DedicatedWorkerContext.cpp:
        (WebCore::DedicatedWorkerContext::reportException):
        * workers/WorkerContext.cpp:
        (WebCore::WorkerContext::reportException):
        * workers/WorkerContext.h:
        * workers/WorkerMessagingProxy.cpp:
        (WebCore::WorkerExceptionTask::performTask):
        * workers/WorkerMessagingProxy.h:

LayoutTests:

2009-07-27  Jian Li  <jianli@chromium.org>

        Reviewed by David Levin.

        Layout tests for fixing error handling in dedicated worker and worker
        context.
        https://bugs.webkit.org/show_bug.cgi?id=27525

        * fast/workers/resources/worker-error-in-handling-script-error.js: Added
        * fast/workers/resources/worker-invalid-syntax.js: Added.
        * fast/workers/resources/worker-script-error-bubbled.js: Added.
        * fast/workers/resources/worker-script-error-handled.js: Added.
        * fast/workers/resources/worker-script-error-unhandled.js: Added.
        * fast/workers/worker-constructor.html:
        * fast/workers/worker-script-error-expected.txt: Added.
        * fast/workers/worker-script-error.html: Added.
        Add test cases to cover different script error handling scenarios.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@46419 268f45cc-cd09-0410-ab3c-d52691b4dbfc

20 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/workers/resources/worker-error-in-handling-script-error.js [new file with mode: 0644]
LayoutTests/fast/workers/resources/worker-invalid-syntax.js [new file with mode: 0644]
LayoutTests/fast/workers/resources/worker-script-error-bubbled.js [new file with mode: 0644]
LayoutTests/fast/workers/resources/worker-script-error-handled.js [new file with mode: 0644]
LayoutTests/fast/workers/resources/worker-script-error-unhandled.js [new file with mode: 0644]
LayoutTests/fast/workers/worker-constructor.html
LayoutTests/fast/workers/worker-script-error-expected.txt [new file with mode: 0644]
LayoutTests/fast/workers/worker-script-error.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/bindings/js/JSEventListener.cpp
WebCore/bindings/js/JSEventListener.h
WebCore/dom/EventListener.h
WebCore/workers/AbstractWorker.cpp
WebCore/workers/AbstractWorker.h
WebCore/workers/DedicatedWorkerContext.cpp
WebCore/workers/WorkerContext.cpp
WebCore/workers/WorkerContext.h
WebCore/workers/WorkerMessagingProxy.cpp
WebCore/workers/WorkerMessagingProxy.h

index db795d3..643ca4e 100644 (file)
@@ -1,3 +1,21 @@
+2009-07-27  Jian Li  <jianli@chromium.org>
+
+        Reviewed by David Levin.
+
+        Layout tests for fixing error handling in dedicated worker and worker
+        context.
+        https://bugs.webkit.org/show_bug.cgi?id=27525
+
+        * fast/workers/resources/worker-error-in-handling-script-error.js: Added
+        * fast/workers/resources/worker-invalid-syntax.js: Added.
+        * fast/workers/resources/worker-script-error-bubbled.js: Added.
+        * fast/workers/resources/worker-script-error-handled.js: Added.
+        * fast/workers/resources/worker-script-error-unhandled.js: Added.
+        * fast/workers/worker-constructor.html:
+        * fast/workers/worker-script-error-expected.txt: Added.
+        * fast/workers/worker-script-error.html: Added.
+        Add test cases to cover different script error handling scenarios.
+
 2009-07-27  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>
 
         Reviewed by George Staikos.
diff --git a/LayoutTests/fast/workers/resources/worker-error-in-handling-script-error.js b/LayoutTests/fast/workers/resources/worker-error-in-handling-script-error.js
new file mode 100644 (file)
index 0000000..18b8d0c
--- /dev/null
@@ -0,0 +1,7 @@
+onerror = function(message, url, lineno)
+{
+    bar.foo = 0;
+    return false;
+}
+
+foo.bar = 0;
diff --git a/LayoutTests/fast/workers/resources/worker-invalid-syntax.js b/LayoutTests/fast/workers/resources/worker-invalid-syntax.js
new file mode 100644 (file)
index 0000000..a74eea6
--- /dev/null
@@ -0,0 +1 @@
+Invalid syntax.
diff --git a/LayoutTests/fast/workers/resources/worker-script-error-bubbled.js b/LayoutTests/fast/workers/resources/worker-script-error-bubbled.js
new file mode 100644 (file)
index 0000000..a20d410
--- /dev/null
@@ -0,0 +1,7 @@
+onerror = function(message, url, lineno)
+{
+    postMessage("onerror invoked for a script that has script error '" + message + "' at line " + lineno);
+    return true;
+}
+
+foo.bar = 0;
diff --git a/LayoutTests/fast/workers/resources/worker-script-error-handled.js b/LayoutTests/fast/workers/resources/worker-script-error-handled.js
new file mode 100644 (file)
index 0000000..50d9077
--- /dev/null
@@ -0,0 +1,7 @@
+onerror = function(message, url, lineno)
+{
+    postMessage("onerror invoked for a script that has script error '" + message + "' at line " + lineno);
+    return false;
+}
+
+foo.bar = 0;
diff --git a/LayoutTests/fast/workers/resources/worker-script-error-unhandled.js b/LayoutTests/fast/workers/resources/worker-script-error-unhandled.js
new file mode 100644 (file)
index 0000000..5a6605d
--- /dev/null
@@ -0,0 +1 @@
+foo.bar = 0;
index 6b69927..0f83be1 100644 (file)
@@ -7,13 +7,22 @@ function log(message)
     document.getElementById("result").innerHTML += message + "<br>";
 }
 
+var testCases = [
+    "testArgumentException",
+    "testRecursiveWorkerCreation",
+    "testNoArgument",
+    "testEmptyScriptUrl",
+    "testInvalidScriptUrl",
+    "testNotExistentScriptUrl",
+    "testSuccessWorkerCreation",
+];
+var testIndex = 0;
+
 function runNextTest()
 {
-    testIndex++;
-    // Loop until there are no more tests to run
-    var testName = 'test' + testIndex;
-    if (window[testName]) {
-        window[testName]();
+    if (testIndex < testCases.length) {
+        testIndex++;
+        window[testCases[testIndex - 1]]();
     } else {
         log("DONE");
         if (window.layoutTestController)
@@ -21,7 +30,7 @@ function runNextTest()
     }
 }
 
-function test1()
+function testArgumentException()
 {
     try {
         new Worker({toString:function(){throw "exception"}})
@@ -35,7 +44,7 @@ function test1()
     runNextTest();
 }
 
-function test2()
+function testRecursiveWorkerCreation()
 {
     try {
         var foo = {toString:function(){new Worker(foo)}}
@@ -47,7 +56,7 @@ function test2()
     runNextTest();
 }
 
-function test3()
+function testNoArgument()
 {
     try {
         new Worker();
@@ -58,7 +67,7 @@ function test3()
     runNextTest();
 }
 
-function test4()
+function testEmptyScriptUrl()
 {
     try {
         var worker = new Worker("");
@@ -72,7 +81,7 @@ function test4()
     }
 }
 
-function test5()
+function testInvalidScriptUrl()
 {
     try {
         var worker = new Worker("invalidurl://");
@@ -86,7 +95,7 @@ function test5()
     }
 }
 
-function test6()
+function testNotExistentScriptUrl()
 {
     try {
         var worker = new Worker("does-not-exist.js");
@@ -100,7 +109,7 @@ function test6()
     }
 }
 
-function test7()
+function testSuccessWorkerCreation()
 {
     try {
         var worker = new Worker("resources/worker-common.js");
@@ -122,7 +131,6 @@ if (window.layoutTestController) {
     layoutTestController.waitUntilDone();
 }
 
-var testIndex = 0;
 runNextTest();
 
 </script>
diff --git a/LayoutTests/fast/workers/worker-script-error-expected.txt b/LayoutTests/fast/workers/worker-script-error-expected.txt
new file mode 100644 (file)
index 0000000..70e24c4
--- /dev/null
@@ -0,0 +1,11 @@
+CONSOLE MESSAGE: line 7: ReferenceError: Can't find variable: foo
+Test Worker script error handling functionality. Should print a series of PASS messages, followed with DONE.
+
+PASS: onerror invoked for a script that has invalid syntax.
+PASS: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 1.
+PASS: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 7.
+PASS: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 7.
+PASS: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 7.
+PASS: message received from WorkerGlobalScope.onerror: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 7.
+DONE
+
diff --git a/LayoutTests/fast/workers/worker-script-error.html b/LayoutTests/fast/workers/worker-script-error.html
new file mode 100644 (file)
index 0000000..e550868
--- /dev/null
@@ -0,0 +1,135 @@
+<body>
+<p>Test Worker script error handling functionality. Should print a series of PASS messages, followed with DONE.</p>
+<div id=result></div>
+<script>
+function log(message)
+{
+    document.getElementById("result").innerHTML += message + "<br>";
+}
+
+var testCases = [
+    "testInvalidScriptSyntax",
+    "testScriptErrorUnhandled",
+    "testErrorInHandlingScriptError",
+    "testScriptErrorBubbledAndHandledInWorker",
+    "testScriptErrorBubbledAndReportedToUser",
+    "testScriptErrorHandled",
+];
+var testIndex = 0;
+
+function runNextTest()
+{
+    if (testIndex < testCases.length) {
+        testIndex++;
+        window[testCases[testIndex - 1]]();
+    } else {
+        log("DONE");
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+}
+
+function testInvalidScriptSyntax()
+{
+    try {
+        var worker = new Worker("resources/worker-invalid-syntax.js");
+        worker.onerror = function() {
+            log("PASS: onerror invoked for a script that has invalid syntax.");
+            runNextTest();
+            return false;
+        }
+    } catch (ex) {
+        log("FAIL: unexpected exception " + ex);
+        runNextTest();
+    }
+}
+
+function testScriptErrorUnhandled()
+{
+    try {
+        var worker = new Worker("resources/worker-script-error-unhandled.js");
+        worker.onerror = function(evt) {
+            log("PASS: onerror invoked for a script that has script error '" + evt.message + "' at line " + evt.lineno + ".");
+            runNextTest();
+            return false;
+        }
+    } catch (ex) {
+        log("FAIL: unexpected exception " + ex);
+        runNextTest();
+    }
+}
+
+function testErrorInHandlingScriptError()
+{
+    try {
+        var worker = new Worker("resources/worker-error-in-handling-script-error.js");
+        worker.onerror = function(evt) {
+            log("PASS: onerror invoked for a script that has script error '" + evt.message + "' at line " + evt.lineno + ".");
+            runNextTest();
+            return false;
+        }
+    } catch (ex) {
+        log("FAIL: unexpected exception " + ex);
+        runNextTest();
+    }
+}
+
+function testScriptErrorBubbledAndHandledInWorker()
+{
+    try {
+        var worker = new Worker("resources/worker-script-error-bubbled.js");
+        worker.onerror = function(evt) {
+            log("PASS: onerror invoked for a script that has script error '" + evt.message + "' at line " + evt.lineno + ".");
+            runNextTest();
+            return false;
+        }
+    } catch (ex) {
+        log("FAIL: unexpected exception " + ex);
+        runNextTest();
+    }
+}
+
+function testScriptErrorBubbledAndReportedToUser()
+{
+    try {
+        var worker = new Worker("resources/worker-script-error-bubbled.js");
+        worker.onerror = function(evt) {
+            log("PASS: onerror invoked for a script that has script error '" + evt.message + "' at line " + evt.lineno + ".");
+            runNextTest();
+            return true;
+        }
+    } catch (ex) {
+        log("FAIL: unexpected exception " + ex);
+        runNextTest();
+    }
+}
+
+function testScriptErrorHandled()
+{
+    try {
+        var worker = new Worker("resources/worker-script-error-handled.js");
+        worker.onerror = function(evt) {
+            log("FAIL: onerror invoked for a script that has script error '" + evt.message + "' at line " + evt.lineno + ".");
+            runNextTest();
+            return false;
+        }
+        worker.onmessage = function(evt) {
+            log("PASS: message received from WorkerGlobalScope.onerror: " + evt.data + ".");
+            runNextTest();
+        }
+    } catch (ex) {
+        log("FAIL: unexpected exception " + ex);
+        runNextTest();
+    }
+}
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+runNextTest();
+
+</script>
+</body>
+
index 4e1dbf2..17f0329 100644 (file)
@@ -1,3 +1,40 @@
+2009-07-27  Jian Li  <jianli@chromium.org>
+
+        Reviewed by David Levin.
+
+        Fix error handling in dedicated worker and worker context.
+        https://bugs.webkit.org/show_bug.cgi?id=27525
+
+        The following problems have been fixed:
+        1) The uncaught runtime script error is not reported using the
+        WorkerGlobalScope object's onerror attribute.
+        2) If the error is still not handled afterwards (onerror attribute
+        is not defined as a function or it returns true), the error should
+        be reported back to the associated Worker object by firing an
+        ErrorEvent.
+        3) If the error is still not handled by the associated Worker
+        object, the error should be reported to the user.
+
+        Test: fast/workers/worker-script-error.html
+
+        * bindings/js/JSEventListener.cpp:
+        (WebCore::JSEventListener::reportError):
+        * bindings/js/JSEventListener.h:
+        * dom/EventListener.h:
+        (WebCore::EventListener::reportError): adds a function to call
+        EventListener as a function with 3 arguments to report an error.
+        * workers/AbstractWorker.cpp:
+        (WebCore::AbstractWorker::dispatchScriptErrorEvent):
+        * workers/AbstractWorker.h:
+        * workers/DedicatedWorkerContext.cpp:
+        (WebCore::DedicatedWorkerContext::reportException):
+        * workers/WorkerContext.cpp:
+        (WebCore::WorkerContext::reportException):
+        * workers/WorkerContext.h:
+        * workers/WorkerMessagingProxy.cpp:
+        (WebCore::WorkerExceptionTask::performTask):
+        * workers/WorkerMessagingProxy.h:
+
 2009-07-27  Nikolas Zimmermann  <nikolas.zimmermann@torchmobile.com>
 
         Reviewed by George Staikos.
index b9ed685..777ac71 100644 (file)
@@ -153,6 +153,53 @@ void JSEventListener::handleEvent(Event* event, bool isWindowEvent)
     }
 }
 
+bool JSEventListener::reportError(const String& message, const String& url, int lineNumber)
+{
+    JSLock lock(false);
+
+    JSObject* jsFunction = this->jsFunction();
+    if (!jsFunction)
+        return false;
+
+    JSDOMGlobalObject* globalObject = m_globalObject;
+    if (!globalObject)
+        return false;
+
+    ExecState* exec = globalObject->globalExec();
+
+    CallData callData;
+    CallType callType = jsFunction->getCallData(callData);
+
+    if (callType == CallTypeNone)
+        return false;
+
+    MarkedArgumentBuffer args;
+    args.append(jsString(exec, message));
+    args.append(jsString(exec, url));
+    args.append(jsNumber(exec, lineNumber));
+
+    // If this event handler is the first JavaScript to execute, then the
+    // dynamic global object should be set to the global object of the
+    // window in which the event occurred.
+    JSGlobalData* globalData = globalObject->globalData();
+    DynamicGlobalObjectScope globalObjectScope(exec, globalData->dynamicGlobalObject ? globalData->dynamicGlobalObject : globalObject);    
+
+    JSValue thisValue = globalObject->toThisObject(exec);
+
+    globalObject->globalData()->timeoutChecker.start();
+    JSValue returnValue = call(exec, jsFunction, callType, callData, thisValue, args);
+    globalObject->globalData()->timeoutChecker.stop();
+
+    // If an error occurs while handling the script error, it should be bubbled up.
+    if (exec->hadException()) {
+        exec->clearException();
+        return false;
+    }
+    
+    bool bubbleEvent;
+    return returnValue.getBoolean(bubbleEvent) && !bubbleEvent;
+}
+
 bool JSEventListener::virtualisAttribute() const
 {
     return m_isAttribute;
index ce34832..2497750 100644 (file)
@@ -45,6 +45,7 @@ namespace WebCore {
     private:
         virtual void markJSFunction();
         virtual void handleEvent(Event*, bool isWindowEvent);
+        virtual bool reportError(const String& message, const String& url, int lineNumber);
         virtual bool virtualisAttribute() const;
         void clearJSFunctionInline();
 
index dbc41b2..d288c8d 100644 (file)
@@ -21,6 +21,7 @@
 #ifndef EventListener_h
 #define EventListener_h
 
+#include "PlatformString.h"
 #include <wtf/RefCounted.h>
 
 namespace JSC {
@@ -35,6 +36,8 @@ namespace WebCore {
     public:
         virtual ~EventListener() { }
         virtual void handleEvent(Event*, bool isWindowEvent = false) = 0;
+        // Return true to indicate that the error is handled.
+        virtual bool reportError(const String& /*message*/, const String& /*url*/, int /*lineNumber*/) { return false; }
         virtual bool wasCreatedFromMarkup() const { return false; }
 
 #if USE(JSC)
index d7766b6..7cb2c11 100644 (file)
@@ -115,18 +115,23 @@ void AbstractWorker::dispatchLoadErrorEvent()
     ASSERT(!ec);
 }
 
-void AbstractWorker::dispatchScriptErrorEvent(const String& message, const String& sourceURL, int lineNumber)
+bool AbstractWorker::dispatchScriptErrorEvent(const String& message, const String& sourceURL, int lineNumber)
 {
+    bool handled = false;
     RefPtr<ErrorEvent> event = ErrorEvent::create(message, sourceURL, static_cast<unsigned>(lineNumber));
     if (m_onErrorListener) {
         event->setTarget(this);
         event->setCurrentTarget(this);
         m_onErrorListener->handleEvent(event.get(), true);
+        if (event->defaultPrevented())
+            handled = true;
     }
 
     ExceptionCode ec = 0;
     dispatchEvent(event.release(), ec);
     ASSERT(!ec);
+
+    return handled;
 }
 
 } // namespace WebCore
index 5bc5e2e..28cc50d 100644 (file)
@@ -56,7 +56,7 @@ namespace WebCore {
 
         // Utility routines to generate appropriate error events for loading and script exceptions.
         void dispatchLoadErrorEvent();
-        void dispatchScriptErrorEvent(const String& errorMessage, const String& sourceURL, int);
+        bool dispatchScriptErrorEvent(const String& errorMessage, const String& sourceURL, int);
 
         void setOnerror(PassRefPtr<EventListener> eventListener) { m_onErrorListener = eventListener; }
         EventListener* onerror() const { return m_onErrorListener.get(); }
index 4d47aaf..97c1581 100644 (file)
@@ -55,7 +55,12 @@ DedicatedWorkerContext::~DedicatedWorkerContext()
 
 void DedicatedWorkerContext::reportException(const String& errorMessage, int lineNumber, const String& sourceURL)
 {
-    thread()->workerObjectProxy().postExceptionToWorkerObject(errorMessage, lineNumber, sourceURL);
+    bool errorHandled = false;
+    if (onerror())
+        errorHandled = onerror()->reportError(errorMessage, sourceURL, lineNumber);
+
+    if (!errorHandled)
+        thread()->workerObjectProxy().postExceptionToWorkerObject(errorMessage, lineNumber, sourceURL);
 }
 
 void DedicatedWorkerContext::postMessage(const String& message, ExceptionCode& ec)
index 3ceb2a9..9d88b75 100644 (file)
@@ -138,11 +138,6 @@ bool WorkerContext::hasPendingActivity() const
     return false;
 }
 
-void WorkerContext::reportException(const String& errorMessage, int lineNumber, const String& sourceURL)
-{
-    m_thread->workerObjectProxy().postExceptionToWorkerObject(errorMessage, lineNumber, sourceURL);
-}
-
 void WorkerContext::addMessage(MessageDestination destination, MessageSource source, MessageType type, MessageLevel level, const String& message, unsigned lineNumber, const String& sourceURL)
 {
     m_thread->workerObjectProxy().postConsoleMessageToWorkerObject(destination, source, type, level, message, lineNumber, sourceURL);
index 828c592..266553a 100644 (file)
@@ -67,7 +67,7 @@ namespace WebCore {
 
         bool hasPendingActivity() const;
 
-        virtual void reportException(const String& errorMessage, int lineNumber, const String& sourceURL);
+        virtual void reportException(const String& errorMessage, int lineNumber, const String& sourceURL) = 0;
         virtual void addMessage(MessageDestination, MessageSource, MessageType, MessageLevel, const String& message, unsigned lineNumber, const String& sourceURL);
         virtual void resourceRetrievedByXMLHttpRequest(unsigned long identifier, const ScriptString& sourceString);
         virtual void scriptImported(unsigned long identifier, const String& sourceString);
index 9bac2c8..5971c1d 100644 (file)
@@ -127,7 +127,15 @@ private:
 
     virtual void performTask(ScriptExecutionContext* context)
     {
-        if (!m_messagingProxy->askedToTerminate())
+        Worker* workerObject = m_messagingProxy->workerObject();
+        if (!workerObject || m_messagingProxy->askedToTerminate())
+            return;
+
+        bool errorHandled = false;
+        if (workerObject->onerror())
+            errorHandled = workerObject->dispatchScriptErrorEvent(m_errorMessage, m_sourceURL, m_lineNumber);
+
+        if (!errorHandled)
             context->reportException(m_errorMessage, m_lineNumber, m_sourceURL);
     }
 
index e13eb11..b705ca4 100644 (file)
@@ -82,6 +82,7 @@ namespace WebCore {
     private:
         friend class MessageWorkerTask;
         friend class WorkerContextDestroyedTask;
+        friend class WorkerExceptionTask;
         friend class WorkerThreadActivityReportTask;
 
         virtual ~WorkerMessagingProxy();