WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(3.64 KB, patch)
2012-02-02 14:39 PST
,
Benjamin Poulain
msaboff
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Benjamin Poulain
Comment 1
2012-02-01 18:40:06 PST
Created
attachment 125063
[details]
Patch
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
Created
attachment 125192
[details]
Patch
Benjamin Poulain
Comment 10
2012-02-02 15:42:52 PST
Committed
r106600
: <
http://trac.webkit.org/changeset/106600
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug