RESOLVED FIXED 77593
Running a Web Worker on about:blank crashes the interpreter
https://bugs.webkit.org/show_bug.cgi?id=77593
Summary Running a Web Worker on about:blank crashes the interpreter
Benjamin Poulain
Reported 2012-02-01 18:26:09 PST
...and that is sad <rdar://problem/10787649>
Attachments
Patch (5.59 KB, patch)
2012-02-01 18:40 PST, Benjamin Poulain
no flags
Patch (3.64 KB, patch)
2012-02-02 14:39 PST, Benjamin Poulain
msaboff: review+
Benjamin Poulain
Comment 1 2012-02-01 18:40:06 PST
Benjamin Poulain
Comment 2 2012-02-01 18:43:10 PST
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 3 2012-02-01 21:47:44 PST
Comment on attachment 125063 [details] Patch What prevents the same for <script src="about:blank"> and for <a href="javascript:">?
Benjamin Poulain
Comment 4 2012-02-01 22:15:34 PST
(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
Comment 5 2012-02-01 23:23:05 PST
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
Comment 6 2012-02-01 23:53:53 PST
(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
Comment 7 2012-02-02 14:27:26 PST
> 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
Comment 8 2012-02-02 14:38:50 PST
I think the regression comes from r99812. Previously we were just calling characters() which return 0 on null String.
Benjamin Poulain
Comment 9 2012-02-02 14:39:05 PST
Benjamin Poulain
Comment 10 2012-02-02 15:42:52 PST
Note You need to log in before you can comment on or make changes to this bug.