Bug 28980 – [V8] Run-time exception in onmessage handler is not forwarded to the worker object.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jian Li]

Run-time exception in onmessage handler is not forwarded to the worker object. Thus the layout test worker-close.html fails in chromium.

[Jian Li]

Created attachment 39083 [details]
Proposed Patch

[Adam Barth]

Comment on attachment 39083 [details]
Proposed Patch

Test?

Also, I don't think getScriptExecutionContext(ScriptState) is correct.  We shouldn't be going through the frame to get the execution context.

[Adam Barth]

Comment on attachment 39083 [details]
Proposed Patch

Jian corrected me that this is already tested by worker-close and that the frame logic is already present.

[Jian Li]

Committed as http://trac.webkit.org/changeset/48072.

[Jian Li]

The commit has been partially reverted due to reliability build break in Chromium. We need to rework on this issue.

[Jian Li]

Created attachment 39289 [details]
Proposed Patch 2

[David Levin]

Comment on attachment 39289 [details]
Proposed Patch 2

Just a few nits to fix on landing.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +        The previous fix is partially reverted due to reliability build break

s/is/was/

s/to/to a/

> +        in chromium. The break happens when an exception is thrown without
> +        setting a message. We need to check for this scenario and handle it.

No need to change this but for future reference, in general this type of comment can go with the function where you are doing the check and handling it.

> diff --git a/WebCore/bindings/v8/V8AbstractEventListener.cpp b/WebCore/bindings/v8/V8AbstractEventListener.cpp
> +        tryCatch.SetVerbose(false);   // We do not want to report the exception to the inspector console.

Single space before end of line comment.

[Jian Li]

Committed as http://trac.webkit.org/changeset/48610.

[Pavel Feldman]

Guys, could we revert this? At least the verbose part that breaks error reporting for devtools. Btw, why do we introduce handling of exceptions with no messages so late in the cycle? I've not seen those before, or do I miss something?

[Adam Barth]

(In reply to comment #9)
> Guys, could we revert this? At least the verbose part that breaks error
> reporting for devtools. Btw, why do we introduce handling of exceptions with no
> messages so late in the cycle? I've not seen those before, or do I miss
> something?

Please file a new bug with this request and cc me, Jian, and levin.

[Yury Semikhatsky]

Filed a new bug: https://bugs.webkit.org/show_bug.cgi?id=31108

(In reply to comment #10)
> (In reply to comment #9)
> > Guys, could we revert this? At least the verbose part that breaks error
> > reporting for devtools. Btw, why do we introduce handling of exceptions with no
> > messages so late in the cycle? I've not seen those before, or do I miss
> > something?
> 
> Please file a new bug with this request and cc me, Jian, and levin.