From a72e8e2c4392df725c7e35ed1ae3f891e79e49ec Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 7 Oct 2015 10:17:53 -0700 Subject: Merge to XFA: Pass v8::Isolate to PDFium at init time. New code changes in JS_Runtime.cpp. (cherry picked from commit 3dedace9623fef6161a8666e53a4ab2b9be61e4c) Original Review URL: https://codereview.chromium.org/1367033002 . Also merge to XFA: Run FXJS_V8 embedder tests against a shared isolate, since the two go together. (cherry picked from commit 4ea721cb7954898a9722c389dae86c62957352d0) Original Review URL: https://codereview.chromium.org/1377293004 . R=thestig@chromium.org Review URL: https://codereview.chromium.org/1381633008 . --- fpdfsdk/include/jsapi/fxjs_v8.h | 7 +++++- fpdfsdk/src/fpdfview.cpp | 6 ++++- fpdfsdk/src/javascript/JS_Runtime.cpp | 27 +++++++++----------- fpdfsdk/src/javascript/JS_Runtime.h | 2 -- fpdfsdk/src/jsapi/fxjs_v8.cpp | 40 ++++++++++++++++++++++-------- fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp | 24 ++++++++++-------- 6 files changed, 66 insertions(+), 40 deletions(-) (limited to 'fpdfsdk') diff --git a/fpdfsdk/include/jsapi/fxjs_v8.h b/fpdfsdk/include/jsapi/fxjs_v8.h index f51340acd3..6e8d2fa478 100644 --- a/fpdfsdk/include/jsapi/fxjs_v8.h +++ b/fpdfsdk/include/jsapi/fxjs_v8.h @@ -68,9 +68,14 @@ using FXJS_CONSTRUCTOR = void (*)(IFXJS_Context* cc, using FXJS_DESTRUCTOR = void (*)(v8::Local obj); // Call before making FXJS_PrepareIsolate call. -void FXJS_Initialize(unsigned int embedderDataSlot); +void FXJS_Initialize(unsigned int embedderDataSlot, v8::Isolate* pIsolate); void FXJS_Release(); +// Gets the global isolate set by FXJS_Initialize(), or makes a new one each +// time if there is no such isolate. Returns true if a new isolate had to be +// created. +bool FXJS_GetIsolate(v8::Isolate** pResultIsolate); + // Call before making FXJS_Define* calls. Resources allocated here are cleared // as part of FXJS_ReleaseRuntime(). void FXJS_PrepareIsolate(v8::Isolate* pIsolate); diff --git a/fpdfsdk/src/fpdfview.cpp b/fpdfsdk/src/fpdfview.cpp index 47da1f49de..cf05d79f5c 100644 --- a/fpdfsdk/src/fpdfview.cpp +++ b/fpdfsdk/src/fpdfview.cpp @@ -20,6 +20,7 @@ #include "../include/fpdfxfa/fpdfxfa_app.h" #include "../include/fpdfxfa/fpdfxfa_page.h" #include "../include/fpdfxfa/fpdfxfa_util.h" +#include "../include/jsapi/fxjs_v8.h" CFPDF_FileStream::CFPDF_FileStream(FPDF_FILEHANDLER* pFS) { m_pFS = pFS; @@ -194,8 +195,11 @@ FPDF_InitLibraryWithConfig(const FPDF_LIBRARY_CONFIG* cfg) { CPDF_ModuleMgr::Get()->SetCodecModule(g_pCodecModule); CPDF_ModuleMgr::Get()->InitPageModule(); CPDF_ModuleMgr::Get()->InitRenderModule(); - CPDFXFA_App::GetInstance()->Initialize(); + if (cfg && cfg->version >= 2) { + FXJS_Initialize(cfg->m_v8EmbedderSlot, + reinterpret_cast(cfg->m_pIsolate)); + } } DLLEXPORT void STDCALL FPDF_DestroyLibrary() { diff --git a/fpdfsdk/src/javascript/JS_Runtime.cpp b/fpdfsdk/src/javascript/JS_Runtime.cpp index f206154029..bc5443daf6 100644 --- a/fpdfsdk/src/javascript/JS_Runtime.cpp +++ b/fpdfsdk/src/javascript/JS_Runtime.cpp @@ -45,17 +45,18 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) if (CPDFXFA_App::GetInstance()->GetJSERuntime()) { // TODO(tsepez): CPDFXFA_App should also use the embedder provided isolate. m_isolate = (v8::Isolate*)CPDFXFA_App::GetInstance()->GetJSERuntime(); - } else if (m_pApp->GetFormFillInfo()->m_pJsPlatform->version >= 2) { - m_isolate = reinterpret_cast( - m_pApp->GetFormFillInfo()->m_pJsPlatform->m_isolate); - } - if (!m_isolate) { - m_pArrayBufferAllocator.reset(new FXJS_ArrayBufferAllocator()); - - v8::Isolate::CreateParams params; - params.array_buffer_allocator = m_pArrayBufferAllocator.get(); - m_isolate = v8::Isolate::New(params); - m_isolateManaged = true; + } else { + IPDF_JSPLATFORM* pPlatform = m_pApp->GetFormFillInfo()->m_pJsPlatform; + if (pPlatform->version <= 2) { + unsigned int embedderDataSlot = 0; + v8::Isolate* pExternalIsolate = nullptr; + if (pPlatform->version == 2) { + pExternalIsolate = reinterpret_cast(pPlatform->m_isolate); + embedderDataSlot = pPlatform->m_v8EmbedderSlot; + } + FXJS_Initialize(embedderDataSlot, pExternalIsolate); + } + m_isolateManaged = FXJS_GetIsolate(&m_isolate); } v8::Isolate* isolate = m_isolate; @@ -69,10 +70,6 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) return; } - unsigned int embedderDataSlot = 0; - if (m_pApp->GetFormFillInfo()->m_pJsPlatform->version >= 2) - embedderDataSlot = pApp->GetFormFillInfo()->m_pJsPlatform->m_v8EmbedderSlot; - FXJS_Initialize(embedderDataSlot); DefineJSObjects(); CPDFXFA_App::GetInstance()->SetJavaScriptInitialized(TRUE); diff --git a/fpdfsdk/src/javascript/JS_Runtime.h b/fpdfsdk/src/javascript/JS_Runtime.h index 0576980233..1034ae8c8b 100644 --- a/fpdfsdk/src/javascript/JS_Runtime.h +++ b/fpdfsdk/src/javascript/JS_Runtime.h @@ -10,7 +10,6 @@ #include #include -#include "../../../third_party/base/nonstd_unique_ptr.h" #include "../../../core/include/fxcrt/fx_basic.h" #include "../../include/javascript/IJavaScript.h" #include "../../include/jsapi/fxjs_v8.h" @@ -71,7 +70,6 @@ class CJS_Runtime : public IFXJS_Runtime { std::set m_FieldEventSet; v8::Isolate* m_isolate; bool m_isolateManaged; - nonstd::unique_ptr m_pArrayBufferAllocator; v8::Global m_context; std::set m_observers; }; diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp index 765ec2700d..0be7cc01e2 100644 --- a/fpdfsdk/src/jsapi/fxjs_v8.cpp +++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp @@ -16,14 +16,14 @@ const wchar_t kFXJSValueNameFxobj[] = L"fxobj"; const wchar_t kFXJSValueNameNull[] = L"null"; const wchar_t kFXJSValueNameUndefined[] = L"undefined"; -static unsigned int g_embedderDataSlot = 1u; - // Keep this consistent with the values defined in gin/public/context_holder.h // (without actually requiring a dependency on gin itself for the standalone // embedders of PDFIum). The value we want to use is: // kPerContextDataStartIndex + kEmbedderPDFium, which is 3. static const unsigned int kPerContextDataIndex = 3u; - +static unsigned int g_embedderDataSlot = 1u; +static v8::Isolate* g_isolate = nullptr; +static FXJS_ArrayBufferAllocator* g_arrayBufferAllocator = nullptr; static v8::Global* g_DefaultGlobalObjectTemplate = nullptr; class CFXJS_PrivateData { @@ -123,6 +123,33 @@ void FXJS_ArrayBufferAllocator::Free(void* data, size_t length) { free(data); } +void FXJS_Initialize(unsigned int embedderDataSlot, v8::Isolate* pIsolate) { + g_embedderDataSlot = embedderDataSlot; + g_isolate = pIsolate; +} + +void FXJS_Release() { + g_DefaultGlobalObjectTemplate = nullptr; + g_isolate = nullptr; + + delete g_arrayBufferAllocator; + g_arrayBufferAllocator = nullptr; +} + +bool FXJS_GetIsolate(v8::Isolate** pResultIsolate) { + if (g_isolate) { + *pResultIsolate = g_isolate; + return false; + } + // Provide backwards compatibility when no external isolate. + if (!g_arrayBufferAllocator) + g_arrayBufferAllocator = new FXJS_ArrayBufferAllocator(); + v8::Isolate::CreateParams params; + params.array_buffer_allocator = g_arrayBufferAllocator; + *pResultIsolate = v8::Isolate::New(params); + return true; +} + // static void FXJS_PerIsolateData::SetUp(v8::Isolate* pIsolate) { if (!pIsolate->GetData(g_embedderDataSlot)) @@ -135,13 +162,6 @@ FXJS_PerIsolateData* FXJS_PerIsolateData::Get(v8::Isolate* pIsolate) { pIsolate->GetData(g_embedderDataSlot)); } -void FXJS_Initialize(unsigned int embedderDataSlot) { - g_embedderDataSlot = embedderDataSlot; -} - -void FXJS_Release() { -} - int FXJS_DefineObj(v8::Isolate* pIsolate, const wchar_t* sObjName, FXJSOBJTYPE eObjType, diff --git a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp index 55eb9c8408..fc2324781a 100644 --- a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp +++ b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp @@ -16,40 +16,42 @@ const wchar_t kScript[] = L"fred = 7"; class FXJSV8Embeddertest : public EmbedderTest { public: - FXJSV8Embeddertest() : m_pIsolate(nullptr) {} - ~FXJSV8Embeddertest() override {} + FXJSV8Embeddertest() + : m_pArrayBufferAllocator(new FXJS_ArrayBufferAllocator) { + v8::Isolate::CreateParams params; + params.array_buffer_allocator = m_pArrayBufferAllocator.get(); + m_pIsolate = v8::Isolate::New(params); + } + + ~FXJSV8Embeddertest() override { m_pIsolate->Dispose(); } void SetUp() override { + EmbedderTest::SetExternalIsolate(m_pIsolate); EmbedderTest::SetUp(); - m_pAllocator.reset(new FXJS_ArrayBufferAllocator()); - - v8::Isolate::CreateParams params; - params.array_buffer_allocator = m_pAllocator.get(); - m_pIsolate = v8::Isolate::New(params); v8::Isolate::Scope isolate_scope(m_pIsolate); v8::Locker locker(m_pIsolate); v8::HandleScope handle_scope(m_pIsolate); - FXJS_Initialize(0); FXJS_PerIsolateData::SetUp(m_pIsolate); FXJS_InitializeRuntime(m_pIsolate, nullptr, nullptr, m_pPersistentContext); } void TearDown() override { FXJS_ReleaseRuntime(m_pIsolate, m_pPersistentContext); + m_pPersistentContext.Reset(); FXJS_Release(); EmbedderTest::TearDown(); } - v8::Isolate* isolate() const { return m_pIsolate; } + v8::Isolate* isolate() { return m_pIsolate; } v8::Local GetV8Context() { - return v8::Local::New(m_pIsolate, m_pPersistentContext); + return m_pPersistentContext.Get(m_pIsolate); } private: + nonstd::unique_ptr m_pArrayBufferAllocator; v8::Isolate* m_pIsolate; v8::Global m_pPersistentContext; - nonstd::unique_ptr m_pAllocator; }; TEST_F(FXJSV8Embeddertest, Getters) { -- cgit v1.2.3