[serializer] API to re-use global proxy in v8::Context::FromSnapshot.

[serializer] API to re-use global proxy in v8::Context::FromSnapshot.

R=jochen@chromium.org, peria@chromium.org
BUG=chromium:617892
Committed: https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
Cr-Commit-Position: refs/heads/master@{#41668}

[peria, 2016-12-13 08:14:32]

Thank you for this fix!
It works well.

LGTM

[Yang, 2016-12-13 09:30:20]

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

[commit-bot: I haz the power, 2016-12-13 09:30:31]

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-13 09:59:47]

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

[commit-bot: I haz the power, 2016-12-13 09:59:48]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-12-13 11:17:39]

lgtm

[Yang, 2016-12-13 11:22:41]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2016-12-13 11:22:47]

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-13 11:24:25]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481628161529270, "parent_rev":
"7e293f0c2efb424b76c11c75e9ec580b6e8ee457", "commit_rev":
"eb597f1b58e6fc50d37768eab8d5ca011b57f853"}

[commit-bot: I haz the power, 2016-12-13 11:24:32]

Description was changed from

==========
[serializer] API to re-use global proxy in v8::Context::FromSnapshot.



R=jochen@chromium.org, peria@chromium.org
BUG=chromium:617892
==========

to

==========
[serializer] API to re-use global proxy in v8::Context::FromSnapshot.



R=jochen@chromium.org, peria@chromium.org
BUG=chromium:617892

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

[commit-bot: I haz the power, 2016-12-13 11:24:33]

Committed patchset #1 (id:1)

[peria, 2016-12-13 11:25:01]

On 2016/12/13 08:14:32, peria wrote:
> Thank you for this fix!
> It works well.
> 
> LGTM

still lgtm, but it did not work correctly in second try. :(


#
# Fatal error in ../../v8/src/factory.cc, line 2234
# Check failed: map->instance_size() == old_map->instance_size().
#

==== C stack trace ===============================

v8::internal::Factory::ReinitializeJSGlobalProxy(v8::internal::Handle<v8::internal::JSGlobalProxy>,
v8::internal::Handle<v8::internal::JSFun
ction>)+0x1fe) [0x1cd0cbe]
v8::internal::Genesis::HookUpGlobalProxy(v8::internal::Handle<v8::internal::JSGlobalProxy>)+0x6d)
[0x16ea21d]
v8::internal::Genesis::Genesis(v8::internal::Isolate*,
v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>,
v8::Local<v8::ObjectTemplate>
, unsigned long, v8::internal::GlobalContextType)+0x4fc) [0x1703c9c]
v8::internal::Bootstrapper::CreateEnvironment(v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>,
v8::Local<v8::ObjectTemplate>, v8::Ext
ensionConfiguration*, unsigned long, v8::internal::GlobalContextType)+0x8b)
[0x16e714b]
v8::NewContext(v8::Isolate*, v8::ExtensionConfiguration*,
v8::MaybeLocal<v8::ObjectTemplate>, v8::MaybeLocal<v8::Value>, unsigned
long)+0x16
6) [0x1614266]
v8::Context::FromSnapshot(v8::Isolate*, unsigned long,
v8::ExtensionConfiguration*, v8::MaybeLocal<v8::Value>)+0x84) [0x1614894]

[commit-bot: I haz the power, 2016-12-13 11:25:09]

Description was changed from

==========
[serializer] API to re-use global proxy in v8::Context::FromSnapshot.



R=jochen@chromium.org, peria@chromium.org
BUG=chromium:617892

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

to

==========
[serializer] API to re-use global proxy in v8::Context::FromSnapshot.

R=jochen@chromium.org, peria@chromium.org
BUG=chromium:617892

