Blink-in-JS: Allow a stackoverflow error thrown by private scripts

Blink-in-JS: Allow a stackoverflow error thrown by private scripts

Currently standard JS errors thrown by private scripts are treated
as real errors of the private scripts and crash the renderer.
However, we need to special-case a stackoverflow error because
user's script can create code that causes a stackoverflow error
in private scripts.

BUG=412143
TEST=fast/dom/private_script_unittest.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181603

[haraken, 2014-09-09 01:57:13]

haraken@chromium.org changed reviewers:
+ jl@opera.com, vivekg@chromium.org, yosin@chromium.org

[haraken, 2014-09-09 01:57:14]

PTAL

[vivekg, 2014-09-09 02:04:04]

https://codereview.chromium.org/553983007/diff/1/Source/core/testing/PrivateS...
File Source/core/testing/PrivateScriptTest.js (right):

https://codereview.chromium.org/553983007/diff/1/Source/core/testing/PrivateS...
Source/core/testing/PrivateScriptTest.js:146:
PrivateScriptTestPrototype.voidMethodThrowsReferenceError = function() {
voidMethodThrowsStackOverflowError ??

[vivekg, 2014-09-09 02:06:47]

https://codereview.chromium.org/553983007/diff/1/LayoutTests/fast/dom/private...
File LayoutTests/fast/dom/private_script_unittest.html (right):

https://codereview.chromium.org/553983007/diff/1/LayoutTests/fast/dom/private...
LayoutTests/fast/dom/private_script_unittest.html:73:
shouldThrow('privateScriptTest.voidMethodThrowsStackOverflowError()');
This might be throwing method not found error and hence passing. I think we
should also check for which exception is being thrown to avoid this kind of
errors. WDYT?

[haraken, 2014-09-09 02:10:04]

https://codereview.chromium.org/553983007/diff/1/LayoutTests/fast/dom/private...
File LayoutTests/fast/dom/private_script_unittest.html (right):

https://codereview.chromium.org/553983007/diff/1/LayoutTests/fast/dom/private...
LayoutTests/fast/dom/private_script_unittest.html:73:
shouldThrow('privateScriptTest.voidMethodThrowsStackOverflowError()');
On 2014/09/09 02:06:47, vivekg_ wrote:
> This might be throwing method not found error and hence passing. I think we
> should also check for which exception is being thrown to avoid this kind of
> errors. WDYT?

Sorry, I was in a rush... Fixed the test case. PTAL :)

[vivekg, 2014-09-09 02:11:35]

https://codereview.chromium.org/553983007/diff/20001/LayoutTests/fast/dom/pri...
File LayoutTests/fast/dom/private_script_unittest-expected.txt (right):

https://codereview.chromium.org/553983007/diff/20001/LayoutTests/fast/dom/pri...
LayoutTests/fast/dom/private_script_unittest-expected.txt:39: PASS
privateScriptTest.voidMethodThrowsStackOverflowError() threw exception
TypeError: undefined is not a function.
RangeError :)

[vivekg, 2014-09-09 02:28:36]

https://codereview.chromium.org/553983007/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/PrivateScriptRunner.cpp (right):

https://codereview.chromium.org/553983007/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/PrivateScriptRunner.cpp:257: // Standard JS errors
thrown by a private script are treated as real errors
Just out-of-curiosity, seems that RangeError is not a defined standard but
within blink/chrome. I just stumbled upon this interesting discussion here [1].

Based on this, I have below two questions:

a. Is there any plan either in ECMAScript6 or ESCMScriptX to make these as
standard exceptions? 
b. Do we need to also handle the out-of-memory case gracefully in the private
scripts? 

[1]
http://esdiscuss.org/topic/standardizing-out-of-memory-and-stack-depth-exceed...

[haraken, 2014-09-09 03:29:45]

Sorry about a lot of confusions. Updated the CL. PTAL.

> thrown by a private script are treated as real errors
> Just out-of-curiosity, seems that RangeError is not a defined standard but
> within blink/chrome. I just stumbled upon this interesting discussion here
[1].
> 
> Based on this, I have below two questions:
> 
> a. Is there any plan either in ECMAScript6 or ESCMScriptX to make these as
> standard exceptions?

I don't know but RangeError is defined as one of standard JS errors in v8.h.
https://code.google.com/p/chromium/codesearch#chromium/src/v8/include/v8.h&q=...

> b. Do we need to also handle the out-of-memory case gracefully in the private
> scripts? 

That's a good point. The answer is yes :)

[vivekg, 2014-09-09 03:34:07]

Also found this case here which probably also should be addressed.

https://code.google.com/p/chromium/codesearch#chromium/src/v8/ChangeLog&l=1235

[vivekg, 2014-09-09 03:41:14]

lgtm

[haraken, 2014-09-09 03:50:22]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-09-09 03:50:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/553983007/80001

[commit-bot: I haz the power, 2014-09-09 04:56:20]

Committed patchset #5 (id:80001) as 181603

[Jens Widell, 2014-09-09 05:12:21]

LGTM