Bug 77593 – Running a Web Worker on about:blank crashes the interpreter

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Benjamin Poulain]

...and that is sad
<rdar://problem/10787649>

[Benjamin Poulain]

Created attachment 125063 [details]
Patch

[Benjamin Poulain]

My first idea was to cut short as soon as we load the data for the Worker. The problem is we expose the execution to the Inspector so I would bypass that.

This patch is the lazy approach. I assume the null page is unlikely and I don't care about the cost of starting the Worker thread.

[Alexey Proskuryakov]

Comment on attachment 125063 [details]
Patch

What prevents the same for <script src="about:blank"> and for <a href="javascript:">?

[Benjamin Poulain]

(In reply to comment #3)
> (From update of attachment 125063 [details])
> What prevents the same for <script src="about:blank"> and for <a href="javascript:">?

The same kind of checks. E.g.: http://trac.webkit.org/browser/trunk/Source/WebCore/dom/ScriptElement.cpp#L270

I did not try <a href="javascript:"> but I assume it is the same.

[Alexey Proskuryakov]

The check you mention is not in ScriptController, it's higher on call stack. I'm not sure if it's semantically correct to return ScriptValue() when evaluating an empty string. Testing in JS console, eval("") returns undefined.

Looking at original bug, it appears that UString::is8Bit() is wrong - string methods don't usually crash on null strings. Is this the only reason why it crashes?

[Benjamin Poulain]

(In reply to comment #5)
> The check you mention is not in ScriptController, it's higher on call stack. I'm not sure if it's semantically correct to return ScriptValue() when evaluating an empty string. Testing in JS console, eval("") returns undefined.

Note this is the _Worker_ScriptController. The return value is never used anywhere (see: https://bugs.webkit.org/show_bug.cgi?id=77587 )
I will have to update the patch before landing since 77587 is landed.

As I said, I considered putting this higher in the stack, but it is harder to make a correct patch due to the inspector.

> Looking at original bug, it appears that UString::is8Bit() is wrong - string methods don't usually crash on null strings. Is this the only reason why it crashes?

This is the same behavior as String.

I think I see your point, you think the interpreter should return undefined on a Null source? In that case I should make sure the JIT does the same and then remove all the null check higher in the stack.

[Benjamin Poulain]

> I think I see your point, you think the interpreter should return undefined on a Null source? In that case I should make sure the JIT does the same and then remove all the null check higher in the stack.

Actually the JIT crashes in exactly the same way. It is just crashing on different timing (need 100ms instead of 10 for some reason).

This is a regression.

[Benjamin Poulain]

I think the regression comes from r99812. Previously we were just calling characters() which return 0 on null String.

[Benjamin Poulain]

Created attachment 125192 [details]
Patch

[Benjamin Poulain]

Committed r106600: <http://trac.webkit.org/changeset/106600>