Add an API to create a detached global object

Add an API to create a detached global object

Such an object can be used to later create a context from it. It has to
have access checks with handlers enabled, as it cannot be accessed
otherwise.

BUG=chromium:618305
R=verwaest@chromium.org

Committed: https://crrev.com/0058f82e2c47b8e12428cbb37093d10eaa4a8064
Cr-Commit-Position: refs/heads/master@{#37594}

[jochen (gone - plz use gerrit), 2016-06-28 14:27:00]

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

https://codereview.chromium.org/2107673003/diff/1/src/api-natives.cc#newcode585
src/api-natives.cc:585: prototype = isolate->factory()->null_value();
we can't create a prototype object without the object function which in turn we
can't create without a context. just use null.

https://codereview.chromium.org/2107673003/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2107673003/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:3981: 
The regular genesis path would also invoke ConfigureGlobalObjects() - that would
copy over any properties defined for the proxy from the template to the object.
The assumption here is that no such properties exist.

https://codereview.chromium.org/2107673003/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:3983: SetObjectPrototype(global_proxy,
factory()->null_value());
regular DetachGlobal would also delete the constructor, but then we can't get to
the AccessCheckInfo anymore

[dcheng, 2016-06-28 15:23:52]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2016-06-28 15:23:53]

https://codereview.chromium.org/2107673003/diff/20001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/2107673003/diff/20001/include/v8.h#newcode7160
include/v8.h:7160: Local<ObjectTemplate> global_template);
Will there be an alternate way of creating a context-less object for Location as
well? Or is it OK to just use this one, despite the fact that "Global" is in the
name?

[jochen (gone - plz use gerrit), 2016-06-28 15:26:02]

https://codereview.chromium.org/2107673003/diff/20001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/2107673003/diff/20001/include/v8.h#newcode7160
include/v8.h:7160: Local<ObjectTemplate> global_template);
On 2016/06/28 at 15:23:53, dcheng wrote:
> Will there be an alternate way of creating a context-less object for Location
as well? Or is it OK to just use this one, despite the fact that "Global" is in
the name?

there will be a separate api for location

[Toon Verwaest, 2016-06-30 14:46:17]

Mostly looking good

https://codereview.chromium.org/2107673003/diff/20001/src/api-natives.cc
File src/api-natives.cc (right):

https://codereview.chromium.org/2107673003/diff/20001/src/api-natives.cc#newc...
src/api-natives.cc:549: Handle<JSFunction> ApiNatives::CreateApiFunctionWithMap(
I'd prefer it if you wouldn't reuse this function. By passing in the map you'd
expect any combination to work, which won't.

Rather just manually create the shared function info, the function and its
initial-map with JS_GLOBAL_PROXY_TYPE in straightline simple code that's obvious
to read. Then you don't need the context_or_undefined hack below for example.

[jochen (gone - plz use gerrit), 2016-07-01 08:49:48]

ptal

[Toon Verwaest, 2016-07-06 15:34:35]

looks good, but the api function should probably be something like RemoteGlobal
which gets in a MaybeHandle<Object> which is possibly an existing global proxy
that becomes remote. That way it's symmetrical with Context::New.

https://codereview.chromium.org/2107673003/diff/40001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2107673003/diff/40001/src/bootstrapper.cc#new...
src/bootstrapper.cc:3940: Genesis::Genesis(Isolate* isolate,
Don't you want to do the same as above and also pass in a
MaybeHandle<JSGlobalProxy> maybe_global_proxy that indicates whether an existing
IF is going OOP, or a new OOPIF is created?

[jochen (gone - plz use gerrit), 2016-07-07 09:01:25]

changed the API as discussed + new tests

[Toon Verwaest, 2016-07-07 11:15:26]

lgtm with one last comment

https://codereview.chromium.org/2107673003/diff/80001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2107673003/diff/80001/src/bootstrapper.cc#new...
src/bootstrapper.cc:4037: isolate->bootstrapper()->DetachGlobal(env);
Why do you manually detach here but not in regular Genesis above? Why not just
assume/dcheck you get a detached global in?

[jochen (gone - plz use gerrit), 2016-07-07 19:09:43]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-07-07 19:09:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/2107673003/#ps100001
(title: "updates")

[commit-bot: I haz the power, 2016-07-07 19:09:55]

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 19:37:10]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-07 19:37:15]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 19:38:16]

Description was changed from

==========
Add an API to create a detached global object

Such an object can be used to later create a context from it. It has to
have access checks with handlers enabled, as it cannot be accessed
otherwise.

BUG=chromium:618305
R=verwaest@chromium.org
==========

to

==========
Add an API to create a detached global object

Such an object can be used to later create a context from it. It has to
have access checks with handlers enabled, as it cannot be accessed
otherwise.

BUG=chromium:618305
R=verwaest@chromium.org

Committed: https://crrev.com/0058f82e2c47b8e12428cbb37093d10eaa4a8064
Cr-Commit-Position: refs/heads/master@{#37594}
==========

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

Patchset 6 (id:??) landed as
https://crrev.com/0058f82e2c47b8e12428cbb37093d10eaa4a8064
Cr-Commit-Position: refs/heads/master@{#37594}

[dcheng, 2016-07-11 07:07:45]

https://codereview.chromium.org/2107673003/diff/100001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/2107673003/diff/100001/include/v8.h#newcode7184
include/v8.h:7184: * It is possible, however, the create a new context from the
global object
I think there might be a typo in this comment?