[inspector] added test infrastructure and test for es6 modules

[inspector] added test infrastructure and test for es6 modules

Test just checks that all basic features are working correctly with modules.

BUG=v8:1569
R=dgozman@chromium.org,alph@chromium.org,adamk@chromium.org

Review-Url: https://codereview.chromium.org/2663743002
Cr-Commit-Position: refs/heads/master@{#42796}
Committed: https://chromium.googlesource.com/v8/v8/+/d6db11fd181061f9ffce4431af5afec0dba2c82c

[kozy, 2017-01-30 08:18:26]

In this CL one test of existing features with modules is added, one-page
proposal for other features could be found by the link:
https://docs.google.com/a/google.com/document/d/1kqmHQ33U-8KBZQloAGuVqEEr6SCk...

Please take a look.

[dgozman, 2017-01-30 19:42:09]

inspector lgtm, but I cannot check the modules usage itself.

https://codereview.chromium.org/2663743002/diff/1/test/inspector/runtime/modu...
File test/inspector/runtime/module.js (right):

https://codereview.chromium.org/2663743002/diff/1/test/inspector/runtime/modu...
test/inspector/runtime/module.js:5: print('Checks basic ES6 modules support.');
Let's rename the file to es6-modules.js.

https://codereview.chromium.org/2663743002/diff/1/test/inspector/task-runner.cc
File test/inspector/task-runner.cc (right):

https://codereview.chromium.org/2663743002/diff/1/test/inspector/task-runner....
test/inspector/task-runner.cc:197: v8::Local<v8::Boolean> is_module =
v8::Boolean::New(isolate, is_module_);
nit: inline?

https://codereview.chromium.org/2663743002/diff/1/test/inspector/task-runner....
test/inspector/task-runner.cc:229: if
(!v8::ScriptCompiler::CompileModule(isolate, &scriptSource)
Adam, could you please check the usage here?

[adamk, 2017-01-30 20:56:59]

lgtm seems good for a start, thanks!

[kozy, 2017-01-30 21:23:33]

all done.

https://codereview.chromium.org/2663743002/diff/1/test/inspector/runtime/modu...
File test/inspector/runtime/module.js (right):

https://codereview.chromium.org/2663743002/diff/1/test/inspector/runtime/modu...
test/inspector/runtime/module.js:5: print('Checks basic ES6 modules support.');
On 2017/01/30 19:42:09, dgozman wrote:
> Let's rename the file to es6-modules.js.

Done.

https://codereview.chromium.org/2663743002/diff/1/test/inspector/task-runner.cc
File test/inspector/task-runner.cc (right):

https://codereview.chromium.org/2663743002/diff/1/test/inspector/task-runner....
test/inspector/task-runner.cc:197: v8::Local<v8::Boolean> is_module =
v8::Boolean::New(isolate, is_module_);
On 2017/01/30 19:42:09, dgozman wrote:
> nit: inline?

Done.

[kozy, 2017-01-30 21:23:57]

Description was changed from

==========
[inspector] added test infrastructure for modules, added test for modules

Test just checks that all basic features are working correctly with modules.

BUG=v8:1569
R=dgozman@chromium.org,alph@chromium.org,adamk@chromium.org
==========

to

==========
[inspector] added test infrastructure and test for es6 modules

Test just checks that all basic features are working correctly with modules.

BUG=v8:1569
R=dgozman@chromium.org,alph@chromium.org,adamk@chromium.org
==========

[kozy, 2017-01-30 21:24:03]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2017-01-30 21:24:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2663743002/#ps20001
(title: "addressed comments")

[commit-bot: I haz the power, 2017-01-30 21:24: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, 2017-01-30 21:42:28]

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

[commit-bot: I haz the power, 2017-01-30 21:42:29]

Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/21769)
  v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/21858)
  v8_win_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng_triggered/bui...)

[kozy, 2017-01-30 23:22:13]

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

[commit-bot: I haz the power, 2017-01-30 23:22:16]

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

