[debug] Add Eval scope type to inspector protocol

[debug] Add Eval scope type to inspector protocol

BUG=v8:5530, chromium:667218
Committed: https://crrev.com/0dcc7a0e209f10f88735cb51c4ae0ec32ba183ba
Cr-Commit-Position: refs/heads/master@{#41205}

[jgruber, 2016-11-21 10:55:13]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 10:55:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 11:20:25]

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

[commit-bot: I haz the power, 2016-11-21 11:20:26]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-11-21 11:26:59]

jgruber@chromium.org changed reviewers:
+ kozyatinskiy@chromium.org, yangguo@chromium.org

[Yang, 2016-11-21 11:34:11]

On 2016/11/21 11:27:00, jgruber wrote:

lgtm. thanks!

[kozy, 2016-11-21 16:05:36]

Thanks! Could we add an inspector test for eval scope type?

https://codereview.chromium.org/2519773003/diff/1/src/inspector/js_protocol-1...
File src/inspector/js_protocol-1.2.json (right):

https://codereview.chromium.org/2519773003/diff/1/src/inspector/js_protocol-1...
src/inspector/js_protocol-1.2.json:463: { "name": "type", "type": "string",
"enum": ["global", "local", "with", "closure", "catch", "block", "script",
"eval"], "description": "Scope type." },
It's a copy of protocol when version was incremented to 1.2, we never change
this file and will introduce -1.3 file on next protocol version increment.
Please revert this line.

[dgozman, 2016-11-21 17:24:12]

dgozman@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2016-11-21 17:24:13]

Do we filter empty eval scopes? Most of the time there are a bunch, but all of
them are empty due to non-strict mode.

[kozy, 2016-11-21 17:52:09]

https://codereview.chromium.org/2519773003/diff/1/src/inspector/debugger-scri...
File src/inspector/debugger-script.js (right):

https://codereview.chromium.org/2519773003/diff/1/src/inspector/debugger-scri...
src/inspector/debugger-script.js:542: if (!properties.length && (scopeType ===
ScopeType.Script || scopeType === ScopeType.Block))
Dmitry, good catch! Jakob, please filter empty eval scope here.

[jgruber, 2016-11-22 13:53:13]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 13:53:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jgruber, 2016-11-22 13:55:21]

Thanks for the reviews, comments addressed.

https://codereview.chromium.org/2519773003/diff/1/src/inspector/debugger-scri...
File src/inspector/debugger-script.js (right):

https://codereview.chromium.org/2519773003/diff/1/src/inspector/debugger-scri...
src/inspector/debugger-script.js:542: if (!properties.length && (scopeType ===
ScopeType.Script || scopeType === ScopeType.Block))
On 2016/11/21 17:52:09, kozyatinskiy wrote:
> Dmitry, good catch! Jakob, please filter empty eval scope here.

Done.

https://codereview.chromium.org/2519773003/diff/1/src/inspector/js_protocol-1...
File src/inspector/js_protocol-1.2.json (right):

https://codereview.chromium.org/2519773003/diff/1/src/inspector/js_protocol-1...
src/inspector/js_protocol-1.2.json:463: { "name": "type", "type": "string",
"enum": ["global", "local", "with", "closure", "catch", "block", "script",
"eval"], "description": "Scope type." },
On 2016/11/21 16:05:36, kozyatinskiy wrote:
> It's a copy of protocol when version was incremented to 1.2, we never change
> this file and will introduce -1.3 file on next protocol version increment.
> Please revert this line.

Done.

[commit-bot: I haz the power, 2016-11-22 14:18:51]

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

[commit-bot: I haz the power, 2016-11-22 14:18:51]

Dry run: This issue passed the CQ dry run.

[dgozman, 2016-11-22 15:28:27]

lgtm. Nice patch!

[kozy, 2016-11-22 15:56:11]

lgtm

[jgruber, 2016-11-23 07:28:02]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-11-23 07:28:03]

The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2519773003/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-11-23 07:28:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-23 07:30:02]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1479886082840370,
"parent_rev": "80fb62a66240a7df48912f1f09dc589b8a43767a", "commit_rev":
"4f3059113356371c55d2eff1268a2c641b0d761a"}

[commit-bot: I haz the power, 2016-11-23 07:30:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-23 07:30:31]

Description was changed from

==========
[debug] Add Eval scope type to inspector protocol

BUG=v8:5530,chromium:667218
==========

to

==========
[debug] Add Eval scope type to inspector protocol

BUG=v8:5530,chromium:667218

Committed: https://crrev.com/0dcc7a0e209f10f88735cb51c4ae0ec32ba183ba
Cr-Commit-Position: refs/heads/master@{#41205}
==========

[commit-bot: I haz the power, 2016-11-23 07:30:32]

Patchset 2 (id:??) landed as
https://crrev.com/0dcc7a0e209f10f88735cb51c4ae0ec32ba183ba
Cr-Commit-Position: refs/heads/master@{#41205}

[Yang, 2016-11-23 08:10:52]

On 2016/11/23 07:30:32, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/0dcc7a0e209f10f88735cb51c4ae0ec32ba183ba
> Cr-Commit-Position: refs/heads/master@{#41205}

With this landed, do we already see eval scopes in the scope view in DevTools,
or is there additional work on the UI side?