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 <dsinclair@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

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<v8::Context> context = m_pRuntime->NewLocalContext();
+  v8::Local<v8::Context> 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<v8::Context> context = NewLocalContext();
+  v8::Local<v8::Context> context = GetV8Context();
   v8::Context::Scope context_scope(context);
 
   v8::Local<v8::Object> 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<v8::Context> context = NewLocalContext();
+  v8::Local<v8::Context> context = GetV8Context();
   v8::Context::Scope context_scope(context);
   v8::Local<v8::Value> 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<v8::Context> context = NewLocalContext();
+  v8::Local<v8::Context> context = GetV8Context();
   v8::Context::Scope context_scope(context);
   v8::Local<v8::Value> propvalue =
-      v8::Local<v8::Value>::New(GetIsolate(), pValue->DirectGetValue());
+      v8::Local<v8::Value>::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<v8::Value> CJS_V8::GetObjectProperty(
     v8::Local<v8::Object> pObj,
@@ -88,14 +86,6 @@ unsigned CJS_V8::GetArrayLength(v8::Local<v8::Array> pArray) {
   return pArray->Length();
 }
 
-v8::Local<v8::Context> CJS_V8::NewLocalContext() {
-  return v8::Local<v8::Context>::New(m_isolate, m_V8PersistentContext);
-}
-
-v8::Local<v8::Context> CJS_V8::GetPersistentContext() {
-  return m_V8PersistentContext.Get(m_isolate);
-}
-
 v8::Local<v8::Number> 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<v8::Context> NewLocalContext();
-  v8::Local<v8::Context> GetPersistentContext();
-
   v8::Local<v8::Value> NewNull();
   v8::Local<v8::Value> NewUndefined();
   v8::Local<v8::Array> 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<v8::Context> context) {
-    m_V8PersistentContext.Reset(m_isolate, context);
-  }
-  void ReleasePersistentContext() { m_V8PersistentContext.Reset(); }
-
  private:
   v8::Isolate* m_isolate;
   std::map<WideString, v8::Global<v8::Array>> m_ConstArrays;
-  v8::Global<v8::Context> 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<v8::Context> context = NewLocalContext();
+  v8::Local<v8::Context> 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<v8::Object> GetThisObj();
-
   v8::Local<v8::Object> 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<v8::Context> GetV8Context() {
+    return v8::Local<v8::Context>::New(GetIsolate(), m_V8Context);
+  }
+
  protected:
   CFXJS_Engine();
 
  private:
+  v8::Global<v8::Context> m_V8Context;
   std::vector<v8::Global<v8::Object>*> 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<v8::Context> context1 = engine1.NewLocalContext();
-  v8::Local<v8::Context> context2 = engine2.NewLocalContext();
+  v8::Local<v8::Context> context1 = engine1.GetV8Context();
+  v8::Local<v8::Context> 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<v8::Context> JSEmbedderTest::GetV8Context() {
-  return m_Engine->GetPersistentContext();
+  return m_Engine->GetV8Context();
 }