From 67687896d2959494d2e689ffbbc9c86098e54280 Mon Sep 17 00:00:00 2001
From: Dan Sinclair <dsinclair@chromium.org>
Date: Mon, 6 Nov 2017 21:35:11 +0000
Subject: Consolidate CFXJSE_Engine construction

This CL removes the Initialize method from CFXJSE_Engine and moves the
code to the constuctor. The DefineJsContext and DefineJSClass methods
are also removed and done directly in the constructor.

Change-Id: I3f849509a17a6bda22b520c640f23ee110e939af
Reviewed-on: https://pdfium-review.googlesource.com/17857
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
---
 fxjs/cfxjse_engine.cpp            | 37 +++++++++++++++----------------------
 fxjs/cfxjse_engine.h              |  5 +----
 xfa/fxfa/parser/cxfa_document.cpp | 11 ++++++-----
 3 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/fxjs/cfxjse_engine.cpp b/fxjs/cfxjse_engine.cpp
index b9ef5008b1..70ab95ae6a 100644
--- a/fxjs/cfxjse_engine.cpp
+++ b/fxjs/cfxjse_engine.cpp
@@ -123,28 +123,33 @@ CXFA_Object* CFXJSE_Engine::ToObject(CFXJSE_Value* pValue,
   return static_cast<CXFA_Object*>(pHostObj);
 }
 
-CFXJSE_Engine::CFXJSE_Engine(CXFA_Document* pDocument)
+CFXJSE_Engine::CFXJSE_Engine(CXFA_Document* pDocument, v8::Isolate* pIsolate)
     : m_pDocument(pDocument),
-      m_pIsolate(nullptr),
+      m_JsContext(CFXJSE_Context::Create(pIsolate,
+                                         &GlobalClassDescriptor,
+                                         pDocument->GetRoot())),
+      m_pIsolate(pIsolate),
       m_pJsClass(nullptr),
       m_eScriptType(XFA_SCRIPTLANGTYPE_Unkown),
       m_pScriptNodeArray(nullptr),
+      m_ResolveProcessor(pdfium::MakeUnique<CFXJSE_ResolveProcessor>()),
       m_pThisObject(nullptr),
       m_dwBuiltInInFlags(0),
-      m_eRunAtType(XFA_ATTRIBUTEENUM_Client) {}
+      m_eRunAtType(XFA_ATTRIBUTEENUM_Client) {
+  RemoveBuiltInObjs(m_JsContext.get());
+  m_JsContext->EnableCompatibleMode();
+
+  // Don't know if this can happen before we remove the builtin objs and set
+  // compatibility mode.
+  m_pJsClass =
+      CFXJSE_Class::Create(m_JsContext.get(), &NormalClassDescriptor, false);
+}
 
 CFXJSE_Engine::~CFXJSE_Engine() {
   for (const auto& pair : m_mapVariableToContext)
     delete ToThisProxy(pair.second->GetGlobalObject().get(), nullptr);
 }
 
-void CFXJSE_Engine::Initialize(v8::Isolate* pIsolate) {
-  m_pIsolate = pIsolate;
-  DefineJsContext();
-  DefineJsClass();
-  m_ResolveProcessor = pdfium::MakeUnique<CFXJSE_ResolveProcessor>();
-}
-
 bool CFXJSE_Engine::RunScript(XFA_SCRIPTLANGTYPE eScriptType,
                               const WideStringView& wsScript,
                               CFXJSE_Value* hRetValue,
@@ -453,13 +458,6 @@ XFA_SCRIPTLANGTYPE CFXJSE_Engine::GetType() {
   return m_eScriptType;
 }
 
-void CFXJSE_Engine::DefineJsContext() {
-  m_JsContext = CFXJSE_Context::Create(m_pIsolate, &GlobalClassDescriptor,
-                                       m_pDocument->GetRoot());
-  RemoveBuiltInObjs(m_JsContext.get());
-  m_JsContext->EnableCompatibleMode();
-}
-
 CFXJSE_Context* CFXJSE_Engine::CreateVariablesContext(CXFA_Node* pScriptNode,
                                                       CXFA_Node* pSubform) {
   if (!pScriptNode || !pSubform)
@@ -553,11 +551,6 @@ bool CFXJSE_Engine::QueryVariableValue(CXFA_Node* pScriptNode,
   return false;
 }
 
-void CFXJSE_Engine::DefineJsClass() {
-  m_pJsClass =
-      CFXJSE_Class::Create(m_JsContext.get(), &NormalClassDescriptor, false);
-}
-
 void CFXJSE_Engine::RemoveBuiltInObjs(CFXJSE_Context* pContext) const {
   static const ByteStringView OBJ_NAME[2] = {"Number", "Date"};
   std::unique_ptr<CFXJSE_Value> pObject = pContext->GetGlobalObject();
diff --git a/fxjs/cfxjse_engine.h b/fxjs/cfxjse_engine.h
index f901850203..bfc2cc1fd9 100644
--- a/fxjs/cfxjse_engine.h
+++ b/fxjs/cfxjse_engine.h
@@ -23,10 +23,9 @@ class CFXJSE_ResolveProcessor;
 
 class CFXJSE_Engine {
  public:
-  explicit CFXJSE_Engine(CXFA_Document* pDocument);
+  explicit CFXJSE_Engine(CXFA_Document* pDocument, v8::Isolate* pIsolate);
   ~CFXJSE_Engine();
 
-  void Initialize(v8::Isolate* pIsolate);
   void SetEventParam(CXFA_EventParam param) { m_eventParam = param; }
   CXFA_EventParam* GetEventParam() { return &m_eventParam; }
   bool RunScript(XFA_SCRIPTLANGTYPE eScriptType,
@@ -97,10 +96,8 @@ class CFXJSE_Engine {
   static CXFA_Object* ToObject(CFXJSE_Value* pValue, CFXJSE_Class* pClass);
 
  private:
-  void DefineJsContext();
   CFXJSE_Context* CreateVariablesContext(CXFA_Node* pScriptNode,
                                          CXFA_Node* pSubform);
-  void DefineJsClass();
   void RemoveBuiltInObjs(CFXJSE_Context* pContext) const;
 
   UnownedPtr<CXFA_Document> const m_pDocument;
diff --git a/xfa/fxfa/parser/cxfa_document.cpp b/xfa/fxfa/parser/cxfa_document.cpp
index fc5373a18a..66778cdcfc 100644
--- a/xfa/fxfa/parser/cxfa_document.cpp
+++ b/xfa/fxfa/parser/cxfa_document.cpp
@@ -286,14 +286,15 @@ CXFA_LocaleMgr* CXFA_Document::GetLocalMgr() {
 }
 
 CFXJSE_Engine* CXFA_Document::InitScriptContext(v8::Isolate* pIsolate) {
-  CFXJSE_Engine* result = GetScriptContext();
-  result->Initialize(pIsolate);
-  return result;
+  ASSERT(!m_pScriptContext);
+  m_pScriptContext = pdfium::MakeUnique<CFXJSE_Engine>(this, pIsolate);
+  return m_pScriptContext.get();
 }
 
+// We have to call |InitScriptContext| before any calls to |GetScriptContext|
+// or the context won't have an isolate set into it.
 CFXJSE_Engine* CXFA_Document::GetScriptContext() {
-  if (!m_pScriptContext)
-    m_pScriptContext = pdfium::MakeUnique<CFXJSE_Engine>(this);
+  ASSERT(m_pScriptContext);
   return m_pScriptContext.get();
 }
 
-- 
cgit v1.2.3