From 1258f7f21725c703269581e0241fbad86d89209c Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 2 Feb 2018 17:37:37 +0000 Subject: Remove V8 context from CJS_V8 Makes CFXJS_Engine and CFXJSE_Engine consistent with each other in that both have a V8 context to themselves, and not one inheritted from the CJS_V8 (which is now more per-isolate than per-context). Consolidate NewLocalContext() and GetPersistentContext(), which both did the exact same thing under the covers once inside v8 land. Rename a few things to make it simpler. Change-Id: I68905db9ad44253063da235fcb276a75627a2dbc Reviewed-on: https://pdfium-review.googlesource.com/25170 Reviewed-by: dsinclair Commit-Queue: Tom Sepez --- fxjs/cjs_event_context.cpp | 2 +- fxjs/cjs_runtime.cpp | 8 ++++---- fxjs/cjs_v8.cpp | 12 +----------- fxjs/cjs_v8.h | 9 --------- fxjs/fxjs_v8.cpp | 6 +++--- fxjs/fxjs_v8.h | 6 +++++- fxjs/fxjs_v8_embeddertest.cpp | 4 ++-- testing/js_embedder_test.cpp | 2 +- 8 files changed, 17 insertions(+), 32 deletions(-) diff --git a/fxjs/cjs_event_context.cpp b/fxjs/cjs_event_context.cpp index d2f270b1c4..195fb1b888 100644 --- a/fxjs/cjs_event_context.cpp +++ b/fxjs/cjs_event_context.cpp @@ -28,7 +28,7 @@ CPDFSDK_FormFillEnvironment* CJS_EventContext::GetFormFillEnv() { bool CJS_EventContext::RunScript(const WideString& script, WideString* info) { v8::Isolate::Scope isolate_scope(m_pRuntime->GetIsolate()); v8::HandleScope handle_scope(m_pRuntime->GetIsolate()); - v8::Local context = m_pRuntime->NewLocalContext(); + v8::Local context = m_pRuntime->GetV8Context(); v8::Context::Scope context_scope(context); if (m_bBusy) { diff --git a/fxjs/cjs_runtime.cpp b/fxjs/cjs_runtime.cpp index 4031304b83..662269514a 100644 --- a/fxjs/cjs_runtime.cpp +++ b/fxjs/cjs_runtime.cpp @@ -180,7 +180,7 @@ CJS_EventContext* CJS_Runtime::GetCurrentEventContext() const { void CJS_Runtime::SetFormFillEnvToDocument() { v8::Isolate::Scope isolate_scope(GetIsolate()); v8::HandleScope handle_scope(GetIsolate()); - v8::Local context = NewLocalContext(); + v8::Local context = GetV8Context(); v8::Context::Scope context_scope(context); v8::Local pThis = GetThisObj(); @@ -235,7 +235,7 @@ bool CJS_Runtime::GetValueByNameFromGlobalObject(const ByteStringView& utf8Name, CFXJSE_Value* pValue) { v8::Isolate::Scope isolate_scope(GetIsolate()); v8::HandleScope handle_scope(GetIsolate()); - v8::Local context = NewLocalContext(); + v8::Local context = GetV8Context(); v8::Context::Scope context_scope(context); v8::Local propvalue = context->Global()->Get( v8::String::NewFromUtf8(GetIsolate(), utf8Name.unterminated_c_str(), @@ -256,10 +256,10 @@ bool CJS_Runtime::SetValueByNameInGlobalObject(const ByteStringView& utf8Name, v8::Isolate* pIsolate = GetIsolate(); v8::Isolate::Scope isolate_scope(pIsolate); v8::HandleScope handle_scope(pIsolate); - v8::Local context = NewLocalContext(); + v8::Local context = GetV8Context(); v8::Context::Scope context_scope(context); v8::Local propvalue = - v8::Local::New(GetIsolate(), pValue->DirectGetValue()); + v8::Local::New(pIsolate, pValue->DirectGetValue()); context->Global()->Set( v8::String::NewFromUtf8(pIsolate, utf8Name.unterminated_c_str(), v8::String::kNormalString, utf8Name.GetLength()), diff --git a/fxjs/cjs_v8.cpp b/fxjs/cjs_v8.cpp index 8f77ec6a88..b4f7fb1ed2 100644 --- a/fxjs/cjs_v8.cpp +++ b/fxjs/cjs_v8.cpp @@ -13,9 +13,7 @@ CJS_V8::CJS_V8(v8::Isolate* isolate) : m_isolate(isolate) {} -CJS_V8::~CJS_V8() { - m_V8PersistentContext.Reset(); -} +CJS_V8::~CJS_V8() = default; v8::Local CJS_V8::GetObjectProperty( v8::Local pObj, @@ -88,14 +86,6 @@ unsigned CJS_V8::GetArrayLength(v8::Local pArray) { return pArray->Length(); } -v8::Local CJS_V8::NewLocalContext() { - return v8::Local::New(m_isolate, m_V8PersistentContext); -} - -v8::Local CJS_V8::GetPersistentContext() { - return m_V8PersistentContext.Get(m_isolate); -} - v8::Local CJS_V8::NewNumber(int number) { return v8::Int32::New(m_isolate, number); } diff --git a/fxjs/cjs_v8.h b/fxjs/cjs_v8.h index b79d236670..1dd4953caa 100644 --- a/fxjs/cjs_v8.h +++ b/fxjs/cjs_v8.h @@ -28,9 +28,6 @@ class CJS_V8 { v8::Isolate* GetIsolate() const { return m_isolate; } - v8::Local NewLocalContext(); - v8::Local GetPersistentContext(); - v8::Local NewNull(); v8::Local NewUndefined(); v8::Local NewArray(); @@ -79,15 +76,9 @@ class CJS_V8 { void SetIsolate(v8::Isolate* pIsolate) { m_isolate = pIsolate; } void ClearConstArray() { m_ConstArrays.clear(); } - void ResetPersistentContext(v8::Local context) { - m_V8PersistentContext.Reset(m_isolate, context); - } - void ReleasePersistentContext() { m_V8PersistentContext.Reset(); } - private: v8::Isolate* m_isolate; std::map> m_ConstArrays; - v8::Global m_V8PersistentContext; }; #endif // FXJS_CJS_V8_H_ diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp index e73ab0a8f0..4c9c5602d2 100644 --- a/fxjs/fxjs_v8.cpp +++ b/fxjs/fxjs_v8.cpp @@ -423,13 +423,13 @@ void CFXJS_Engine::InitializeEngine() { } } } - ResetPersistentContext(v8Context); + m_V8Context.Reset(GetIsolate(), v8Context); } void CFXJS_Engine::ReleaseEngine() { v8::Isolate::Scope isolate_scope(GetIsolate()); v8::HandleScope handle_scope(GetIsolate()); - v8::Local context = NewLocalContext(); + v8::Local context = GetV8Context(); v8::Context::Scope context_scope(context); FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(GetIsolate()); if (!pData) @@ -457,7 +457,7 @@ void CFXJS_Engine::ReleaseEngine() { } } - ReleasePersistentContext(); + m_V8Context.Reset(); if (GetIsolate() == g_isolate && --g_isolate_ref_count > 0) return; diff --git a/fxjs/fxjs_v8.h b/fxjs/fxjs_v8.h index 0372ade9ce..851bdd5428 100644 --- a/fxjs/fxjs_v8.h +++ b/fxjs/fxjs_v8.h @@ -178,7 +178,6 @@ class CFXJS_Engine : public CJS_V8 { int Execute(const WideString& script, FXJSErr* perror); v8::Local GetThisObj(); - v8::Local NewFxDynamicObj(int nObjDefnID, bool bStatic = false); // Native object binding. @@ -189,10 +188,15 @@ class CFXJS_Engine : public CJS_V8 { void Error(const WideString& message); + v8::Local GetV8Context() { + return v8::Local::New(GetIsolate(), m_V8Context); + } + protected: CFXJS_Engine(); private: + v8::Global m_V8Context; std::vector*> m_StaticObjects; }; diff --git a/fxjs/fxjs_v8_embeddertest.cpp b/fxjs/fxjs_v8_embeddertest.cpp index 21e6460b8c..9e90663aa2 100644 --- a/fxjs/fxjs_v8_embeddertest.cpp +++ b/fxjs/fxjs_v8_embeddertest.cpp @@ -51,8 +51,8 @@ TEST_F(FXJSV8EmbedderTest, MultipleEngines) { CFXJS_Engine engine2(isolate()); engine2.InitializeEngine(); - v8::Local context1 = engine1.NewLocalContext(); - v8::Local context2 = engine2.NewLocalContext(); + v8::Local context1 = engine1.GetV8Context(); + v8::Local context2 = engine2.GetV8Context(); v8::Context::Scope context_scope(GetV8Context()); ExecuteInCurrentContext(WideString(kScript0)); diff --git a/testing/js_embedder_test.cpp b/testing/js_embedder_test.cpp index e9eb70d1ec..8e95aee142 100644 --- a/testing/js_embedder_test.cpp +++ b/testing/js_embedder_test.cpp @@ -40,5 +40,5 @@ v8::Isolate* JSEmbedderTest::isolate() { } v8::Local JSEmbedderTest::GetV8Context() { - return m_Engine->GetPersistentContext(); + return m_Engine->GetV8Context(); } -- cgit v1.2.3