<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[56329] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/56329">56329</a></dd>
<dt>Author</dt> <dd>yurys@chromium.org</dd>
<dt>Date</dt> <dd>2010-03-22 02:46:16 -0700 (Mon, 22 Mar 2010)</dd>
</dl>

<h3>Log Message</h3>
<pre>2010-03-22  Yury Semikhatsky  &lt;yurys@chromium.org&gt;

        Reviewed by Pavel Feldman.

        Handle worker exceptions in V8MessageHandler like it's done in regular documents. This way all worker exceptions will be logged in the console not only those which happen in event listeners. 

        https://bugs.webkit.org/show_bug.cgi?id=31171

        * bindings/v8/V8AbstractEventListener.cpp:
        (WebCore::V8AbstractEventListener::invokeEventHandler): Removed explicit call to reportException.
        * bindings/v8/V8Utilities.cpp: reportException function was removed since it's not used.
        (WebCore::getScriptExecutionContext):
        * bindings/v8/V8Utilities.h:
        * bindings/v8/WorkerContextExecutionProxy.cpp:
        (WebCore::v8MessageHandler):
        (WebCore::WorkerContextExecutionProxy::initContextIfNeeded): Setup message handler when first worker context is created.

2010-03-22  Yury Semikhatsky  &lt;yurys@chromium.org&gt;

        Reviewed by Pavel Feldman.

        Test that uncaught exception thrown from setTimeout callback in a Worker is reported to the worker object.

        https://bugs.webkit.org/show_bug.cgi?id=31171

        * fast/workers/resources/worker-exception-in-timeout-callback.js: Added.
        * fast/workers/worker-script-error-expected.txt:
        * fast/workers/worker-script-error.html:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastworkersworkerscripterrorexpectedtxt">trunk/LayoutTests/fast/workers/worker-script-error-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastworkersworkerscripterrorhtml">trunk/LayoutTests/fast/workers/worker-script-error.html</a></li>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCorebindingsv8V8AbstractEventListenercpp">trunk/WebCore/bindings/v8/V8AbstractEventListener.cpp</a></li>
