[Promise] Avoid crash in stack exhausted circumstance.

[Promise] Avoid crash in stack exhausted circumstance.

Some operations such as createClosure and v8::ObjectTemplate::NewInstance
returns an empty handle when the stack is exhausted.
Promise should throw a StackOverFlow exception in such a case.

BUG=345088
R=haraken

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

[haraken, 2014-02-21 04:19:20]

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/V8Initial...
File Source/bindings/v8/V8Initializer.cpp (right):

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/V8Initial...
Source/bindings/v8/V8Initializer.cpp:88: static const char
stackOverFlowMessageString[] = "Uncaught RangeError: Maximum call stack size
exceeded";

Can we share code with handleMaxRecursionDepthExeceeded() in V8Binding.h ?

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/custom/V8...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8PromiseCustom.cpp:56: #define
RETURN_IF_EMPTY(handle, result) \

I don't know if this macro is worth adding. We normally just write 'if
(a.IsEmpty()) return b;'.

[yhirano, 2014-02-21 07:32:51]

I'm sorry, there were some missing empty guards at PS1 and I added them at PS2.

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/V8Initial...
File Source/bindings/v8/V8Initializer.cpp (right):

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/V8Initial...
Source/bindings/v8/V8Initializer.cpp:88: static const char
stackOverFlowMessageString[] = "Uncaught RangeError: Maximum call stack size
exceeded";
On 2014/02/21 04:19:21, haraken wrote:
> 
> Can we share code with handleMaxRecursionDepthExeceeded() in V8Binding.h ?

Done.

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/custom/V8...
File Source/bindings/v8/custom/V8PromiseCustom.cpp (right):

https://codereview.chromium.org/174073009/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8PromiseCustom.cpp:56: #define
RETURN_IF_EMPTY(handle, result) \
On 2014/02/21 04:19:21, haraken wrote:
> 
> I don't know if this macro is worth adding. We normally just write 'if
> (a.IsEmpty()) return b;'.

Done.

[haraken, 2014-02-21 09:34:06]

(Probably I should have mentioned this before starting reviewing...) I'm not
sure if it's a good idea to judge a stack overflow using the contents of an
exception message. It looks fragile. Probably we want to add a V8 API that
provides us the information about the stack overflow.

I'd like to delegate the judgement to abarth.

[haraken, 2014-02-21 09:34:50]

+jochen, dan

[yhirano, 2014-02-26 01:14:09]

abarth, jochen, dcarney: PTAL?

It is now a stable blocker.

[abarth-chromium, 2014-02-26 06:34:40]

This CL seems a bit hacky (e.g., doing string comparisons to determine what
happened in another part of the engine), but I don't really feel qualified to
review it.  Hopefully one of the other reviewers can help you out.

[dcarney, 2014-02-26 10:01:16]

Given that errors can be monkey patched, this is it not a great way to detect
what type of error is coming through.  I'm looking at adding something to the v8
api right now.  If it's too complex, I guess we're stuck with this.

[dcarney, 2014-02-26 12:08:44]

https://codereview.chromium.org/176843003/ gives you an api.  Please wait for it
as it will change the magic string you are using.

[yhirano, 2014-02-27 08:52:16]

Discussed with dcarney.
We decided to separate changes to two CLs.
This CL changes V8PromiseCustom only.

Note: With the inspector enabled the renderer process still crashes.

[haraken, 2014-02-27 09:08:19]

LGTM

[dcarney, 2014-02-27 09:08:47]

lgtm

[yhirano, 2014-02-28 17:48:33]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-02-28 17:48:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/174073009/170001

[yhirano, 2014-02-28 17:49:26]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2014-03-03 08:41:22]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-03-03 08:41:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/174073009/170001

[commit-bot: I haz the power, 2014-03-03 11:17:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-03 11:17:51]

Retried try job too often on win_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[yhirano, 2014-03-03 12:49:37]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-03-03 12:49:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/174073009/170001

[commit-bot: I haz the power, 2014-03-03 12:52:20]

Change committed as 168306