Move SimulatorHelper into V8 out of profiler clients.

Move SimulatorHelper into V8 out of profiler clients.

Clients should not know about the simulator.

BUG=v8:4789
Committed: https://crrev.com/b837241150409db71b07bd989872a494a31ca673
Cr-Commit-Position: refs/heads/master@{#37617}

[alph, 2016-07-07 16:59:50]

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

[commit-bot: I haz the power, 2016-07-07 16:59:54]

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-07-07 17:24:16]

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

[commit-bot: I haz the power, 2016-07-07 17:24:17]

Dry run: This issue passed the CQ dry run.

[alph, 2016-07-07 17:24:50]

alph@chromium.org changed reviewers:
+ jochen@chromium.org, lpy@chromium.org, yangguo@chromium.org

[lpy, 2016-07-07 17:57:57]

LGTM with nits.

https://codereview.chromium.org/2128613004/diff/1/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/log.cc#newcode649
src/log.cc:649: Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate());
Since sampler has a method called isolate(), should we avoid using `isolate` as
a temporary variable?

https://codereview.chromium.org/2128613004/diff/1/src/profiler/cpu-profiler.cc
File src/profiler/cpu-profiler.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/profiler/cpu-profiler.c...
src/profiler/cpu-profiler.cc:29: Isolate* isolate =
reinterpret_cast<Isolate*>(this->isolate());
Same here.

[alph, 2016-07-07 18:05:29]

Thanks

https://codereview.chromium.org/2128613004/diff/1/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/log.cc#newcode649
src/log.cc:649: Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate());
On 2016/07/07 17:57:56, lpy wrote:
> Since sampler has a method called isolate(), should we avoid using `isolate`
as
> a temporary variable?

What's wrong with it? I'm not aware of a policy/rule that prohibits using the
same name for a local variable.

[Yang, 2016-07-08 08:40:47]

LGTM with nits

https://codereview.chromium.org/2128613004/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/api.cc#newcode7585
src/api.cc:7585: return;
brackets around this please :)

https://codereview.chromium.org/2128613004/diff/1/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/log.cc#newcode649
src/log.cc:649: Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate());
On 2016/07/07 18:05:29, alph wrote:
> On 2016/07/07 17:57:56, lpy wrote:
> > Since sampler has a method called isolate(), should we avoid using `isolate`
> as
> > a temporary variable?
> 
> What's wrong with it? I'm not aware of a policy/rule that prohibits using the
> same name for a local variable.

I think this is fine. We do this elsewhere too.

https://codereview.chromium.org/2128613004/diff/1/src/profiler/tick-sample.cc
File src/profiler/tick-sample.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/profiler/tick-sample.cc...
src/profiler/tick-sample.cc:202: return false;  // Can't collect stack.
I'd like to keep these brackets.

[alph, 2016-07-08 21:31:17]

The CQ bit was checked by alph@chromium.org

[alph, 2016-07-08 21:31:17]

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

[commit-bot: I haz the power, 2016-07-08 21:31:22]

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

[alph, 2016-07-08 21:47:42]

https://codereview.chromium.org/2128613004/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/api.cc#newcode7585
src/api.cc:7585: return;
On 2016/07/08 08:40:46, Yang wrote:
> brackets around this please :)

Done.

https://codereview.chromium.org/2128613004/diff/1/src/profiler/tick-sample.cc
File src/profiler/tick-sample.cc (right):

https://codereview.chromium.org/2128613004/diff/1/src/profiler/tick-sample.cc...
src/profiler/tick-sample.cc:202: return false;  // Can't collect stack.
On 2016/07/08 08:40:47, Yang wrote:
> I'd like to keep these brackets.

Done.

[alph, 2016-07-08 22:21:29]

The CQ bit was unchecked by alph@chromium.org

[alph, 2016-07-08 22:22:13]

The CQ bit was checked by alph@chromium.org

[commit-bot: I haz the power, 2016-07-08 22:22:22]

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-07-08 23:52:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-08 23:53:01]

Description was changed from

==========
Move SimulatorHelper into V8 out of profiler clients.

Clients should not know about the simulator.

BUG=v8:4789
==========

to

==========
Move SimulatorHelper into V8 out of profiler clients.

Clients should not know about the simulator.

BUG=v8:4789

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

[commit-bot: I haz the power, 2016-07-08 23:53:02]

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

[Michael Achenbach, 2016-07-11 07:40:08]

Note: Collective revert: https://codereview.chromium.org/2138643003/