Merge to master: contention over isolate data slots
Work on this was first performed on the XFA branch, since
it has additional requirements (FXJSE layer) that needed
to be accomodated by the solution.
(cherry picked from commit ed7b2b50aa1744e0bc5a60bef12c61fa91d863b7)
Original Review URL: https://codereview.chromium.org/1351173002 .
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/1354593004 .
diff --git a/fpdfsdk/include/jsapi/fxjs_v8.h b/fpdfsdk/include/jsapi/fxjs_v8.h
index a154159..eb810c0 100644
--- a/fpdfsdk/include/jsapi/fxjs_v8.h
+++ b/fpdfsdk/include/jsapi/fxjs_v8.h
@@ -11,7 +11,12 @@
#define FPDFSDK_INCLUDE_JSAPI_FXJS_V8_H_
#include <v8.h>
-#include "../../../core/include/fxcrt/fx_string.h" // For CFX_WideString
+#include "../../../core/include/fxcrt/fx_basic.h"
+
+// FXJS_V8 places no interpretation on these two classes; it merely
+// passes them on to the caller-provided FXJS_CONSTRUCTORs.
+class IFXJS_Context;
+class IFXJS_Runtime;
enum FXJSOBJTYPE {
FXJS_DYNAMIC = 0,
@@ -24,6 +29,18 @@
unsigned linnum;
};
+class FXJS_PerIsolateData {
+ public:
+ static void SetUp(v8::Isolate* pIsolate);
+ static FXJS_PerIsolateData* Get(v8::Isolate* pIsolate);
+
+ CFX_PtrArray m_ObjectDefnArray;
+ IFXJS_Runtime* m_pFXJSRuntime;
+
+ protected:
+ FXJS_PerIsolateData() : m_pFXJSRuntime(nullptr) {}
+};
+
extern const wchar_t kFXJSValueNameString[];
extern const wchar_t kFXJSValueNameNumber[];
extern const wchar_t kFXJSValueNameBoolean[];
@@ -33,10 +50,6 @@
extern const wchar_t kFXJSValueNameNull[];
extern const wchar_t kFXJSValueNameUndefined[];
-// FXJS_V8 places no interpretation on these two classes; it merely
-// passes them on to the caller-provided FXJS_CONSTRUCTORs.
-class IFXJS_Context;
-class IFXJS_Runtime;
class FXJS_ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
void* Allocate(size_t length) override;
diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp
index 69ea2cb..c5697c7 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp
@@ -17,22 +17,32 @@
const wchar_t kFXJSValueNameNull[] = L"null";
const wchar_t kFXJSValueNameUndefined[] = L"undefined";
-static unsigned int g_embedderDataSlot = 0u;
+static unsigned int g_embedderDataSlot = 1u;
class CFXJS_PrivateData {
public:
- CFXJS_PrivateData() : ObjDefID(-1), pPrivate(NULL) {}
+ CFXJS_PrivateData(int nObjDefID) : ObjDefID(nObjDefID), pPrivate(NULL) {}
+
int ObjDefID;
void* pPrivate;
};
-class CFXJS_ObjDefintion {
+class CFXJS_ObjDefinition {
public:
- CFXJS_ObjDefintion(v8::Isolate* isolate,
- const wchar_t* sObjName,
- FXJSOBJTYPE eObjType,
- FXJS_CONSTRUCTOR pConstructor,
- FXJS_DESTRUCTOR pDestructor)
+ static int MaxID(v8::Isolate* pIsolate) {
+ return static_cast<int>(
+ FXJS_PerIsolateData::Get(pIsolate)->m_ObjectDefnArray.GetSize());
+ }
+ static CFXJS_ObjDefinition* ForID(v8::Isolate* pIsolate, int id) {
+ // Note: GetAt() halts if out-of-range even in release builds.
+ return static_cast<CFXJS_ObjDefinition*>(
+ FXJS_PerIsolateData::Get(pIsolate)->m_ObjectDefnArray.GetAt(id));
+ }
+ CFXJS_ObjDefinition(v8::Isolate* isolate,
+ const wchar_t* sObjName,
+ FXJSOBJTYPE eObjType,
+ FXJS_CONSTRUCTOR pConstructor,
+ FXJS_DESTRUCTOR pDestructor)
: objName(sObjName),
objType(eObjType),
m_pConstructor(pConstructor),
@@ -51,12 +61,11 @@
m_bSetAsGlobalObject = TRUE;
}
}
- ~CFXJS_ObjDefintion() {
+ ~CFXJS_ObjDefinition() {
m_objTemplate.Reset();
m_StaticObj.Reset();
}
- public:
const wchar_t* objName;
FXJSOBJTYPE objType;
FXJS_CONSTRUCTOR m_pConstructor;
@@ -79,9 +88,16 @@
free(data);
}
-void FXJS_PrepareIsolate(v8::Isolate* pIsolate) {
+// static
+void FXJS_PerIsolateData::SetUp(v8::Isolate* pIsolate) {
if (!pIsolate->GetData(g_embedderDataSlot))
- pIsolate->SetData(g_embedderDataSlot, new CFX_PtrArray());
+ pIsolate->SetData(g_embedderDataSlot, new FXJS_PerIsolateData());
+}
+
+// static
+FXJS_PerIsolateData* FXJS_PerIsolateData::Get(v8::Isolate* pIsolate) {
+ return static_cast<FXJS_PerIsolateData*>(
+ pIsolate->GetData(g_embedderDataSlot));
}
int FXJS_DefineObj(v8::Isolate* pIsolate,
@@ -92,12 +108,11 @@
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
- FXJS_PrepareIsolate(pIsolate);
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- CFXJS_ObjDefintion* pObjDef = new CFXJS_ObjDefintion(
- pIsolate, sObjName, eObjType, pConstructor, pDestructor);
- pArray->Add(pObjDef);
- return pArray->GetSize() - 1;
+ FXJS_PerIsolateData::SetUp(pIsolate);
+ FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate);
+ pData->m_ObjectDefnArray.Add(new CFXJS_ObjDefinition(
+ pIsolate, sObjName, eObjType, pConstructor, pDestructor));
+ return pData->m_ObjectDefnArray.GetSize() - 1;
}
void FXJS_DefineObjMethod(v8::Isolate* pIsolate,
@@ -107,14 +122,12 @@
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
- CFX_WideString ws = CFX_WideString(sMethodName);
- CFX_ByteString bsMethodName = ws.UTF8Encode();
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
-
- // Note: GetAt() halts if out-of-range even in release builds.
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(nObjDefnID);
+ CFX_ByteString bsMethodName = CFX_WideString(sMethodName).UTF8Encode();
+ CFXJS_ObjDefinition* pObjDef =
+ CFXJS_ObjDefinition::ForID(pIsolate, nObjDefnID);
v8::Local<v8::ObjectTemplate> objTemp =
v8::Local<v8::ObjectTemplate>::New(pIsolate, pObjDef->m_objTemplate);
+
objTemp->Set(
v8::String::NewFromUtf8(pIsolate, bsMethodName.c_str(),
v8::NewStringType::kNormal).ToLocalChecked(),
@@ -130,12 +143,9 @@
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
- CFX_WideString ws = CFX_WideString(sPropName);
- CFX_ByteString bsPropertyName = ws.UTF8Encode();
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
-
- // Note: GetAt() halts if out-of-range even in release builds.
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(nObjDefnID);
+ CFX_ByteString bsPropertyName = CFX_WideString(sPropName).UTF8Encode();
+ CFXJS_ObjDefinition* pObjDef =
+ CFXJS_ObjDefinition::ForID(pIsolate, nObjDefnID);
v8::Local<v8::ObjectTemplate> objTemp =
v8::Local<v8::ObjectTemplate>::New(pIsolate, pObjDef->m_objTemplate);
objTemp->SetAccessor(
@@ -153,10 +163,9 @@
v8::NamedPropertyDeleterCallback pPropDel) {
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- // Note: GetAt() halts if out-of-range even in release builds.
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(nObjDefnID);
+ CFXJS_ObjDefinition* pObjDef =
+ CFXJS_ObjDefinition::ForID(pIsolate, nObjDefnID);
v8::Local<v8::ObjectTemplate> objTemp =
v8::Local<v8::ObjectTemplate>::New(pIsolate, pObjDef->m_objTemplate);
objTemp->SetNamedPropertyHandler(pPropGet, pPropPut, pPropQurey, pPropDel);
@@ -170,12 +179,9 @@
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
- CFX_WideString ws = CFX_WideString(sConstName);
- CFX_ByteString bsConstName = ws.UTF8Encode();
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
-
- // Note: GetAt() halts if out-of-range even in release builds.
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(nObjDefnID);
+ CFX_ByteString bsConstName = CFX_WideString(sConstName).UTF8Encode();
+ CFXJS_ObjDefinition* pObjDef =
+ CFXJS_ObjDefinition::ForID(pIsolate, nObjDefnID);
v8::Local<v8::ObjectTemplate> objTemp =
v8::Local<v8::ObjectTemplate>::New(pIsolate, pObjDef->m_objTemplate);
objTemp->Set(pIsolate, bsConstName.c_str(), pDefault);
@@ -187,10 +193,9 @@
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- ASSERT(pArray != NULL);
- for (int i = 0; i < pArray->GetSize(); i++) {
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(i);
+ int maxID = CFXJS_ObjDefinition::MaxID(pIsolate);
+ for (int i = 0; i < maxID; ++i) {
+ CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(pIsolate, i);
if (pObjDef->m_bSetAsGlobalObject)
return pObjDef->m_objTemplate;
}
@@ -204,9 +209,7 @@
v8::Isolate::Scope isolate_scope(pIsolate);
v8::HandleScope handle_scope(pIsolate);
- CFX_WideString ws = CFX_WideString(sMethodName);
- CFX_ByteString bsMethodName = ws.UTF8Encode();
-
+ CFX_ByteString bsMethodName = CFX_WideString(sMethodName).UTF8Encode();
v8::Local<v8::FunctionTemplate> funTempl =
v8::FunctionTemplate::New(pIsolate, pMethodCall);
v8::Local<v8::ObjectTemplate> objTemp;
@@ -264,15 +267,13 @@
v8::Local<v8::ObjectTemplate>::New(pIsolate, globalObjTemp));
v8::Context::Scope context_scope(v8Context);
- v8::Local<v8::External> ptr = v8::External::New(pIsolate, pFXRuntime);
- v8Context->SetEmbedderData(1, ptr);
+ FXJS_PerIsolateData::SetUp(pIsolate);
+ FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate);
+ pData->m_pFXJSRuntime = pFXRuntime;
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- if (!pArray)
- return;
-
- for (int i = 0; i < pArray->GetSize(); i++) {
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(i);
+ int maxID = CFXJS_ObjDefinition::MaxID(pIsolate);
+ for (int i = 0; i < maxID; ++i) {
+ CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(pIsolate, i);
CFX_WideString ws = CFX_WideString(pObjDef->objName);
CFX_ByteString bs = ws.UTF8Encode();
v8::Local<v8::String> objName =
@@ -283,14 +284,11 @@
if (pObjDef->objType == FXJS_DYNAMIC) {
// Document is set as global object, need to construct it first.
if (ws.Equal(L"Document")) {
- CFXJS_PrivateData* pPrivateData = new CFXJS_PrivateData;
- pPrivateData->ObjDefID = i;
-
v8Context->Global()
->GetPrototype()
->ToObject(v8Context)
.ToLocalChecked()
- ->SetAlignedPointerInInternalField(0, pPrivateData);
+ ->SetAlignedPointerInInternalField(0, new CFXJS_PrivateData(i));
if (pObjDef->m_pConstructor)
pObjDef->m_pConstructor(context, v8Context->Global()
@@ -319,12 +317,13 @@
v8::Local<v8::Context>::New(pIsolate, v8PersistentContext);
v8::Context::Scope context_scope(context);
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- if (!pArray)
+ FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate);
+ if (!pData)
return;
- for (int i = 0; i < pArray->GetSize(); i++) {
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(i);
+ int maxID = CFXJS_ObjDefinition::MaxID(pIsolate);
+ for (int i = 0; i < maxID; ++i) {
+ CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(pIsolate, i);
if (!pObjDef->m_StaticObj.IsEmpty()) {
v8::Local<v8::Object> pObj =
v8::Local<v8::Object>::New(pIsolate, pObjDef->m_StaticObj);
@@ -334,8 +333,9 @@
}
delete pObjDef;
}
- delete pArray;
- pIsolate->SetData(g_embedderDataSlot, NULL);
+
+ pIsolate->SetData(g_embedderDataSlot, nullptr);
+ delete pData;
}
void FXJS_Initialize(unsigned int embedderDataSlot) {
@@ -382,7 +382,7 @@
int nObjDefnID) {
v8::Isolate::Scope isolate_scope(pIsolate);
v8::Local<v8::Context> context = pIsolate->GetCurrentContext();
- if (-1 == nObjDefnID) {
+ if (nObjDefnID == -1) {
v8::Local<v8::ObjectTemplate> objTempl = v8::ObjectTemplate::New(pIsolate);
v8::Local<v8::Object> obj;
if (objTempl->NewInstance(context).ToLocal(&obj))
@@ -390,13 +390,15 @@
return v8::Local<v8::Object>();
}
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- if (!pArray)
+ FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate);
+ if (!pData)
return v8::Local<v8::Object>();
- if (nObjDefnID < 0 || nObjDefnID >= pArray->GetSize())
+ if (nObjDefnID < 0 || nObjDefnID >= CFXJS_ObjDefinition::MaxID(pIsolate))
return v8::Local<v8::Object>();
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(nObjDefnID);
+
+ CFXJS_ObjDefinition* pObjDef =
+ CFXJS_ObjDefinition::ForID(pIsolate, nObjDefnID);
v8::Local<v8::ObjectTemplate> objTemp =
v8::Local<v8::ObjectTemplate>::New(pIsolate, pObjDef->m_objTemplate);
@@ -404,10 +406,7 @@
if (!objTemp->NewInstance(context).ToLocal(&obj))
return v8::Local<v8::Object>();
- CFXJS_PrivateData* pPrivateData = new CFXJS_PrivateData;
- pPrivateData->ObjDefID = nObjDefnID;
-
- obj->SetAlignedPointerInInternalField(0, pPrivateData);
+ obj->SetAlignedPointerInInternalField(0, new CFXJS_PrivateData(nObjDefnID));
if (pObjDef->m_pConstructor)
pObjDef->m_pConstructor(
pJSContext, obj,
@@ -417,12 +416,11 @@
}
v8::Local<v8::Object> FXJS_GetThisObj(v8::Isolate* pIsolate) {
- // Return the global object.
v8::Isolate::Scope isolate_scope(pIsolate);
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- if (!pArray)
+ if (!FXJS_PerIsolateData::Get(pIsolate))
return v8::Local<v8::Object>();
+ // Return the global object.
v8::Local<v8::Context> context = pIsolate->GetCurrentContext();
return context->Global()->GetPrototype()->ToObject(context).ToLocalChecked();
}
@@ -448,12 +446,12 @@
int FXJS_GetObjDefnID(v8::Isolate* pIsolate, const wchar_t* pObjName) {
v8::Isolate::Scope isolate_scope(pIsolate);
- CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot);
- if (!pArray)
+ if (!FXJS_PerIsolateData::Get(pIsolate))
return -1;
- for (int i = 0; i < pArray->GetSize(); i++) {
- CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(i);
+ int maxID = CFXJS_ObjDefinition::MaxID(pIsolate);
+ for (int i = 0; i < maxID; ++i) {
+ CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(pIsolate, i);
if (FXSYS_wcscmp(pObjDef->objName, pObjName) == 0)
return i;
}
diff --git a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
index 2671a91..7726c40 100644
--- a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
+++ b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp
@@ -31,7 +31,7 @@
v8::Isolate::Scope isolate_scope(m_pIsolate);
v8::HandleScope handle_scope(m_pIsolate);
FXJS_Initialize(0);
- FXJS_PrepareIsolate(m_pIsolate);
+ FXJS_PerIsolateData::SetUp(m_pIsolate);
FXJS_InitializeRuntime(m_pIsolate, nullptr, nullptr, m_pPersistentContext);
}
