From 7d0fcbf8198f04a5a5bd15482fdbdae919fb1891 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 15 Sep 2015 15:30:34 -0700 Subject: Merge to XFA: Refactor fxjs_v8 and add embeddertests for it. (cherry picked from commit b17d62601b21dfce85718e08cfd0ffce3a45d74e) (cherry picked from commit 09ed30750282bf56a92d0e646ab22c64bea81a36) Manual edits: fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp - add lockers. fppdfsdk/src/javascript/JS_Runtime.cpp - rework XFA init path. Original Review URL: https://codereview.chromium.org/1338073002 . R=thestig@chromium.org Review URL: https://codereview.chromium.org/1348433002 . --- BUILD.gn | 1 + fpdfsdk/include/javascript/IJavaScript.h | 4 +- fpdfsdk/include/javascript/JS_Runtime.h | 10 +---- fpdfsdk/include/jsapi/fxjs_v8.h | 30 ++++++++++--- fpdfsdk/src/javascript/JS_Runtime.cpp | 34 ++++---------- fpdfsdk/src/jsapi/fxjs_v8.cpp | 33 ++++++++++---- fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp | 71 ++++++++++++++++++++++++++++++ pdfium.gyp | 1 + 8 files changed, 133 insertions(+), 51 deletions(-) create mode 100644 fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp diff --git a/BUILD.gn b/BUILD.gn index 18a6a42c5d..b10819e6d1 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1459,6 +1459,7 @@ test("pdfium_embeddertests") { "fpdfsdk/src/fpdfview_c_api_test.c", "fpdfsdk/src/fpdfview_c_api_test.h", "fpdfsdk/src/fpdfview_embeddertest.cpp", + "fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp", "testing/embedder_test.cpp", "testing/embedder_test.h", "testing/embedder_test_mock_delegate.h", diff --git a/fpdfsdk/include/javascript/IJavaScript.h b/fpdfsdk/include/javascript/IJavaScript.h index d85fc6f109..65dae74ab4 100644 --- a/fpdfsdk/include/javascript/IJavaScript.h +++ b/fpdfsdk/include/javascript/IJavaScript.h @@ -153,7 +153,7 @@ class IFXJS_Runtime { class CJS_RuntimeFactory { public: - CJS_RuntimeFactory() : m_bInit(FALSE), m_nRef(0) {} + CJS_RuntimeFactory() : m_bInit(false), m_nRef(0) {} ~CJS_RuntimeFactory(); IFXJS_Runtime* NewJSRuntime(CPDFDoc_Environment* pApp); @@ -162,7 +162,7 @@ class CJS_RuntimeFactory { void Release(); private: - FX_BOOL m_bInit; + bool m_bInit; int m_nRef; }; diff --git a/fpdfsdk/include/javascript/JS_Runtime.h b/fpdfsdk/include/javascript/JS_Runtime.h index ac1a688369..f0285b271c 100644 --- a/fpdfsdk/include/javascript/JS_Runtime.h +++ b/fpdfsdk/include/javascript/JS_Runtime.h @@ -15,12 +15,6 @@ class CJS_Context; -class CJS_ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { - void* Allocate(size_t length) override; - void* AllocateUninitialized(size_t length) override; - void Free(void* data, size_t length) override; -}; - class CJS_FieldEvent { public: CFX_WideString sTargetName; @@ -30,7 +24,7 @@ class CJS_FieldEvent { class CJS_Runtime : public IFXJS_Runtime { public: - CJS_Runtime(CPDFDoc_Environment* pApp); + explicit CJS_Runtime(CPDFDoc_Environment* pApp); ~CJS_Runtime() override; // IFXJS_Runtime @@ -70,7 +64,7 @@ class CJS_Runtime : public IFXJS_Runtime { CJS_FieldEvent* m_pFieldEventPath; v8::Isolate* m_isolate; bool m_isolateManaged; - nonstd::unique_ptr m_pArrayBufferAllocator; + nonstd::unique_ptr m_pArrayBufferAllocator; v8::Global m_context; }; diff --git a/fpdfsdk/include/jsapi/fxjs_v8.h b/fpdfsdk/include/jsapi/fxjs_v8.h index 4195686731..6e4fc6f7a7 100644 --- a/fpdfsdk/include/jsapi/fxjs_v8.h +++ b/fpdfsdk/include/jsapi/fxjs_v8.h @@ -45,14 +45,30 @@ extern const wchar_t kFXJSValueNameFxobj[]; 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 LP_CONSTRUCTORs. class IFXJS_Context; class IFXJS_Runtime; +class JS_ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { + void* Allocate(size_t length) override; + void* AllocateUninitialized(size_t length) override; + void Free(void* data, size_t length) override; +}; + typedef void (*LP_CONSTRUCTOR)(IFXJS_Context* cc, v8::Local obj, v8::Local global); typedef void (*LP_DESTRUCTOR)(v8::Local obj); +// Call before making JS_PrepareIsolate call. +void JS_Initialize(unsigned int embedderDataSlot); +void JS_Release(); + +// Call before making JS_Define* calls. Resources allocated here are cleared +// as part of JS_ReleaseRuntime(). +void JS_PrepareIsolate(v8::Isolate* pIsolate); + // Always returns a valid, newly-created objDefnID. int JS_DefineObj(v8::Isolate* pIsolate, const wchar_t* sObjName, @@ -86,19 +102,21 @@ void JS_DefineGlobalConst(v8::Isolate* pIsolate, const wchar_t* sConstName, v8::Local pDefault); -void JS_InitialRuntime(v8::Isolate* pIsolate, - IFXJS_Runtime* pFXRuntime, - IFXJS_Context* context, - v8::Global& v8PersistentContext); +// Called after JS_Define* calls made. +void JS_InitializeRuntime(v8::Isolate* pIsolate, + IFXJS_Runtime* pFXRuntime, + IFXJS_Context* context, + v8::Global& v8PersistentContext); void JS_ReleaseRuntime(v8::Isolate* pIsolate, v8::Global& v8PersistentContext); -void JS_Initial(unsigned int embedderDataSlot); -void JS_Release(); + +// Called after JS_InitializeRuntime call made. int JS_Execute(v8::Isolate* pIsolate, IFXJS_Context* pJSContext, const wchar_t* script, long length, FXJSErr* perror); + v8::Local JS_NewFxDynamicObj(v8::Isolate* pIsolate, IFXJS_Context* pJSContext, int nObjDefnID); diff --git a/fpdfsdk/src/javascript/JS_Runtime.cpp b/fpdfsdk/src/javascript/JS_Runtime.cpp index afc6db6ae5..a85067be1b 100644 --- a/fpdfsdk/src/javascript/JS_Runtime.cpp +++ b/fpdfsdk/src/javascript/JS_Runtime.cpp @@ -32,24 +32,14 @@ CJS_RuntimeFactory::~CJS_RuntimeFactory() {} IFXJS_Runtime* CJS_RuntimeFactory::NewJSRuntime(CPDFDoc_Environment* pApp) { - if (!m_bInit) { - unsigned int embedderDataSlot = 0; - if (pApp->GetFormFillInfo()->m_pJsPlatform->version >= 2) { - embedderDataSlot = - pApp->GetFormFillInfo()->m_pJsPlatform->m_v8EmbedderSlot; - } - JS_Initial(embedderDataSlot); - m_bInit = TRUE; - } + m_bInit = true; return new CJS_Runtime(pApp); } void CJS_RuntimeFactory::AddRef() { - // to do.Should be implemented as atom manipulation. m_nRef++; } void CJS_RuntimeFactory::Release() { if (m_bInit) { - // to do.Should be implemented as atom manipulation. if (--m_nRef == 0) { JS_Release(); m_bInit = FALSE; @@ -61,18 +51,6 @@ void CJS_RuntimeFactory::DeleteJSRuntime(IFXJS_Runtime* pRuntime) { delete (CJS_Runtime*)pRuntime; } -void* CJS_ArrayBufferAllocator::Allocate(size_t length) { - return calloc(1, length); -} - -void* CJS_ArrayBufferAllocator::AllocateUninitialized(size_t length) { - return malloc(length); -} - -void CJS_ArrayBufferAllocator::Free(void* data, size_t length) { - free(data); -} - /* ------------------------------ CJS_Runtime ------------------------------ */ v8::Global& _getGlobalObjectTemplate(v8::Isolate* pIsolate); @@ -91,7 +69,7 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) m_pApp->GetFormFillInfo()->m_pJsPlatform->m_isolate); } if (!m_isolate) { - m_pArrayBufferAllocator.reset(new CJS_ArrayBufferAllocator()); + m_pArrayBufferAllocator.reset(new JS_ArrayBufferAllocator()); v8::Isolate::CreateParams params; params.array_buffer_allocator = m_pArrayBufferAllocator.get(); @@ -105,15 +83,19 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) v8::HandleScope handle_scope(isolate); if (CPDFXFA_App::GetInstance()->InitRuntime(FALSE)) { CJS_Context* pContext = (CJS_Context*)NewContext(); - JS_InitialRuntime(GetIsolate(), this, pContext, m_context); + JS_InitializeRuntime(GetIsolate(), this, pContext, m_context); ReleaseContext(pContext); return; } + unsigned int embedderDataSlot = 0; + if (m_pApp->GetFormFillInfo()->m_pJsPlatform->version >= 2) + embedderDataSlot = pApp->GetFormFillInfo()->m_pJsPlatform->m_v8EmbedderSlot; + JS_Initialize(embedderDataSlot); DefineJSObjects(); CJS_Context* pContext = (CJS_Context*)NewContext(); - JS_InitialRuntime(GetIsolate(), this, pContext, m_context); + JS_InitializeRuntime(GetIsolate(), this, pContext, m_context); ReleaseContext(pContext); } diff --git a/fpdfsdk/src/jsapi/fxjs_v8.cpp b/fpdfsdk/src/jsapi/fxjs_v8.cpp index b0fd6fbb9f..135b5e9349 100644 --- a/fpdfsdk/src/jsapi/fxjs_v8.cpp +++ b/fpdfsdk/src/jsapi/fxjs_v8.cpp @@ -75,6 +75,23 @@ class CJS_ObjDefintion { v8::Global m_StaticObj; }; +void* JS_ArrayBufferAllocator::Allocate(size_t length) { + return calloc(1, length); +} + +void* JS_ArrayBufferAllocator::AllocateUninitialized(size_t length) { + return malloc(length); +} + +void JS_ArrayBufferAllocator::Free(void* data, size_t length) { + free(data); +} + +void JS_PrepareIsolate(v8::Isolate* pIsolate) { + if (!pIsolate->GetData(g_embedderDataSlot)) + pIsolate->SetData(g_embedderDataSlot, new CFX_PtrArray()); +} + int JS_DefineObj(v8::Isolate* pIsolate, const wchar_t* sObjName, FXJSOBJTYPE eObjType, @@ -82,11 +99,9 @@ int JS_DefineObj(v8::Isolate* pIsolate, LP_DESTRUCTOR pDestructor) { v8::Isolate::Scope isolate_scope(pIsolate); v8::HandleScope handle_scope(pIsolate); + + JS_PrepareIsolate(pIsolate); CFX_PtrArray* pArray = (CFX_PtrArray*)pIsolate->GetData(g_embedderDataSlot); - if (!pArray) { - pArray = new CFX_PtrArray(); - pIsolate->SetData(g_embedderDataSlot, pArray); - } CJS_ObjDefintion* pObjDef = new CJS_ObjDefintion(pIsolate, sObjName, eObjType, pConstructor, pDestructor); pArray->Add(pObjDef); @@ -243,10 +258,10 @@ void JS_DefineGlobalConst(v8::Isolate* pIsolate, globalObjTemp.Reset(pIsolate, objTemp); } -void JS_InitialRuntime(v8::Isolate* pIsolate, - IFXJS_Runtime* pFXRuntime, - IFXJS_Context* context, - v8::Global& v8PersistentContext) { +void JS_InitializeRuntime(v8::Isolate* pIsolate, + IFXJS_Runtime* pFXRuntime, + IFXJS_Context* context, + v8::Global& v8PersistentContext) { v8::Isolate::Scope isolate_scope(pIsolate); v8::Locker locker(pIsolate); v8::HandleScope handle_scope(pIsolate); @@ -340,7 +355,7 @@ void JS_ReleaseRuntime(v8::Isolate* pIsolate, pIsolate->SetData(2, NULL); } -void JS_Initial(unsigned int embedderDataSlot) { +void JS_Initialize(unsigned int embedderDataSlot) { g_embedderDataSlot = embedderDataSlot; } diff --git a/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp new file mode 100644 index 0000000000..f45570b10f --- /dev/null +++ b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp @@ -0,0 +1,71 @@ +// Copyright 2015 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "../../../core/include/fpdfapi/fpdf_parser.h" +#include "../../../testing/embedder_test.h" +#include "../../include/fsdk_mgr.h" +#include "../../include/javascript/JS_Runtime.h" +#include "../../include/jsapi/fxjs_v8.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +const wchar_t kScript[] = L"fred = 7"; + +} // namespace + +class FXJSV8Embeddertest : public EmbedderTest { + public: + FXJSV8Embeddertest() : m_pIsolate(nullptr) {} + ~FXJSV8Embeddertest() override {} + + void SetUp() override { + EmbedderTest::SetUp(); + m_pAllocator.reset(new JS_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); + JS_Initialize(0); + JS_PrepareIsolate(m_pIsolate); + JS_InitializeRuntime(m_pIsolate, nullptr, nullptr, m_pPersistentContext); + } + + void TearDown() override { + JS_ReleaseRuntime(m_pIsolate, m_pPersistentContext); + JS_Release(); + EmbedderTest::TearDown(); + } + + v8::Isolate* isolate() const { return m_pIsolate; } + v8::Local GetV8Context() { + return v8::Local::New(m_pIsolate, m_pPersistentContext); + } + + private: + v8::Isolate* m_pIsolate; + v8::Global m_pPersistentContext; + nonstd::unique_ptr m_pAllocator; +}; + +TEST_F(FXJSV8Embeddertest, Getters) { + v8::Isolate::Scope isolate_scope(isolate()); + v8::Locker locker(isolate()); + v8::HandleScope handle_scope(isolate()); + v8::Context::Scope context_scope(GetV8Context()); + + FXJSErr error; + CFX_WideString wsInfo; + CFX_WideString wsScript(kScript); + int sts = JS_Execute(isolate(), nullptr, kScript, wcslen(kScript), &error); + EXPECT_EQ(0, sts); + + v8::Local This = JS_GetThisObj(isolate()); + v8::Local fred = JS_GetObjectElement(isolate(), This, L"fred"); + EXPECT_TRUE(fred->IsNumber()); +} diff --git a/pdfium.gyp b/pdfium.gyp index ade3db5506..bf1248fb8b 100644 --- a/pdfium.gyp +++ b/pdfium.gyp @@ -830,6 +830,7 @@ 'fpdfsdk/src/fpdfview_c_api_test.c', 'fpdfsdk/src/fpdfview_c_api_test.h', 'fpdfsdk/src/fpdfview_embeddertest.cpp', + 'fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp', 'testing/embedder_test.cpp', 'testing/embedder_test.h', 'testing/embedder_test_mock_delegate.h', -- cgit v1.2.3