From 9967cc5861fbff894eed8fca40e1e5ed524b04c6 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 24 Mar 2016 11:45:37 -0700 Subject: Re-land "Preserve m_pDynamicObjsMap until FXJS_PerIsolateData is destroyed."" Fix the destruction order. This reverts commit 129a60ec2d4d98ed953beaca629c99a52fb7e9b0. BUG=594120 TBR=jochen@chromium.org Review URL: https://codereview.chromium.org/1831723004 . --- fpdfsdk/include/jsapi/fxjs_v8.h | 3 +- fpdfsdk/jsapi/fxjs_v8.cpp | 2 +- fpdfsdk/jsapi/fxjs_v8_embeddertest.cpp | 74 ++++++++++++++++++++++++++++++---- 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/fpdfsdk/include/jsapi/fxjs_v8.h b/fpdfsdk/include/jsapi/fxjs_v8.h index dad1eaa888..00107ef7e5 100644 --- a/fpdfsdk/include/jsapi/fxjs_v8.h +++ b/fpdfsdk/include/jsapi/fxjs_v8.h @@ -97,7 +97,8 @@ class FXJS_PerIsolateData { static void SetUp(v8::Isolate* pIsolate); static FXJS_PerIsolateData* Get(v8::Isolate* pIsolate); void CreateDynamicObjsMap(v8::Isolate* pIsolate) { - m_pDynamicObjsMap = new V8TemplateMap(pIsolate); + if (!m_pDynamicObjsMap) + m_pDynamicObjsMap = new V8TemplateMap(pIsolate); } void ReleaseDynamicObjsMap() { delete m_pDynamicObjsMap; diff --git a/fpdfsdk/jsapi/fxjs_v8.cpp b/fpdfsdk/jsapi/fxjs_v8.cpp index 473b546318..034f2c6642 100644 --- a/fpdfsdk/jsapi/fxjs_v8.cpp +++ b/fpdfsdk/jsapi/fxjs_v8.cpp @@ -374,7 +374,6 @@ void FXJS_ReleaseRuntime(v8::Isolate* pIsolate, FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate); if (!pData) return; - pData->ReleaseDynamicObjsMap(); int maxID = CFXJS_ObjDefinition::MaxID(pIsolate); for (int i = 0; i < maxID; ++i) { @@ -399,6 +398,7 @@ void FXJS_ReleaseRuntime(v8::Isolate* pIsolate, if (pIsolate == g_isolate && --g_isolate_ref_count > 0) return; + pData->ReleaseDynamicObjsMap(); for (int i = 0; i < maxID; ++i) delete CFXJS_ObjDefinition::ForID(pIsolate, i); diff --git a/fpdfsdk/jsapi/fxjs_v8_embeddertest.cpp b/fpdfsdk/jsapi/fxjs_v8_embeddertest.cpp index b7700c8b60..e89bb28113 100644 --- a/fpdfsdk/jsapi/fxjs_v8_embeddertest.cpp +++ b/fpdfsdk/jsapi/fxjs_v8_embeddertest.cpp @@ -7,11 +7,32 @@ namespace { -const wchar_t kScript[] = L"fred = 7"; +const double kExpected0 = 6.0; +const double kExpected1 = 7.0; +const double kExpected2 = 8.0; + +const wchar_t kScript0[] = L"fred = 6"; +const wchar_t kScript1[] = L"fred = 7"; +const wchar_t kScript2[] = L"fred = 8"; } // namespace -class FXJSV8EmbedderTest : public JSEmbedderTest {}; +class FXJSV8EmbedderTest : public JSEmbedderTest { + public: + void ExecuteInCurrentContext(const wchar_t* script) { + FXJSErr error; + int sts = FXJS_Execute(isolate(), nullptr, script, &error); + EXPECT_EQ(0, sts); + } + void CheckAssignmentInCurrentContext(double expected) { + v8::Local This = FXJS_GetThisObj(isolate()); + v8::Local fred = FXJS_GetObjectElement(isolate(), This, L"fred"); + EXPECT_TRUE(fred->IsNumber()); + EXPECT_EQ(expected, fred->ToNumber(isolate()->GetCurrentContext()) + .ToLocalChecked() + ->Value()); + } +}; TEST_F(FXJSV8EmbedderTest, Getters) { v8::Isolate::Scope isolate_scope(isolate()); @@ -21,11 +42,48 @@ TEST_F(FXJSV8EmbedderTest, Getters) { v8::HandleScope handle_scope(isolate()); v8::Context::Scope context_scope(GetV8Context()); - FXJSErr error; - int sts = FXJS_Execute(isolate(), nullptr, kScript, &error); - EXPECT_EQ(0, sts); + ExecuteInCurrentContext(kScript1); + CheckAssignmentInCurrentContext(kExpected1); +} + +TEST_F(FXJSV8EmbedderTest, MultipleRutimes) { + v8::Isolate::Scope isolate_scope(isolate()); +#ifdef PDF_ENABLE_XFA + v8::Locker locker(isolate()); +#endif // PDF_ENABLE_XFA + v8::HandleScope handle_scope(isolate()); + + v8::Global global_context1; + std::vector*> static_objects1; + FXJS_InitializeRuntime(isolate(), nullptr, &global_context1, + &static_objects1); + + v8::Global global_context2; + std::vector*> static_objects2; + FXJS_InitializeRuntime(isolate(), nullptr, &global_context2, + &static_objects2); + + v8::Context::Scope context_scope(GetV8Context()); + ExecuteInCurrentContext(kScript0); + CheckAssignmentInCurrentContext(kExpected0); + + { + v8::Local context1 = + v8::Local::New(isolate(), global_context1); + v8::Context::Scope context_scope(context1); + ExecuteInCurrentContext(kScript1); + CheckAssignmentInCurrentContext(kExpected1); + } + FXJS_ReleaseRuntime(isolate(), &global_context1, &static_objects1); + + { + v8::Local context2 = + v8::Local::New(isolate(), global_context2); + v8::Context::Scope context_scope(context2); + ExecuteInCurrentContext(kScript2); + CheckAssignmentInCurrentContext(kExpected2); + } + FXJS_ReleaseRuntime(isolate(), &global_context2, &static_objects2); - v8::Local This = FXJS_GetThisObj(isolate()); - v8::Local fred = FXJS_GetObjectElement(isolate(), This, L"fred"); - EXPECT_TRUE(fred->IsNumber()); + CheckAssignmentInCurrentContext(kExpected0); } -- cgit v1.2.3