From 09ed30750282bf56a92d0e646ab22c64bea81a36 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 15 Sep 2015 14:03:52 -0700 Subject: Refactor fxjs_v8 and add embeddertests for it. This forces the layer defined by fxjs_v8.h to be (more) self-contained, so that it can be tested apart from the CJS_* objects (in fpdfsdk/{src,include}/javascript. This implies the array buffer allocator must be part of fxjs_v8. One wrinkle is that we'd like to be able to test an isolate upon which no native objects have been added, so some initialization that would have occurred as part of object definition must be made explicit. R=thestig@chromium.org Review URL: https://codereview.chromium.org/1338073002 . --- 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 | 31 +++----------- fpdfsdk/src/jsapi/fxjs_v8.cpp | 33 ++++++++++---- fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp | 69 ++++++++++++++++++++++++++++++ pdfium.gyp | 2 + 8 files changed, 130 insertions(+), 50 deletions(-) create mode 100644 fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp diff --git a/BUILD.gn b/BUILD.gn index 8f1f279cca..17276e0744 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -765,6 +765,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 3524a5b8ff..47e4c17faf 100644 --- a/fpdfsdk/include/javascript/IJavaScript.h +++ b/fpdfsdk/include/javascript/IJavaScript.h @@ -146,7 +146,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); @@ -155,7 +155,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 5ae6f1bc79..9d1927f192 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 @@ -65,7 +59,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 8ba7e81654..37b0d353e2 100644 --- a/fpdfsdk/src/javascript/JS_Runtime.cpp +++ b/fpdfsdk/src/javascript/JS_Runtime.cpp @@ -30,24 +30,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; @@ -59,18 +49,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 ------------------------------ */ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) @@ -80,12 +58,14 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) m_pFieldEventPath(NULL), m_isolate(NULL), m_isolateManaged(false) { + unsigned int embedderDataSlot = 0; if (m_pApp->GetFormFillInfo()->m_pJsPlatform->version >= 2) { m_isolate = reinterpret_cast( m_pApp->GetFormFillInfo()->m_pJsPlatform->m_isolate); + embedderDataSlot = pApp->GetFormFillInfo()->m_pJsPlatform->m_v8EmbedderSlot; } 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(); @@ -93,10 +73,11 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) m_isolateManaged = true; } + 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 e0c8938c34..c516645dc2 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::HandleScope handle_scope(pIsolate); @@ -331,7 +346,7 @@ void JS_ReleaseRuntime(v8::Isolate* pIsolate, pIsolate->SetData(g_embedderDataSlot, 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..cb4c3fe072 --- /dev/null +++ b/fpdfsdk/src/jsapi/fxjs_v8_embeddertest.cpp @@ -0,0 +1,69 @@ +// 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::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::HandleScope handle_scope(isolate()); + v8::Context::Scope context_scope(GetV8Context()); + + FXJSErr error; + CFX_WideString wsInfo; + CFX_WideString wsScript(kScript); + int sts = JS_Execute(nullptr, 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 866b563e96..5c8799e9a5 100644 --- a/pdfium.gyp +++ b/pdfium.gyp @@ -737,6 +737,7 @@ 'include_dirs': [ '<(DEPTH)', '<(DEPTH)/v8', + '<(DEPTH)/v8/include', ], 'sources': [ 'core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp', @@ -748,6 +749,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