Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(13)

Issue 1424933013: Keep "static" objects per-context rather than per isolate. (Closed)

Created:
5 years, 2 months ago by Tom Sepez
Modified:
5 years, 2 months ago
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 4

Patch Set 2 : Nullptr #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -28 lines) Patch
M fpdfsdk/include/jsapi/fxjs_v8.h View 1 2 chunks +7 lines, -5 lines 0 comments Download
M fpdfsdk/src/javascript/JS_Runtime.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
M fpdfsdk/src/javascript/JS_Runtime.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M fpdfsdk/src/jsapi/fxjs_v8.cpp View 1 6 chunks +24 lines, -19 lines 0 comments Download
M fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp View 1 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (4 generated)
jochen (gone - plz use gerrit)
https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/javascript/JS_Runtime.h File fpdfsdk/src/javascript/JS_Runtime.h (right): https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/javascript/JS_Runtime.h#newcode75 fpdfsdk/src/javascript/JS_Runtime.h:75: std::vector<v8::Global<v8::Object>*> m_StaticObjects; Global has move<> constructors, so you should ...
5 years, 2 months ago (2015-11-07 16:36:27 UTC) #5
Tom Sepez
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%3C&sq=package:chromium&type=cs https://codereview.chromium.org/1424933013/diff/1/fpdfsdk/src/javascript/JS_Runtime.h File fpdfsdk/src/javascript/JS_Runtime.h ...
5 years, 2 months ago (2015-11-09 22:00:43 UTC) #6
jochen (gone - plz use gerrit)
lgtm
5 years, 2 months ago (2015-11-10 20:01:31 UTC) #7
Tom Sepez
5 years, 2 months ago (2015-11-10 23:07:10 UTC) #8
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
7d4c74d1b96d82681d49519c129262a1568e87d4 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698