From 70bc04b16646c92f221c5aa56831b01d6ec7c1ca Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 22 Sep 2015 15:49:14 -0700 Subject: 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 . --- fpdfsdk/src/jsapi/fxjs_v8.cpp | 158 ++++++++++++++--------------- fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp | 2 +- 2 files changed, 79 insertions(+), 81 deletions(-) (limited to 'fpdfsdk/src/jsapi') diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp index 69ea2cb48b..c5697c7084 100644 --- a/fpdfsdk/src/jsapi/fxjs_v8.cpp +++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp @@ -17,22 +17,32 @@ const wchar_t kFXJSValueNameFxobj[] = L"fxobj"; 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( + 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( + 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 @@ class CFXJS_ObjDefintion { 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 @@ void FXJS_ArrayBufferAllocator::Free(void* data, size_t length) { 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( + pIsolate->GetData(g_embedderDataSlot)); } int FXJS_DefineObj(v8::Isolate* pIsolate, @@ -92,12 +108,11 @@ int FXJS_DefineObj(v8::Isolate* pIsolate, 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 @@ void FXJS_DefineObjMethod(v8::Isolate* pIsolate, 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 objTemp = v8::Local::New(pIsolate, pObjDef->m_objTemplate); + objTemp->Set( v8::String::NewFromUtf8(pIsolate, bsMethodName.c_str(), v8::NewStringType::kNormal).ToLocalChecked(), @@ -130,12 +143,9 @@ void FXJS_DefineObjProperty(v8::Isolate* pIsolate, 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 objTemp = v8::Local::New(pIsolate, pObjDef->m_objTemplate); objTemp->SetAccessor( @@ -153,10 +163,9 @@ void FXJS_DefineObjAllProperties(v8::Isolate* pIsolate, 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 objTemp = v8::Local::New(pIsolate, pObjDef->m_objTemplate); objTemp->SetNamedPropertyHandler(pPropGet, pPropPut, pPropQurey, pPropDel); @@ -170,12 +179,9 @@ void FXJS_DefineObjConst(v8::Isolate* pIsolate, 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 objTemp = v8::Local::New(pIsolate, pObjDef->m_objTemplate); objTemp->Set(pIsolate, bsConstName.c_str(), pDefault); @@ -187,10 +193,9 @@ static v8::Global& _getGlobalObjectTemplate( 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 @@ void FXJS_DefineGlobalMethod(v8::Isolate* pIsolate, 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 funTempl = v8::FunctionTemplate::New(pIsolate, pMethodCall); v8::Local objTemp; @@ -264,15 +267,13 @@ void FXJS_InitializeRuntime(v8::Isolate* pIsolate, v8::Local::New(pIsolate, globalObjTemp)); v8::Context::Scope context_scope(v8Context); - v8::Local 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 objName = @@ -283,14 +284,11 @@ void FXJS_InitializeRuntime(v8::Isolate* pIsolate, 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 @@ void FXJS_ReleaseRuntime(v8::Isolate* pIsolate, v8::Local::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 pObj = v8::Local::New(pIsolate, pObjDef->m_StaticObj); @@ -334,8 +333,9 @@ void FXJS_ReleaseRuntime(v8::Isolate* pIsolate, } 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 @@ v8::Local FXJS_NewFxDynamicObj(v8::Isolate* pIsolate, int nObjDefnID) { v8::Isolate::Scope isolate_scope(pIsolate); v8::Local context = pIsolate->GetCurrentContext(); - if (-1 == nObjDefnID) { + if (nObjDefnID == -1) { v8::Local objTempl = v8::ObjectTemplate::New(pIsolate); v8::Local obj; if (objTempl->NewInstance(context).ToLocal(&obj)) @@ -390,13 +390,15 @@ v8::Local FXJS_NewFxDynamicObj(v8::Isolate* pIsolate, return v8::Local(); } - CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot); - if (!pArray) + FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate); + if (!pData) return v8::Local(); - if (nObjDefnID < 0 || nObjDefnID >= pArray->GetSize()) + if (nObjDefnID < 0 || nObjDefnID >= CFXJS_ObjDefinition::MaxID(pIsolate)) return v8::Local(); - CFXJS_ObjDefintion* pObjDef = (CFXJS_ObjDefintion*)pArray->GetAt(nObjDefnID); + + CFXJS_ObjDefinition* pObjDef = + CFXJS_ObjDefinition::ForID(pIsolate, nObjDefnID); v8::Local objTemp = v8::Local::New(pIsolate, pObjDef->m_objTemplate); @@ -404,10 +406,7 @@ v8::Local FXJS_NewFxDynamicObj(v8::Isolate* pIsolate, if (!objTemp->NewInstance(context).ToLocal(&obj)) return v8::Local(); - 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 FXJS_NewFxDynamicObj(v8::Isolate* pIsolate, } v8::Local 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(); + // Return the global object. v8::Local context = pIsolate->GetCurrentContext(); return context->Global()->GetPrototype()->ToObject(context).ToLocalChecked(); } @@ -448,12 +446,12 @@ v8::Isolate* FXJS_GetRuntime(v8::Local pObj) { 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 2671a91fff..7726c40c5e 100644 --- a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp +++ b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp @@ -31,7 +31,7 @@ class FXJSV8Embeddertest : public EmbedderTest { 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); } -- cgit v1.2.3