Keep "static" objects per-context rather than per isolate.

Keep "static" objects per-context rather than per isolate.

Every time we initialize a new v8::Context, we make a new set of
pre-existing native objects, and overwrite the v8::Global
handles to those in the previous set. Thus, we may be
theoretically releasing some v8::Globals early.

R=jochen@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/7d4c74d1b96d82681d49519c129262a1568e87d4

[Tom Sepez, 2015-11-06 23:19:12]

Description was changed from

==========
Keep "static" objects per-context rather than per isolate.

We may be theoretically releasing some v8::Globals early.
==========

to

==========
Keep "static" objects per-context rather than per isolate.

Every time we initialize a new context, we make a new set of
pre-existing naitve objects, and overwrite the v8::Global
handles to those in the previous set. Thus, we may be
theoretically releasing some v8::Globals early.
==========

[Tom Sepez, 2015-11-06 23:19:12]

tsepez@chromium.org changed reviewers:
+ jochen@chromium.org

[Tom Sepez, 2015-11-06 23:19:30]

Description was changed from

==========
Keep "static" objects per-context rather than per isolate.

Every time we initialize a new context, we make a new set of
pre-existing naitve objects, and overwrite the v8::Global
handles to those in the previous set. Thus, we may be
theoretically releasing some v8::Globals early.
==========

to

==========
Keep "static" objects per-context rather than per isolate.

Every time we initialize a new v8::Context, we make a new set of
pre-existing naitve objects, and overwrite the v8::Global
handles to those in the previous set. Thus, we may be
theoretically releasing some v8::Globals early.
==========

[Tom Sepez, 2015-11-06 23:35:26]

Description was changed from

==========
Keep "static" objects per-context rather than per isolate.

Every time we initialize a new v8::Context, we make a new set of
pre-existing naitve objects, and overwrite the v8::Global
handles to those in the previous set. Thus, we may be
theoretically releasing some v8::Globals early.
==========

to

==========
Keep "static" objects per-context rather than per isolate.

Every time we initialize a new v8::Context, we make a new set of
pre-existing native objects, and overwrite the v8::Global
handles to those in the previous set. Thus, we may be
theoretically releasing some v8::Globals early.
==========

[jochen (gone - plz use gerrit), 2015-11-07 16:36:27]

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/javascript/JS_R...
File fpdfsdk/src/javascript/JS_Runtime.h (right):

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.h:75: std::vector<v8::Global<v8::Object>*>
m_StaticObjects;
Global has move<> constructors, so you should be able to just put them directly
into a vector:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/include/v8.h&l=827

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/jsapi/fxjs_v8.cpp
File fpdfsdk/src/jsapi/fxjs_v8.cpp (right):

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/jsapi/fxjs_v8.c...
fpdfsdk/src/jsapi/fxjs_v8.cpp:345: delete pStaticObjects->at(i);
set pStaticObjects->at(i) to nullptr?

[Tom Sepez, 2015-11-09 22:00:43]

There doesn't seem to be any precedent for globals in std::vectors.

https://code.google.com/p/chromium/codesearch#search/&q=vector%3Cv8::global%3...

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/javascript/JS_R...
File fpdfsdk/src/javascript/JS_Runtime.h (right):

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.h:75: std::vector<v8::Global<v8::Object>*>
m_StaticObjects;
On 2015/11/07 16:36:27, jochen (slow - traveling) wrote:
> Global has move<> constructors, so you should be able to just put them
directly
> into a vector:
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/include/v8.h&l=827

I don't think this can work until we get c++11 std::library vector.  I tried
this and it took an execption at runtime.

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/jsapi/fxjs_v8.cpp
File fpdfsdk/src/jsapi/fxjs_v8.cpp (right):

https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/jsapi/fxjs_v8.c...
fpdfsdk/src/jsapi/fxjs_v8.cpp:345: delete pStaticObjects->at(i);
On 2015/11/07 16:36:27, jochen (slow - traveling) wrote:
> set pStaticObjects->at(i) to nullptr?

Done.

[jochen (gone - plz use gerrit), 2015-11-10 20:01:31]

lgtm

[Tom Sepez, 2015-11-10 23:07:10]

Committed patchset #2 (id:20001) manually as
7d4c74d1b96d82681d49519c129262a1568e87d4 (presubmit successful).