Introduce a Gin class instead of using global functions to control gin

Introduce a Gin class instead of using global functions to control gin

The Gin class holds and controls a v8::Isolate. The isolate is not
entered by default, i.e. before you can use gin for a given Gin
instance, you need to enter the isolate first, e.g. by using a
v8::Isolate::Scope.

This has the advantage that we don't rely on the deprecate default
isolate, and also support having multiple isolates in one process.

BUG=317398
R=abarth@chromium.org
TEST=gin_unittests and mojo_js_bindings_unittests pass

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=236029

[abarth-chromium, 2013-11-19 15:49:21]

LGTM

https://codereview.chromium.org/76353002/diff/1/gin/gin.cc
File gin/gin.cc (right):

https://codereview.chromium.org/76353002/diff/1/gin/gin.cc#newcode20
gin/gin.cc:20: base::RandBytes(buffer, amount);
Thanks!

https://codereview.chromium.org/76353002/diff/1/gin/runner_unittest.cc
File gin/runner_unittest.cc (right):

https://codereview.chromium.org/76353002/diff/1/gin/runner_unittest.cc#newcode23
gin/runner_unittest.cc:23: scoped_ptr<gin::Gin> instance(new gin::Gin);
Why not allocate on the stack?

https://codereview.chromium.org/76353002/diff/1/gin/shell/gin_main.cc
File gin/shell/gin_main.cc (right):

https://codereview.chromium.org/76353002/diff/1/gin/shell/gin_main.cc#newcode64
gin/shell/gin_main.cc:64: scoped_ptr<gin::Gin> instance(new gin::Gin());
Why not allocate on the stack?

https://codereview.chromium.org/76353002/diff/1/gin/test/file_runner.cc
File gin/test/file_runner.cc (right):

https://codereview.chromium.org/76353002/diff/1/gin/test/file_runner.cc#newco...
gin/test/file_runner.cc:59: v8::HandleScope handle_scope(instance->isolate());
We're still in the Runner's scope.  I'm not sure this part is needed.  Maybe we
should put { } earlier so that RunUntilIdle happens outside the runner's scope?

https://codereview.chromium.org/76353002/diff/1/mojo/apps/js/main.cc
File mojo/apps/js/main.cc (right):

https://codereview.chromium.org/76353002/diff/1/mojo/apps/js/main.cc#newcode21
mojo/apps/js/main.cc:21: gin::Gin* instance = new gin::Gin;
Why not allocate on the stack?

[jochen (gone - plz use gerrit), 2013-11-19 15:57:23]

https://codereview.chromium.org/76353002/diff/1/gin/runner_unittest.cc
File gin/runner_unittest.cc (right):

https://codereview.chromium.org/76353002/diff/1/gin/runner_unittest.cc#newcode23
gin/runner_unittest.cc:23: scoped_ptr<gin::Gin> instance(new gin::Gin);
On 2013/11/19 15:49:21, abarth wrote:
> Why not allocate on the stack?

Done.

https://codereview.chromium.org/76353002/diff/1/gin/shell/gin_main.cc
File gin/shell/gin_main.cc (right):

https://codereview.chromium.org/76353002/diff/1/gin/shell/gin_main.cc#newcode64
gin/shell/gin_main.cc:64: scoped_ptr<gin::Gin> instance(new gin::Gin());
On 2013/11/19 15:49:21, abarth wrote:
> Why not allocate on the stack?

Done.

https://codereview.chromium.org/76353002/diff/1/gin/test/file_runner.cc
File gin/test/file_runner.cc (right):

https://codereview.chromium.org/76353002/diff/1/gin/test/file_runner.cc#newco...
gin/test/file_runner.cc:59: v8::HandleScope handle_scope(instance->isolate());
On 2013/11/19 15:49:21, abarth wrote:
> We're still in the Runner's scope.  I'm not sure this part is needed.  Maybe
we
> should put { } earlier so that RunUntilIdle happens outside the runner's
scope?

Right, I added this before I fixed Runner::Scope to have an Isolate::Scope as
member

https://codereview.chromium.org/76353002/diff/1/mojo/apps/js/main.cc
File mojo/apps/js/main.cc (right):

https://codereview.chromium.org/76353002/diff/1/mojo/apps/js/main.cc#newcode21
mojo/apps/js/main.cc:21: gin::Gin* instance = new gin::Gin;
On 2013/11/19 15:49:21, abarth wrote:
> Why not allocate on the stack?

Done.

[commit-bot: I haz the power, 2013-11-19 15:57:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/76353002/10017

[commit-bot: I haz the power, 2013-11-19 16:13:30]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[jochen (gone - plz use gerrit), 2013-11-19 16:14:47]

Aaron, can you rubberstamp plz?

[abarth-chromium, 2013-11-19 16:48:57]

https://codereview.chromium.org/76353002/diff/10017/gin/test/file_runner.cc
File gin/test/file_runner.cc (right):

https://codereview.chromium.org/76353002/diff/10017/gin/test/file_runner.cc#n...
gin/test/file_runner.cc:50: scoped_ptr<gin::Gin> instance(new gin::Gin);
Why not on the stack?

[abarth-chromium, 2013-11-19 16:56:12]

On 2013/11/19 16:14:47, jochen wrote:
> Aaron, can you rubberstamp plz?

I've just been added to the OWNERS file, so presubmit should be happy now.

[commit-bot: I haz the power, 2013-11-19 16:57:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/76353002/10017

[commit-bot: I haz the power, 2013-11-19 17:03:36]

Failed to trigger a try job on win_x64_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-11-19 17:04:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/76353002/240002

[commit-bot: I haz the power, 2013-11-19 19:25:16]

Change committed as 236029