ScriptController: Expose methods to compile a script and execute a compiled script.

ScriptController: Expose methods to compile a script and execute a compiled script.

Simple unit tests included.

BUG=591558
Committed: https://crrev.com/1e1d645d1cf402a63805f2106452d8d689130f1b
Cr-Commit-Position: refs/heads/master@{#439974}

[jbroman, 2016-12-09 16:31:57]

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

[commit-bot: I haz the power, 2016-12-09 16:32:24]

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-12-09 18:24:00]

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

[commit-bot: I haz the power, 2016-12-09 18:24:01]

Dry run: This issue passed the CQ dry run.

[jbroman, 2016-12-09 18:25:35]

jbroman@chromium.org changed reviewers:
+ haraken@chromium.org

[jbroman, 2016-12-09 18:25:36]

This pushes the separation in V8ScriptRunner through ScriptController.

[haraken, 2016-12-12 00:53:57]

LGTM

https://codereview.chromium.org/2564903002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp (right):

https://codereview.chromium.org/2564903002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:192:
compiledScript.startPosition()));

Shall we introduce two trace events: "CompileScript" and "RunScript"?

- compileScript uses "CompileScript"
- executeScript uses "RunScript"
- executeScriptAndReturnValue uses "CompileScript" and "RunScript"

https://codereview.chromium.org/2564903002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:209:
InspectorUpdateCountersEvent::data());

I don't fully understand this trace event. Is it expected to be called on both
compileScript and executeScript? Before this CL, it was called only once for
each compile & execution step.

[jbroman, 2016-12-13 15:53:21]

+cc pfeldman for questions about devtools.timeline trace events

https://codereview.chromium.org/2564903002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp (right):

https://codereview.chromium.org/2564903002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:192:
compiledScript.startPosition()));
On 2016/12/12 at 00:53:57, haraken wrote:
> Shall we introduce two trace events: "CompileScript" and "RunScript"?
> 
> - compileScript uses "CompileScript"
> - executeScript uses "RunScript"
> - executeScriptAndReturnValue uses "CompileScript" and "RunScript"

I'm not opposed in principle, but these string constants are written into the
devtools timeline model code. It does already have a compile event, triggering
off "v8.compile" (which, along with "v8.run", already exists in V8ScriptRunner).
I admit to not being entirely sure why we have this separate event given that
v8.compile and v8.run both exist in the devtools.timeline category.

https://codereview.chromium.org/2564903002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:209:
InspectorUpdateCountersEvent::data());
On 2016/12/12 at 00:53:57, haraken wrote:
> I don't fully understand this trace event. Is it expected to be called on both
compileScript and executeScript? Before this CL, it was called only once for
each compile & execution step.

From what I can tell, it makes a note of the heap size, JS listener count,
document and node count. Since the script compile may increase the heap size
(and execution certainly can), it seems reasonable to update these in the
devtools timeline at both times.

[jbroman, 2016-12-16 14:53:46]

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

[jbroman, 2016-12-16 14:53:46]

moving pfeldman to reviewers

[pfeldman, 2016-12-19 21:58:10]

Wrong bug number?

[pfeldman, 2016-12-19 22:20:20]

lgtm

[jbroman, 2016-12-20 17:53:51]

Description was changed from

==========
ScriptController: Expose methods to compile a script and execute a compiled
script.

Simple unit tests included.

BUG=591588
==========

to

==========
ScriptController: Expose methods to compile a script and execute a compiled
script.

Simple unit tests included.

BUG=591558
==========

[jbroman, 2016-12-20 17:53:58]

The CQ bit was checked by jbroman@chromium.org

[jbroman, 2016-12-20 17:53:58]

Fixed bug number.

[jbroman, 2016-12-20 17:54:04]

The CQ bit was unchecked by jbroman@chromium.org

[jbroman, 2016-12-20 17:54:33]

haraken, verdict on trace events?

[haraken, 2016-12-20 23:50:56]

LGTM

[jbroman, 2016-12-21 00:10:30]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2016-12-21 00:11:07]

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-12-21 01:58:27]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1482279030723360, "parent_rev":
"e3b1347f19e848b894cd435c8afee425ed72b15d", "commit_rev":
"77d55856c0d79035b33faeb7524bf7317a9967f6"}

[commit-bot: I haz the power, 2016-12-21 01:58:58]

Description was changed from

==========
ScriptController: Expose methods to compile a script and execute a compiled
script.

Simple unit tests included.

BUG=591558
==========

to

==========
ScriptController: Expose methods to compile a script and execute a compiled
script.

Simple unit tests included.

BUG=591558

Review-Url: https://codereview.chromium.org/2564903002
==========

[commit-bot: I haz the power, 2016-12-21 01:58:59]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-21 02:01:20]

Description was changed from

==========
ScriptController: Expose methods to compile a script and execute a compiled
script.

Simple unit tests included.

BUG=591558

Review-Url: https://codereview.chromium.org/2564903002
==========

to

==========
ScriptController: Expose methods to compile a script and execute a compiled
script.

Simple unit tests included.

BUG=591558

Committed: https://crrev.com/1e1d645d1cf402a63805f2106452d8d689130f1b
Cr-Commit-Position: refs/heads/master@{#439974}
==========

[commit-bot: I haz the power, 2016-12-21 02:01:21]

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