From 0b2a9874bddf44b4226fcbafa9ce159a9b3735a8 Mon Sep 17 00:00:00 2001
From: weili <weili@chromium.org>
Date: Wed, 21 Sep 2016 11:50:43 -0700
Subject: Set up isolate in CFXJS_Engine's constructor

CFXJS_Engine class should always be constructed with an isolate, except
for its subclasses which may need to create an isolate by themselves.

Move SetIsolate() function to be protected so that only subclasses can
access it.

Review-Url: https://codereview.chromium.org/2354353002
---
 fxjs/fxjs_v8.cpp              | 2 ++
 fxjs/fxjs_v8_embeddertest.cpp | 6 ++----
 fxjs/include/fxjs_v8.h        | 9 ++++++---
 testing/js_embedder_test.cpp  | 3 +--
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp
index 5304254242..ebe38fdb7f 100644
--- a/fxjs/fxjs_v8.cpp
+++ b/fxjs/fxjs_v8.cpp
@@ -219,6 +219,8 @@ FXJS_PerIsolateData::FXJS_PerIsolateData() : m_pDynamicObjsMap(nullptr) {}
 
 CFXJS_Engine::CFXJS_Engine() : m_isolate(nullptr) {}
 
+CFXJS_Engine::CFXJS_Engine(v8::Isolate* pIsolate) : m_isolate(pIsolate) {}
+
 CFXJS_Engine::~CFXJS_Engine() {
   m_V8PersistentContext.Reset();
 }
diff --git a/fxjs/fxjs_v8_embeddertest.cpp b/fxjs/fxjs_v8_embeddertest.cpp
index 2da313192d..a877a36e56 100644
--- a/fxjs/fxjs_v8_embeddertest.cpp
+++ b/fxjs/fxjs_v8_embeddertest.cpp
@@ -45,12 +45,10 @@ TEST_F(FXJSV8EmbedderTest, MultipleEngines) {
   v8::Isolate::Scope isolate_scope(isolate());
   v8::HandleScope handle_scope(isolate());
 
-  CFXJS_Engine engine1;
-  engine1.SetIsolate(isolate());
+  CFXJS_Engine engine1(isolate());
   engine1.InitializeEngine();
 
-  CFXJS_Engine engine2;
-  engine2.SetIsolate(isolate());
+  CFXJS_Engine engine2(isolate());
   engine2.InitializeEngine();
 
   v8::Context::Scope context_scope(GetV8Context());
diff --git a/fxjs/include/fxjs_v8.h b/fxjs/include/fxjs_v8.h
index c08cc8b7ad..8b5fc8399a 100644
--- a/fxjs/include/fxjs_v8.h
+++ b/fxjs/include/fxjs_v8.h
@@ -138,7 +138,7 @@ size_t FXJS_GlobalIsolateRefCount();
 
 class CFXJS_Engine {
  public:
-  CFXJS_Engine();
+  explicit CFXJS_Engine(v8::Isolate* pIsolate);
   ~CFXJS_Engine();
 
   using Constructor = void (*)(CFXJS_Engine* pEngine,
@@ -155,8 +155,6 @@ class CFXJS_Engine {
                               CFXJS_Engine* pEngine);
 #endif  // PDF_ENABLE_XFA
 
-  // TODO(tsepez): to constructor.
-  void SetIsolate(v8::Isolate* pIsolate) { m_isolate = pIsolate; }
   v8::Isolate* GetIsolate() const { return m_isolate; }
 
   // Always returns a valid, newly-created objDefnID.
@@ -258,6 +256,11 @@ class CFXJS_Engine {
   v8::Local<v8::String> WSToJSString(const CFX_WideString& wsPropertyName);
   void Error(const CFX_WideString& message);
 
+ protected:
+  CFXJS_Engine();
+
+  void SetIsolate(v8::Isolate* pIsolate) { m_isolate = pIsolate; }
+
  private:
   v8::Isolate* m_isolate;
   v8::Global<v8::Context> m_V8PersistentContext;
diff --git a/testing/js_embedder_test.cpp b/testing/js_embedder_test.cpp
index f7aa7e38f7..125d2bff3e 100644
--- a/testing/js_embedder_test.cpp
+++ b/testing/js_embedder_test.cpp
@@ -21,8 +21,7 @@ void JSEmbedderTest::SetUp() {
   v8::Isolate::Scope isolate_scope(m_pIsolate);
   v8::HandleScope handle_scope(m_pIsolate);
   FXJS_PerIsolateData::SetUp(m_pIsolate);
-  m_Engine.reset(new CFXJS_Engine);
-  m_Engine->SetIsolate(m_pIsolate);
+  m_Engine.reset(new CFXJS_Engine(m_pIsolate));
   m_Engine->InitializeEngine();
 }
 
-- 
cgit v1.2.3