<li><a href="#trunkWebCorebindingsv8V8Utilitiescpp">trunk/WebCore/bindings/v8/V8Utilities.cpp</a></li>
<li><a href="#trunkWebCorebindingsv8V8Utilitiesh">trunk/WebCore/bindings/v8/V8Utilities.h</a></li>
<li><a href="#trunkWebCorebindingsv8WorkerContextExecutionProxycpp">trunk/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastworkersresourcesworkerexceptionintimeoutcallbackjs">trunk/LayoutTests/fast/workers/resources/worker-exception-in-timeout-callback.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/LayoutTests/ChangeLog        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2010-03-22  Yury Semikhatsky  &lt;yurys@chromium.org&gt;
+
+        Reviewed by Pavel Feldman.
+
+        Test that uncaught exception thrown from setTimeout callback in a Worker is reported to the worker object.
+
+        https://bugs.webkit.org/show_bug.cgi?id=31171
+
+        * fast/workers/resources/worker-exception-in-timeout-callback.js: Added.
+        * fast/workers/worker-script-error-expected.txt:
+        * fast/workers/worker-script-error.html:
+
</ins><span class="cx"> 2010-03-22  Kenneth Rohde Christiansen  &lt;kenneth@webkit.org&gt;
</span><span class="cx"> 
</span><span class="cx">         Reviewed by Simon Hausmann.
</span></span></pre></div>
<a id="trunkLayoutTestsfastworkersresourcesworkerexceptionintimeoutcallbackjs"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/workers/resources/worker-exception-in-timeout-callback.js (0 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/workers/resources/worker-exception-in-timeout-callback.js                                (rev 0)
+++ trunk/LayoutTests/fast/workers/resources/worker-exception-in-timeout-callback.js        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+setTimeout(function() {
+    throw &quot;Exception in setTimeout callback&quot;;
+}, 0);
</ins></span></pre></div>
<a id="trunkLayoutTestsfastworkersworkerscripterrorexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/workers/worker-script-error-expected.txt (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/workers/worker-script-error-expected.txt        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/LayoutTests/fast/workers/worker-script-error-expected.txt        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -8,5 +8,6 @@
</span><span class="cx"> PASS: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 7.
</span><span class="cx"> PASS: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 7.
</span><span class="cx"> PASS: message received from WorkerGlobalScope.onerror: onerror invoked for a script that has script error 'ReferenceError: Can't find variable: foo' at line 7.
</span><ins>+PASS: onerror invoked for an exception in setTimeout callback.
</ins><span class="cx"> DONE
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkLayoutTestsfastworkersworkerscripterrorhtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/workers/worker-script-error.html (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/workers/worker-script-error.html        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/LayoutTests/fast/workers/worker-script-error.html        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -15,6 +15,7 @@
</span><span class="cx">     &quot;testScriptErrorBubbledAndHandledInWorker&quot;,
</span><span class="cx">     &quot;testScriptErrorBubbledAndReportedToUser&quot;,
</span><span class="cx">     &quot;testScriptErrorHandled&quot;,
</span><ins>+    &quot;testExceptionInTimeoutCallback&quot;,
</ins><span class="cx"> ];
</span><span class="cx"> var testIndex = 0;
</span><span class="cx"> 
</span><span class="lines">@@ -140,6 +141,21 @@
</span><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+function testExceptionInTimeoutCallback()
+{
+    try {
+        var worker = new Worker(&quot;resources/worker-exception-in-timeout-callback.js&quot;);
+        worker.onerror = function() {
+            log(&quot;PASS: onerror invoked for an exception in setTimeout callback.&quot;);
+            runNextTest();
+            return false;
+        }
+    } catch (ex) {
+        log(&quot;FAIL: unexpected exception &quot; + ex);
+        runNextTest();
+    }
+}
+
</ins><span class="cx"> if (window.layoutTestController) {
</span><span class="cx">     layoutTestController.dumpAsText();
</span><span class="cx">     layoutTestController.waitUntilDone();
</span></span></pre></div>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/WebCore/ChangeLog        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -1,3 +1,20 @@
</span><ins>+2010-03-22  Yury Semikhatsky  &lt;yurys@chromium.org&gt;
+
+        Reviewed by Pavel Feldman.
+
+        Handle worker exceptions in V8MessageHandler like it's done in regular documents. This way all worker exceptions will be logged in the console not only those which happen in event listeners. 
+
+        https://bugs.webkit.org/show_bug.cgi?id=31171
+
+        * bindings/v8/V8AbstractEventListener.cpp:
+        (WebCore::V8AbstractEventListener::invokeEventHandler): Removed explicit call to reportException.
+        * bindings/v8/V8Utilities.cpp: reportException function was removed since it's not used.
+        (WebCore::getScriptExecutionContext):
+        * bindings/v8/V8Utilities.h:
+        * bindings/v8/WorkerContextExecutionProxy.cpp:
+        (WebCore::v8MessageHandler):
+        (WebCore::WorkerContextExecutionProxy::initContextIfNeeded): Setup message handler when first worker context is created.
+
</ins><span class="cx"> 2010-03-22  Leandro Pereira  &lt;leandro@profusion.mobi&gt;
</span><span class="cx"> 
</span><span class="cx">         Reviewed by Simon Hausmann.
</span></span></pre></div>
<a id="trunkWebCorebindingsv8V8AbstractEventListenercpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/v8/V8AbstractEventListener.cpp (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/v8/V8AbstractEventListener.cpp        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/WebCore/bindings/v8/V8AbstractEventListener.cpp        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -145,11 +145,8 @@
</span><span class="cx">         if (!tryCatch.CanContinue())
</span><span class="cx">             return;
</span><span class="cx"> 
</span><del>-        // If an error occurs while handling the event, it should be reported.
-        if (tryCatch.HasCaught()) {
-            reportException(0, tryCatch);
-            tryCatch.Reset();
-        }
</del><ins>+        // If an error occurs while handling the event, it should be reported in a regular way.
+        tryCatch.Reset();
</ins><span class="cx"> 
</span><span class="cx">         // Restore the old event. This must be done for all exit paths through this method.
</span><span class="cx">         if (savedEvent.IsEmpty())
</span></span></pre></div>
<a id="trunkWebCorebindingsv8V8Utilitiescpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/v8/V8Utilities.cpp (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/v8/V8Utilities.cpp        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/WebCore/bindings/v8/V8Utilities.cpp        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -125,7 +125,7 @@
</span><span class="cx">         frame-&gt;redirectScheduler()-&gt;scheduleLocationChange(url.string(), callingFrame-&gt;loader()-&gt;outgoingReferrer(), lockHistory, lockBackForwardList, processingUserGesture());
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-ScriptExecutionContext* getScriptExecutionContext(ScriptState* scriptState)
</del><ins>+ScriptExecutionContext* getScriptExecutionContext()
</ins><span class="cx"> {
</span><span class="cx"> #if ENABLE(WORKERS)
</span><span class="cx">     WorkerContextExecutionProxy* proxy = WorkerContextExecutionProxy::retrieve();
</span><span class="lines">@@ -133,46 +133,11 @@
</span><span class="cx">         return proxy-&gt;workerContext()-&gt;scriptExecutionContext();
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-    Frame* frame;
-    if (scriptState) {
-        v8::HandleScope handleScope;
-        frame = V8Proxy::retrieveFrame(scriptState-&gt;context());
-    } else
-        frame = V8Proxy::retrieveFrameForCurrentContext();
-
</del><ins>+    Frame* frame = V8Proxy::retrieveFrameForCurrentContext();
</ins><span class="cx">     if (frame)
</span><span class="cx">         return frame-&gt;document()-&gt;scriptExecutionContext();
</span><span class="cx"> 
</span><span class="cx">     return 0;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void reportException(ScriptState* scriptState, v8::TryCatch&amp; exceptionCatcher)
-{
-    String errorMessage;
-    int lineNumber = 0;
-    String sourceURL;
-
-    // There can be a situation that an exception is thrown without setting a message.
-    v8::Local&lt;v8::Message&gt; message = exceptionCatcher.Message();
-    if (message.IsEmpty()) {
-        v8::Local&lt;v8::String&gt; exceptionString = exceptionCatcher.Exception()-&gt;ToString();
-        // Conversion of the exception object to string can fail if an
-        // exception is thrown during conversion.
-        if (!exceptionString.IsEmpty())
-            errorMessage = toWebCoreString(exceptionString);
-    } else {
-        errorMessage = toWebCoreString(message-&gt;Get());
-        lineNumber = message-&gt;GetLineNumber();
-        sourceURL = toWebCoreString(message-&gt;GetScriptResourceName());
-    }
-
-    // Do not report the exception if the current execution context is Document because we do not want to lead to duplicate error messages in the console.
-    // FIXME (31171): need better design to solve the duplicate error message reporting problem.
-    ScriptExecutionContext* context = getScriptExecutionContext(scriptState);
-    // During the frame teardown, there may not be a valid context.
-    if (context &amp;&amp; !context-&gt;isDocument())
-        context-&gt;reportException(errorMessage, lineNumber, sourceURL);
-    exceptionCatcher.Reset();
-}
-
</del><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkWebCorebindingsv8V8Utilitiesh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/v8/V8Utilities.h (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/v8/V8Utilities.h        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/WebCore/bindings/v8/V8Utilities.h        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -54,13 +54,8 @@
</span><span class="cx">     KURL completeURL(const String&amp; relativeURL);
</span><span class="cx">     void navigateIfAllowed(Frame*, const KURL&amp;, bool lockHistory, bool lockBackForwardList);
</span><span class="cx"> 
</span><del>-    ScriptExecutionContext* getScriptExecutionContext(ScriptState*);
-    inline ScriptExecutionContext* getScriptExecutionContext() {
-        return getScriptExecutionContext(0);
-    }
</del><ins>+    ScriptExecutionContext* getScriptExecutionContext();
</ins><span class="cx"> 
</span><del>-    void reportException(ScriptState*, v8::TryCatch&amp;);
-
</del><span class="cx">     class AllowAllocation {
</span><span class="cx">     public:
</span><span class="cx">         inline AllowAllocation()
</span></span></pre></div>
<a id="trunkWebCorebindingsv8WorkerContextExecutionProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp (56328 => 56329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp        2010-03-22 09:43:28 UTC (rev 56328)
+++ trunk/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp        2010-03-22 09:46:16 UTC (rev 56329)
</span><span class="lines">@@ -40,6 +40,7 @@
</span><span class="cx"> #include &quot;SharedWorker.h&quot;
</span><span class="cx"> #include &quot;SharedWorkerContext.h&quot;
</span><span class="cx"> #include &quot;V8Binding.h&quot;
</span><ins>+#include &quot;V8ConsoleMessage.h&quot;
</ins><span class="cx"> #include &quot;V8DOMMap.h&quot;
</span><span class="cx"> #include &quot;V8DedicatedWorkerContext.h&quot;
</span><span class="cx"> #include &quot;V8Proxy.h&quot;
</span><span class="lines">@@ -58,6 +59,26 @@
</span><span class="cx">     CRASH();
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+static void v8MessageHandler(v8::Handle&lt;v8::Message&gt; message, v8::Handle&lt;v8::Value&gt; data)
+{
+    static bool isReportingException = false;
+    // Exceptions that occur in error handler should be ignored since in that case
+    // WorkerContext::reportException will send the exception to the worker object.
+    if (isReportingException)
+        return;
+    isReportingException = true;
+
+    // During the frame teardown, there may not be a valid context.
+    if (ScriptExecutionContext* context = getScriptExecutionContext()) {
+        String errorMessage = toWebCoreString(message-&gt;Get());
+        int lineNumber = message-&gt;GetLineNumber();
+        String sourceURL = toWebCoreString(message-&gt;GetScriptResourceName());
+        context-&gt;reportException(errorMessage, lineNumber, sourceURL);
+    }
+
+    isReportingException = false;
+}
+
</ins><span class="cx"> WorkerContextExecutionProxy::WorkerContextExecutionProxy(WorkerContext* workerContext)
</span><span class="cx">     : m_workerContext(workerContext)
</span><span class="cx">     , m_recursion(0)
</span><span class="lines">@@ -127,6 +148,12 @@
</span><span class="cx">     if (!m_context.IsEmpty())
</span><span class="cx">         return;
</span><span class="cx"> 
</span><ins>+    // Setup the security handlers and message listener. This only has
+    // to be done once.
+    static bool isV8Initialized = false;
+    if (!isV8Initialized)
+        v8::V8::AddMessageListener(&amp;v8MessageHandler);
+
</ins><span class="cx">     // Create a new environment
</span><span class="cx">     v8::Persistent&lt;v8::ObjectTemplate&gt; globalTemplate;
</span><span class="cx">     m_context = v8::Context::New(0, globalTemplate);
</span></span></pre>
</div>
</div>

</body>
</html>