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
Review URL: https://codereview.chromium.org/1424933013 .
diff --git a/fpdfsdk/include/jsapi/fxjs_v8.h b/fpdfsdk/include/jsapi/fxjs_v8.h
index c5a8640..b8a63c6 100644
--- a/fpdfsdk/include/jsapi/fxjs_v8.h
+++ b/fpdfsdk/include/jsapi/fxjs_v8.h
@@ -59,7 +59,6 @@
extern const wchar_t kFXJSValueNameNull[];
extern const wchar_t kFXJSValueNameUndefined[];
-
class FXJS_ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
void* Allocate(size_t length) override;
void* AllocateUninitialized(size_t length) override;
@@ -123,11 +122,14 @@
v8::Local<v8::Value> pDefault);
// Called after FXJS_Define* calls made.
-void FXJS_InitializeRuntime(v8::Isolate* pIsolate,
- IJS_Runtime* pIRuntime,
- v8::Global<v8::Context>& v8PersistentContext);
+void FXJS_InitializeRuntime(
+ v8::Isolate* pIsolate,
+ IJS_Runtime* pIRuntime,
+ v8::Global<v8::Context>* pV8PersistentContext,
+ std::vector<v8::Global<v8::Object>*>* pStaticObjects);
void FXJS_ReleaseRuntime(v8::Isolate* pIsolate,
- v8::Global<v8::Context>& v8PersistentContext);
+ v8::Global<v8::Context>* pV8PersistentContext,
+ std::vector<v8::Global<v8::Object>*>* pStaticObjects);
IJS_Runtime* FXJS_GetRuntimeFromIsolate(v8::Isolate* pIsolate);
// Called after FXJS_InitializeRuntime call made.
diff --git a/fpdfsdk/src/javascript/JS_Runtime.cpp b/fpdfsdk/src/javascript/JS_Runtime.cpp
index 34a18dc..74d9a44 100644
--- a/fpdfsdk/src/javascript/JS_Runtime.cpp
+++ b/fpdfsdk/src/javascript/JS_Runtime.cpp
@@ -64,7 +64,7 @@
DefineJSObjects();
CJS_Context* pContext = (CJS_Context*)NewContext();
- FXJS_InitializeRuntime(GetIsolate(), this, m_context);
+ FXJS_InitializeRuntime(GetIsolate(), this, &m_context, &m_StaticObjects);
ReleaseContext(pContext);
}
@@ -76,7 +76,7 @@
delete m_ContextArray.GetAt(i);
m_ContextArray.RemoveAll();
- FXJS_ReleaseRuntime(GetIsolate(), m_context);
+ FXJS_ReleaseRuntime(GetIsolate(), &m_context, &m_StaticObjects);
m_pApp = NULL;
m_pDocument = NULL;
diff --git a/fpdfsdk/src/javascript/JS_Runtime.h b/fpdfsdk/src/javascript/JS_Runtime.h
index 4a55c2e..1bdd8e6 100644
--- a/fpdfsdk/src/javascript/JS_Runtime.h
+++ b/fpdfsdk/src/javascript/JS_Runtime.h
@@ -9,6 +9,7 @@
#include <set>
#include <utility>
+#include <vector>
#include "../../include/javascript/IJavaScript.h"
#include "../../include/jsapi/fxjs_v8.h"
@@ -71,6 +72,7 @@
v8::Isolate* m_isolate;
bool m_isolateManaged;
v8::Global<v8::Context> m_context;
+ std::vector<v8::Global<v8::Object>*> m_StaticObjects;
std::set<Observer*> m_observers;
};
diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp
index 0c58a1b..d21970e 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp
@@ -95,7 +95,6 @@
v8::Isolate* m_pIsolate;
v8::Global<v8::FunctionTemplate> m_FunctionTemplate;
v8::Global<v8::Signature> m_Signature;
- v8::Global<v8::Object> m_StaticObj;
};
static v8::Local<v8::ObjectTemplate> GetGlobalObjectTemplate(
@@ -273,9 +272,11 @@
pDefault, v8::ReadOnly);
}
-void FXJS_InitializeRuntime(v8::Isolate* pIsolate,
- IJS_Runtime* pIRuntime,
- v8::Global<v8::Context>& v8PersistentContext) {
+void FXJS_InitializeRuntime(
+ v8::Isolate* pIsolate,
+ IJS_Runtime* pIRuntime,
+ v8::Global<v8::Context>* pV8PersistentContext,
+ std::vector<v8::Global<v8::Object>*>* pStaticObjects) {
if (pIsolate == g_isolate)
++g_isolate_ref_count;
@@ -289,14 +290,9 @@
v8Context->SetAlignedPointerInEmbedderData(kPerContextDataIndex, pIRuntime);
int maxID = CFXJS_ObjDefinition::MaxID(pIsolate);
+ pStaticObjects->resize(maxID + 1);
for (int i = 0; i < maxID; ++i) {
CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(pIsolate, i);
- CFX_ByteString bs = CFX_WideString(pObjDef->m_ObjName).UTF8Encode();
- v8::Local<v8::String> m_ObjName =
- v8::String::NewFromUtf8(pIsolate, bs.c_str(),
- v8::NewStringType::kNormal,
- bs.GetLength()).ToLocalChecked();
-
if (pObjDef->m_ObjType == FXJSOBJTYPE_GLOBAL) {
v8Context->Global()
->GetPrototype()
@@ -310,23 +306,27 @@
->ToObject(v8Context)
.ToLocalChecked());
} else if (pObjDef->m_ObjType == FXJSOBJTYPE_STATIC) {
+ CFX_ByteString bs = CFX_WideString(pObjDef->m_ObjName).UTF8Encode();
+ v8::Local<v8::String> m_ObjName =
+ v8::String::NewFromUtf8(pIsolate, bs.c_str(),
+ v8::NewStringType::kNormal,
+ bs.GetLength()).ToLocalChecked();
+
v8::Local<v8::Object> obj = FXJS_NewFxDynamicObj(pIsolate, pIRuntime, i);
v8Context->Global()->Set(v8Context, m_ObjName, obj).FromJust();
- pObjDef->m_StaticObj.Reset(pIsolate, obj);
+ pStaticObjects->at(i) = new v8::Global<v8::Object>(pIsolate, obj);
}
}
- v8PersistentContext.Reset(pIsolate, v8Context);
+ pV8PersistentContext->Reset(pIsolate, v8Context);
}
void FXJS_ReleaseRuntime(v8::Isolate* pIsolate,
- v8::Global<v8::Context>& v8PersistentContext) {
- if (pIsolate == g_isolate && --g_isolate_ref_count > 0)
- return;
-
+ v8::Global<v8::Context>* pV8PersistentContext,
+ std::vector<v8::Global<v8::Object>*>* pStaticObjects) {
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
v8::Local<v8::Context> context =
- v8::Local<v8::Context>::New(pIsolate, v8PersistentContext);
+ v8::Local<v8::Context>::New(pIsolate, *pV8PersistentContext);
v8::Context::Scope context_scope(context);
FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate);
@@ -340,8 +340,10 @@
if (pObjDef->m_ObjType == FXJSOBJTYPE_GLOBAL) {
pObj =
context->Global()->GetPrototype()->ToObject(context).ToLocalChecked();
- } else if (!pObjDef->m_StaticObj.IsEmpty()) {
- pObj = v8::Local<v8::Object>::New(pIsolate, pObjDef->m_StaticObj);
+ } else if (pStaticObjects->at(i) && !pStaticObjects->at(i)->IsEmpty()) {
+ pObj = v8::Local<v8::Object>::New(pIsolate, *pStaticObjects->at(i));
+ delete pStaticObjects->at(i);
+ pStaticObjects->at(i) = nullptr;
}
if (!pObj.IsEmpty()) {
@@ -352,6 +354,9 @@
delete pObjDef;
}
+ if (pIsolate == g_isolate && --g_isolate_ref_count > 0)
+ return;
+
pIsolate->SetData(g_embedderDataSlot, nullptr);
delete pData;
}
diff --git a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
index b965ced..eaaee86 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
@@ -32,11 +32,12 @@
v8::Isolate::Scope isolate_scope(m_pIsolate);
v8::HandleScope handle_scope(m_pIsolate);
FXJS_PerIsolateData::SetUp(m_pIsolate);
- FXJS_InitializeRuntime(m_pIsolate, nullptr, m_pPersistentContext);
+ FXJS_InitializeRuntime(m_pIsolate, nullptr, &m_pPersistentContext,
+ &m_StaticObjects);
}
void TearDown() override {
- FXJS_ReleaseRuntime(m_pIsolate, m_pPersistentContext);
+ FXJS_ReleaseRuntime(m_pIsolate, &m_pPersistentContext, &m_StaticObjects);
m_pPersistentContext.Reset();
FXJS_Release();
EmbedderTest::TearDown();
@@ -51,6 +52,7 @@
nonstd::unique_ptr<FXJS_ArrayBufferAllocator> m_pArrayBufferAllocator;
v8::Isolate* m_pIsolate;
v8::Global<v8::Context> m_pPersistentContext;
+ std::vector<v8::Global<v8::Object>*> m_StaticObjects;
};
TEST_F(FXJSV8Embeddertest, Getters) {