Committed: https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
Cr-Commit-Position: refs/heads/master@{#41668}
==========

[commit-bot: I haz the power, 2016-12-13 11:25:10]

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

[Yang, 2016-12-13 11:27:17]

On 2016/12/13 11:25:10, commit-bot: I haz the power wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
> Cr-Commit-Position: refs/heads/master@{#41668}

Thanks. I'll investigate that some more. Do you have a repro?

[Yang, 2016-12-13 11:29:21]

On 2016/12/13 11:27:17, Yang wrote:
> On 2016/12/13 11:25:10, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
> > Cr-Commit-Position: refs/heads/master@{#41668}
> 
> Thanks. I'll investigate that some more. Do you have a repro?

I think it only works if you reuse the global object on a context from the same
snapshot.

[peria, 2016-12-13 11:44:16]

On 2016/12/13 11:29:21, Yang wrote:
> On 2016/12/13 11:27:17, Yang wrote:
> > On 2016/12/13 11:25:10, commit-bot: I haz the power wrote:
> > > Patchset 1 (id:??) landed as
> > > https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
> > > Cr-Commit-Position: refs/heads/master@{#41668}
> > 
> > Thanks. I'll investigate that some more. Do you have a repro?
> 
> I think it only works if you reuse the global object on a context from the
same
> snapshot.

Update the wip CL (PS6) https://codereview.chromium.org/2309163002/
For the first call, must we use the global object in the snapshot?
If yes, how can we do it?

[Yang, 2016-12-13 11:50:00]

On 2016/12/13 11:44:16, peria wrote:
> On 2016/12/13 11:29:21, Yang wrote:
> > On 2016/12/13 11:27:17, Yang wrote:
> > > On 2016/12/13 11:25:10, commit-bot: I haz the power wrote:
> > > > Patchset 1 (id:??) landed as
> > > > https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
> > > > Cr-Commit-Position: refs/heads/master@{#41668}
> > > 
> > > Thanks. I'll investigate that some more. Do you have a repro?
> > 
> > I think it only works if you reuse the global object on a context from the
> same
> > snapshot.
> 
> Update the wip CL (PS6) https://codereview.chromium.org/2309163002/
> For the first call, must we use the global object in the snapshot?
> If yes, how can we do it?

For the first call, call FromSnapshot with just the index, and provide no global
object. For the second call, use the global object from the first call. See test
case.

[Yang, 2016-12-13 11:51:10]

On 2016/12/13 11:50:00, Yang wrote:
> On 2016/12/13 11:44:16, peria wrote:
> > On 2016/12/13 11:29:21, Yang wrote:
> > > On 2016/12/13 11:27:17, Yang wrote:
> > > > On 2016/12/13 11:25:10, commit-bot: I haz the power wrote:
> > > > > Patchset 1 (id:??) landed as
> > > > > https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
> > > > > Cr-Commit-Position: refs/heads/master@{#41668}
> > > > 
> > > > Thanks. I'll investigate that some more. Do you have a repro?
> > > 
> > > I think it only works if you reuse the global object on a context from the
> > same
> > > snapshot.
> > 
> > Update the wip CL (PS6) https://codereview.chromium.org/2309163002/
> > For the first call, must we use the global object in the snapshot?
> > If yes, how can we do it?
> 
> For the first call, call FromSnapshot with just the index, and provide no
global
> object. For the second call, use the global object from the first call. See
test
> case.

This is similar to the restriction we have for Context::New that requires the
global object in the second call to be from the same object template as the
first call.

[peria, 2016-12-13 11:59:57]

On 2016/12/13 11:51:10, Yang wrote:
> On 2016/12/13 11:50:00, Yang wrote:
> > On 2016/12/13 11:44:16, peria wrote:
> > > On 2016/12/13 11:29:21, Yang wrote:
> > > > On 2016/12/13 11:27:17, Yang wrote:
> > > > > On 2016/12/13 11:25:10, commit-bot: I haz the power wrote:
> > > > > > Patchset 1 (id:??) landed as
> > > > > > https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
> > > > > > Cr-Commit-Position: refs/heads/master@{#41668}
> > > > > 
> > > > > Thanks. I'll investigate that some more. Do you have a repro?
> > > > 
> > > > I think it only works if you reuse the global object on a context from
the
> > > same
> > > > snapshot.
> > > 
> > > Update the wip CL (PS6) https://codereview.chromium.org/2309163002/
> > > For the first call, must we use the global object in the snapshot?
> > > If yes, how can we do it?
> > 
> > For the first call, call FromSnapshot with just the index, and provide no
> global
> > object. For the second call, use the global object from the first call. See
> test
> > case.
> 
> This is similar to the restriction we have for Context::New that requires the
> global object in the second call to be from the same object template as the
> first call.

Ah, right.  It works.  Thank you very much!

[peria, 2016-12-13 13:12:12]

On 2016/12/13 11:59:57, peria wrote:
> On 2016/12/13 11:51:10, Yang wrote:
> > On 2016/12/13 11:50:00, Yang wrote:
> > > On 2016/12/13 11:44:16, peria wrote:
> > > > On 2016/12/13 11:29:21, Yang wrote:
> > > > > On 2016/12/13 11:27:17, Yang wrote:
> > > > > > On 2016/12/13 11:25:10, commit-bot: I haz the power wrote:
> > > > > > > Patchset 1 (id:??) landed as
> > > > > > > https://crrev.com/449829b85ccb8e76ae5b2aafed65606e275a9bd1
> > > > > > > Cr-Commit-Position: refs/heads/master@{#41668}
> > > > > > 
> > > > > > Thanks. I'll investigate that some more. Do you have a repro?
> > > > > 
> > > > > I think it only works if you reuse the global object on a context from
> the
> > > > same
> > > > > snapshot.
> > > > 
> > > > Update the wip CL (PS6) https://codereview.chromium.org/2309163002/
> > > > For the first call, must we use the global object in the snapshot?
> > > > If yes, how can we do it?
> > > 
> > > For the first call, call FromSnapshot with just the index, and provide no
> > global
> > > object. For the second call, use the global object from the first call.
See
> > test
> > > case.
> > 
> > This is similar to the restriction we have for Context::New that requires
the
> > global object in the second call to be from the same object template as the
> > first call.
> 
> Ah, right.  It works.  Thank you very much!

Looking in detail, it didn't work and failed in the CHECK in #13.
In succeeded cases, it ran with Context::New() as a fall back.