Bug 30904 – [v8] Missing IsEmpty() check in V8Proxy::sourceName()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Yury Semikhatsky]

V8Proxy::sourceName() calls 'frameSourceName' JavaScript function to retrieve current source name. This call may cause stack overflow in which case result handle is empty. We should check that the result is not an empty handle before converting it to a String.

[Yury Semikhatsky]

Created attachment 42110 [details]
patch

I would like to add a layout test for this patch but it needs to overflow JavaScript stack and takes for about 30 seconds to complete on my machine. I'm going to provide some means for setting JavaScript stack limit from layout tests first and after that commit the test case itself. I'll post the patch with the long running test case a bit later in a separate patch so that you can see what I'm talking about.

Since the patch is fixing one of the most frequent current Chromium crashes I'd prefer to submit it sooner and add the test later.

[Yury Semikhatsky]

Created attachment 42112 [details]
patch

Previous patch with commented code removed.

[Yury Semikhatsky]

Created attachment 42118 [details]
long running test 

Here is the test that I was talking about.

[Eric Seidel (no email)]

Assuming this does not require any special 2-sided coordination, I'm happy to add this to the commit-queue.  As far as I can tell Yury is not a committer yet.

[Pavel Feldman]

(In reply to comment #4)
> Assuming this does not require any special 2-sided coordination, I'm happy to
> add this to the commit-queue.  As far as I can tell Yury is not a committer
> yet.

He was willing to test it on Chrome Mac first. I'll do it for him tomorrow if he is fine with it. Patch looks right to me and is tested on Windows.

[Yury Semikhatsky]

Password for '[e-mail]': 
	M	WebCore/ChangeLog
	M	WebCore/bindings/scripts/CodeGeneratorV8.pm
	M	WebCore/bindings/v8/ScriptCallStack.cpp
	M	WebCore/bindings/v8/ScriptCallStack.h
	M	WebCore/bindings/v8/V8Proxy.cpp
	M	WebCore/bindings/v8/V8Proxy.h
	M	WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp
Committed r50327

[Yury Semikhatsky]

Created attachment 42208 [details]
test case that completes in a reasonable time

Layout test that completes in a reasonable time. Thanks olliej for his suggestion!

[Yury Semikhatsky]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/dom/console-log-stack-overflow-expected.txt
	A	LayoutTests/fast/dom/console-log-stack-overflow.html
Committed r50331