Devtools: parse variables scopes and sourcemap them

Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=

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

[sergeyv, 2016-02-11 02:13:06]

Description was changed from

==========
[WIP] Devtools: parse variables scopes and sourcemap them

BUG=
==========

to

==========
[WIP] Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=
==========

[sergeyv, 2016-02-11 02:13:06]

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

[sergeyv, 2016-02-20 05:06:08]

Description was changed from

==========
[WIP] Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=
==========

to

==========
Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=
==========

[sergeyv, 2016-02-20 05:07:14]

please take a look!

[dgozman, 2016-02-22 17:24:57]

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger/debugger-scope-minified-variables.html
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/sources/debugger/debugger-scope-minified-variables.html:34:
<input type='button' onclick='testFunction()' value='Test'/>
Remove this?

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:33:
var cachedPromise = this._resolvingPromises.get(scope);
Move this check before |var startLocation = ...|

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:61:
var prefix = "function fui";
Maybe add () after function name for perfectly valid JS?

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:72:
if (node.type === "FunctionDeclaration" || node.type === "FunctionExpression")
Does it support arrow functions?

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:76:
declarators.push(node)
missing semicolon

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:96:
var lineEndings = content.lineEndings();
Move this call out of the loop.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:267:
for (var compiledName of namesMapping.keys()) {
Is this operation common enough to justify reverse mapping?

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/inspector.json (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/inspector.json:11: { "name":
"es_tree", "type": "autostart" },
Can we make it on-demand? I'd like to avoid loading acorn if possible.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js:293: var names =
map.names || [];
Do we validate source map contents?

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:715:
WebInspector.sourceMapNamesResolver.resolveScopeInObject(localScope).getAllProperties(false,
this._prepareScopeVariables.bind(this, callFrame));
Can we avoid global singleton?

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sources/module.json (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/module.json:344: "es_tree",
Probably don't need this.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:466: 
nit: extra blank line

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.cpp
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.cpp:166:
v8::Local<v8::Array> locations =
v8::Local<v8::Array>::Cast(m_client->callInternalFunction(func, callFrame, 0,
0).ToLocalChecked());
0, nullptr

[sergeyv, 2016-02-22 21:42:54]

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger/debugger-scope-minified-variables.html
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/sources/debugger/debugger-scope-minified-variables.html:34:
<input type='button' onclick='testFunction()' value='Test'/>
On 2016/02/22 17:24:56, dgozman wrote:
> Remove this?

Done.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:33:
var cachedPromise = this._resolvingPromises.get(scope);
On 2016/02/22 17:24:56, dgozman wrote:
> Move this check before |var startLocation = ...|

Done.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:61:
var prefix = "function fui";
On 2016/02/22 17:24:56, dgozman wrote:
> Maybe add () after function name for perfectly valid JS?

usually scopes start with ();  so we shouldn't add them second time

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:72:
if (node.type === "FunctionDeclaration" || node.type === "FunctionExpression")
On 2016/02/22 17:24:56, dgozman wrote:
> Does it support arrow functions?

I will check this && support it in follow-ups

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:76:
declarators.push(node)
On 2016/02/22 17:24:56, dgozman wrote:
> missing semicolon

Done.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:96:
var lineEndings = content.lineEndings();
On 2016/02/22 17:24:56, dgozman wrote:
> Move this call out of the loop.

Done. But there was no real necessity in this: lineEndings() caches its result.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/bindings/SourceMapNamesResolver.js:267:
for (var compiledName of namesMapping.keys()) {
On 2016/02/22 17:24:56, dgozman wrote:
> Is this operation common enough to justify reverse mapping?

It is used only when user changes a property value in ObjectPropertySection. In
my opinion almost nobody knows about this feature, so this happens extremely
rarely.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/inspector.json (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/inspector.json:11: { "name":
"es_tree", "type": "autostart" },
On 2016/02/22 17:24:56, dgozman wrote:
> Can we make it on-demand? I'd like to avoid loading acorn if possible.

Done. Moved SourceMapNamesResolver.js to sources, so es_tree could be on-demand

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js:293: var names =
map.names || [];
On 2016/02/22 17:24:56, dgozman wrote:
> Do we validate source map contents?

no, we don't validate it.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:715:
WebInspector.sourceMapNamesResolver.resolveScopeInObject(localScope).getAllProperties(false,
this._prepareScopeVariables.bind(this, callFrame));
On 2016/02/22 17:24:56, dgozman wrote:
> Can we avoid global singleton?

Done. Made static function

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sources/module.json (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/module.json:344: "es_tree",
On 2016/02/22 17:24:56, dgozman wrote:
> Probably don't need this.

Done.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js (right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/DebuggerScript.js:466: 
On 2016/02/22 17:24:56, dgozman wrote:
> nit: extra blank line

Done.

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.cpp
(right):

https://codereview.chromium.org/1653053002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/JavaScriptCallFrame.cpp:166:
v8::Local<v8::Array> locations =
v8::Local<v8::Array>::Cast(m_client->callInternalFunction(func, callFrame, 0,
0).ToLocalChecked());
On 2016/02/22 17:24:57, dgozman wrote:
> 0, nullptr

Done.

[dgozman, 2016-02-23 01:00:26]

lgtm

[sergeyv, 2016-03-02 18:37:51]

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

[commit-bot: I haz the power, 2016-03-02 18:38:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1653053002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1653053002/140001

[sergeyv, 2016-03-02 18:39:41]

sergeyv@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2016-03-02 18:40:39]

lgtm

[commit-bot: I haz the power, 2016-03-02 19:33:32]

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

[commit-bot: I haz the power, 2016-03-02 19:33:33]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sergeyv, 2016-03-02 19:46:31]

The CQ bit was checked by sergeyv@chromium.org

[sergeyv, 2016-03-02 19:46:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1653053002/#ps160001
(title: " ")

[commit-bot: I haz the power, 2016-03-02 19:47:09]

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

[commit-bot: I haz the power, 2016-03-02 21:25:39]

Description was changed from

==========
Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=
==========

to

==========
Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=
==========

[commit-bot: I haz the power, 2016-03-02 21:25:40]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-03-02 21:26:41]

Description was changed from

==========
Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=
==========

to

==========
Devtools: [Experiment] parse variables in scopes and sourcemap them

BUG=

Committed: https://crrev.com/e2152de29f6d5b846cd6c7cb3e0a864cfb7a1634
Cr-Commit-Position: refs/heads/master@{#378836}
==========

[commit-bot: I haz the power, 2016-03-02 21:26:42]

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