Move ApiLocalScope out of class ApiState into class Thread so that the API local handles and zone e…

Move ApiLocalScope out of class ApiState into class Thread so that the API local handles and zone etc. are thread specific instead of being Isolate specific.

R=zra@google.com

Committed: https://github.com/dart-lang/sdk/commit/01b69ebabc3d324dd1880563cd1983fd6428d5a2

[siva, 2015-11-25 00:41:16]

asiva@google.com changed reviewers:
+ iposva@google.com, zra@google.com

[zra, 2015-11-25 17:36:56]

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:375: Isolate* isolate = thread->isolate();
Just noting that isolate is only used for the ASSERT here.

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:1534: CHECK_API_SCOPE(thread);
Sometimes this is called "thread" and sometimes "T". It might be nice to have
the same name everywhere.

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:2223: Isolate* I = T->isolate();
In a few places it looks like these isolates are only used to pass to
SpaceForExternal. Should SpaceForExternal take a Thread instead?

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:2673: args));
Indentation looks a bit weird to me here.

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/thread_regis...
File runtime/vm/thread_registry.h (right):

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/thread_regis...
runtime/vm/thread_registry.h:83: // for these specific cases, we should also
determine if we should allow
cases. We

[siva, 2015-11-25 18:25:08]

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:375: Isolate* isolate = thread->isolate();
On 2015/11/25 17:36:56, zra wrote:
> Just noting that isolate is only used for the ASSERT here.

Made it ASSERT(thread->isolate() != NULL);

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:1534: CHECK_API_SCOPE(thread);
On 2015/11/25 17:36:56, zra wrote:
> Sometimes this is called "thread" and sometimes "T". It might be nice to have
> the same name everywhere.

I have changed all the CHECK_API_SCOPE spots, maybe we should a sweep through
this file for other uses of thread (will hold that off for another CL).

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:2223: Isolate* I = T->isolate();
On 2015/11/25 17:36:56, zra wrote:
> In a few places it looks like these isolates are only used to pass to
> SpaceForExternal. Should SpaceForExternal take a Thread instead?

Done.

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:2673: args));
On 2015/11/25 17:36:56, zra wrote:
> Indentation looks a bit weird to me here.

Done.

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/thread_regis...
File runtime/vm/thread_registry.h (right):

https://codereview.chromium.org/1473403003/diff/20001/runtime/vm/thread_regis...
runtime/vm/thread_registry.h:83: // for these specific cases, we should also
determine if we should allow
On 2015/11/25 17:36:56, zra wrote:
> cases. We

Done.

[zra, 2015-11-25 18:28:28]

lgtm

[siva, 2015-11-25 19:07:25]

Description was changed from

==========
Move ApiLocalScope out of class ApiState into class Thread so that the API local
handles and zone etc. are thread specific instead of being Isolate specific.
==========

to

==========
Move ApiLocalScope out of class ApiState into class Thread so that the API local
handles and zone etc. are thread specific instead of being Isolate specific.

R=zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/01b69ebabc3d324dd1880563cd1983fd6428d5a2
==========

[siva, 2015-11-25 19:07:26]

Committed patchset #4 (id:60001) manually as
01b69ebabc3d324dd1880563cd1983fd6428d5a2 (presubmit successful).

[Ivan Posva, 2015-11-25 20:19:51]

DBC -ip

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:1537: I->object_store()->sticky_error());
Single line?

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_impl.h
File runtime/vm/dart_api_impl.h (right):

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.h:62: Isolate* tmpI = (tmpT->isolate());               
                         \
Parens?

[siva, 2015-11-25 22:45:12]

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:1537: I->object_store()->sticky_error());
On 2015/11/25 20:19:51, Ivan Posva wrote:
> Single line?

Done.

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_impl.h
File runtime/vm/dart_api_impl.h (right):

https://codereview.chromium.org/1473403003/diff/40001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.h:62: Isolate* tmpI = (tmpT->isolate());               
                         \
On 2015/11/25 20:19:51, Ivan Posva wrote:
> Parens?

Done.