[kozy, 2017-01-30 23:25:11]

Adam, please take another look.

https://codereview.chromium.org/2663743002/diff/40001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2663743002/diff/40001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:1888: if (is_module_scope())
AsModuleScope()->AllocateModuleVariables();
Adam, could you please take a look on these lines?

Previous version of CL doesn't pass tests with crash, link to try bot with crash
stack trace [1].
I think that we should do AllocationModuleVariables here in the same way as we
do in DeclarationScope::AllocateVariables function for the same reason.. WDYT?

[1]
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.v8%2Fv8_linux_...

[adamk, 2017-01-30 23:34:35]

https://codereview.chromium.org/2663743002/diff/40001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2663743002/diff/40001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:1888: if (is_module_scope())
AsModuleScope()->AllocateModuleVariables();
On 2017/01/30 23:25:11, kozy wrote:
> Adam, could you please take a look on these lines?
> 
> Previous version of CL doesn't pass tests with crash, link to try bot with
crash
> stack trace [1].
> I think that we should do AllocationModuleVariables here in the same way as we
> do in DeclarationScope::AllocateVariables function for the same reason.. WDYT?
> 
> [1]
>
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.v8%2Fv8_linux_...

Can you guard this with "info != nullptr" as well? It looks like only in that
case do we call ResolveTo(), which is where this goes badly.

[kozy, 2017-01-30 23:40:40]

https://codereview.chromium.org/2663743002/diff/40001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2663743002/diff/40001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:1888: if (is_module_scope())
AsModuleScope()->AllocateModuleVariables();
On 2017/01/30 23:34:35, adamk wrote:
> On 2017/01/30 23:25:11, kozy wrote:
> > Adam, could you please take a look on these lines?
> > 
> > Previous version of CL doesn't pass tests with crash, link to try bot with
> crash
> > stack trace [1].
> > I think that we should do AllocationModuleVariables here in the same way as
we
> > do in DeclarationScope::AllocateVariables function for the same reason..
WDYT?
> > 
> > [1]
> >
>
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.v8%2Fv8_linux_...
> 
> Can you guard this with "info != nullptr" as well? It looks like only in that
> case do we call ResolveTo(), which is where this goes badly.

Done.

[kozy, 2017-01-30 23:40:51]

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

[commit-bot: I haz the power, 2017-01-30 23:40:55]

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

[adamk, 2017-01-30 23:44:42]

lgtm

[commit-bot: I haz the power, 2017-01-31 00:08:34]

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

[commit-bot: I haz the power, 2017-01-31 00:08:34]

Dry run: This issue passed the CQ dry run.

[kozy, 2017-01-31 00:17:57]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2017-01-31 00:17:57]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2663743002/#ps60001
(title: "addressed comment")

[commit-bot: I haz the power, 2017-01-31 00:18:04]

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, 2017-01-31 00:19:43]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485821877094090,
"parent_rev": "3901e247b39e49b62c45904e816cb8ede2f32338", "commit_rev":
"d6db11fd181061f9ffce4431af5afec0dba2c82c"}

[commit-bot: I haz the power, 2017-01-31 00:19:48]

Description was changed from

==========
[inspector] added test infrastructure and test for es6 modules

Test just checks that all basic features are working correctly with modules.

BUG=v8:1569
R=dgozman@chromium.org,alph@chromium.org,adamk@chromium.org
==========

to

==========
[inspector] added test infrastructure and test for es6 modules

Test just checks that all basic features are working correctly with modules.

BUG=v8:1569
R=dgozman@chromium.org,alph@chromium.org,adamk@chromium.org

Review-Url: https://codereview.chromium.org/2663743002
Cr-Commit-Position: refs/heads/master@{#42796}
Committed:
https://chromium.googlesource.com/v8/v8/+/d6db11fd181061f9ffce4431af5afec0dba...
==========

[commit-bot: I haz the power, 2017-01-31 00:19:49]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/d6db11fd181061f9ffce4431af5afec0dba...