[V8] Add name of function for function's closure scope

[V8] Add name of function for function's closure scope

Added ScopeDetails.name field for closure scopes. It contains function's debug name of current context of scope.

BUG=493156
LOG=Y
R=yurys@chromium.org,yangguo@chromium.org

Committed: https://crrev.com/ce54e16270e6fe46b1afbd4256a0f23243ff623e
Cr-Commit-Position: refs/heads/master@{#31028}

[kozy, 2015-09-29 00:52:41]

Yury, please take a look.

[yurys, 2015-09-29 02:05:19]

Please add a test

[kozy, 2015-09-29 03:33:27]

On 2015/09/29 02:05:19, yurys wrote:
> Please add a test

Done.

[yurys, 2015-09-29 08:24:00]

lgtm

https://codereview.chromium.org/1375813002/diff/20001/test/mjsunit/debug-scop...
File test/mjsunit/debug-scopes.js (right):

https://codereview.chromium.org/1375813002/diff/20001/test/mjsunit/debug-scop...
test/mjsunit/debug-scopes.js:156: assertScopeMirrorEquals(all_scopes[i], scope);
I think we can omit this check here as it is unrelated to the name and already
tested elsewhere. Same for allCopes.length vs. scopeCount since allScopes() is
just a shortcut to get all scopes at once as an array.

[kozy, 2015-09-29 15:15:32]

All done. Yang, please take a look.

https://codereview.chromium.org/1375813002/diff/20001/test/mjsunit/debug-scop...
File test/mjsunit/debug-scopes.js (right):

https://codereview.chromium.org/1375813002/diff/20001/test/mjsunit/debug-scop...
test/mjsunit/debug-scopes.js:156: assertScopeMirrorEquals(all_scopes[i], scope);
On 2015/09/29 08:24:00, yurys wrote:
> I think we can omit this check here as it is unrelated to the name and already
> tested elsewhere. Same for allCopes.length vs. scopeCount since allScopes() is
> just a shortcut to get all scopes at once as an array.

Done.

[Yang, 2015-09-30 10:55:04]

lgtm with nits

https://codereview.chromium.org/1375813002/diff/40001/src/debug/debug-scopes.cc
File src/debug/debug-scopes.cc (right):

https://codereview.chromium.org/1375813002/diff/40001/src/debug/debug-scopes....
src/debug/debug-scopes.cc:135: if (HasContext() && CurrentContext()->closure())
{
Please do explicitly compare with NULL here. We do this elsewhere too.

https://codereview.chromium.org/1375813002/diff/40001/src/debug/debug-scopes....
src/debug/debug-scopes.cc:138: if (closure_name.location() &&
closure_name->length())
How about (!closure_name.is_null() && (closure_name->length() != 0)) instead?

https://codereview.chromium.org/1375813002/diff/40001/src/debug/mirrors.js
File src/debug/mirrors.js (right):

https://codereview.chromium.org/1375813002/diff/40001/src/debug/mirrors.js#ne...
src/debug/mirrors.js:2238: var kScopeDetailsNameIndex = 2;
Please add a comment here that these indices correspond to the definitions in
debug-scopes.h

[kozy, 2015-09-30 17:00:18]

All done.

https://codereview.chromium.org/1375813002/diff/40001/src/debug/debug-scopes.cc
File src/debug/debug-scopes.cc (right):

https://codereview.chromium.org/1375813002/diff/40001/src/debug/debug-scopes....
src/debug/debug-scopes.cc:135: if (HasContext() && CurrentContext()->closure())
{
On 2015/09/30 10:55:04, Yang OOO until mid October wrote:
> Please do explicitly compare with NULL here. We do this elsewhere too.

Done.

https://codereview.chromium.org/1375813002/diff/40001/src/debug/debug-scopes....
src/debug/debug-scopes.cc:138: if (closure_name.location() &&
closure_name->length())
On 2015/09/30 10:55:04, Yang OOO until mid October wrote:
> How about (!closure_name.is_null() && (closure_name->length() != 0)) instead?

Done.

https://codereview.chromium.org/1375813002/diff/40001/src/debug/mirrors.js
File src/debug/mirrors.js (right):

https://codereview.chromium.org/1375813002/diff/40001/src/debug/mirrors.js#ne...
src/debug/mirrors.js:2238: var kScopeDetailsNameIndex = 2;
On 2015/09/30 10:55:04, Yang OOO until mid October wrote:
> Please add a comment here that these indices correspond to the definitions in
> debug-scopes.h

Done.

[kozy, 2015-09-30 17:00:22]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2015-09-30 17:00:22]

The patchset sent to the CQ was uploaded after l-g-t-m from yurys@chromium.org,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1375813002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-09-30 17:00:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1375813002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1375813002/60001

[commit-bot: I haz the power, 2015-09-30 17:24:32]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-30 17:24:42]

Patchset 4 (id:??) landed as
https://crrev.com/ce54e16270e6fe46b1afbd4256a0f23243ff623e
Cr-Commit-Position: refs/heads/master@{#31028}