Use AutoRestorer to scope script_node_array_ in CFXJSE_Engine

Introduce CFXJSE_Engine::ScriptNodeArrayScope to manage the lifetime
and restoration of script_node_array_ using AutoRestorer. This replaces
the manual pair of SetNodesOfRunScript(&refNodes) and
SetNodesOfRunScript(nullptr).

Fix two issues: Restoring to nullptr in all cases was likely a bug
for nested calculate scripts. Failure to set to nullptr upon entry
in the non-calculate cases was likely a bug allowing the nested script
to pollute the caller's ref-nodes.

Using an RAII scope ensures that nested script invocations restore the
outer script's node array upon completion and prevents leaving dangling
pointers if an early return occurs.

Change-Id: Ia405f3f6a0457e2b30850ddc9efe3bbd15a4b478
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/162130
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

fxjs/xfa/cfxjse_engine.cpp

diff --git a/fxjs/xfa/cfxjse_engine.cpp b/fxjs/xfa/cfxjse_engine.cpp
index 6064507..5981872 100644
--- a/fxjs/xfa/cfxjse_engine.cpp
+++ b/fxjs/xfa/cfxjse_engine.cpp
@@ -191,6 +191,13 @@
   engine_->event_param_ = prev_event_param_;
 }
 
+CFXJSE_Engine::ScriptNodeArrayScope::ScriptNodeArrayScope(
+    CFXJSE_Engine* engine,
+    std::vector<cppgc::Persistent<CXFA_Node>>* array)
+    : restorer_(&engine->script_node_array_, array) {}
+
+CFXJSE_Engine::ScriptNodeArrayScope::~ScriptNodeArrayScope() = default;
+
 // The runtime belongs to the form fill environment, which may depart before
 // this object does, so look it up rather than holding it.
 CJS_Runtime* CFXJSE_Engine::GetCJSRuntime() const {
@@ -951,11 +958,6 @@
   return NewNormalXFAObject(pObject);
 }
 
-void CFXJSE_Engine::SetNodesOfRunScript(
-    std::vector<cppgc::Persistent<CXFA_Node>>* pArray) {
-  script_node_array_ = pArray;
-}
-
 void CFXJSE_Engine::AddNodesOfRunScript(CXFA_Node* pNode) {
   if (script_node_array_ &&
       !pdfium::Contains(*script_node_array_, pNode,

fxjs/xfa/cfxjse_engine.h

diff --git a/fxjs/xfa/cfxjse_engine.h b/fxjs/xfa/cfxjse_engine.h
index fafc729..4cccc25 100644
--- a/fxjs/xfa/cfxjse_engine.h
+++ b/fxjs/xfa/cfxjse_engine.h
@@ -12,6 +12,7 @@
 #include <type_traits>
 #include <vector>
 
+#include "core/fxcrt/autorestorer.h"
 #include "core/fxcrt/mask.h"
 #include "core/fxcrt/unowned_ptr.h"
 #include "fxjs/cfx_isolate_wrapper.h"
@@ -129,6 +130,19 @@
   };
   friend class EventParamScope;
 
+  class ScriptNodeArrayScope {
+    CPPGC_STACK_ALLOCATED();
+
+   public:
+    ScriptNodeArrayScope(CFXJSE_Engine* engine,
+                         std::vector<cppgc::Persistent<CXFA_Node>>* array);
+    ~ScriptNodeArrayScope();
+
+   private:
+    AutoRestorer<UnownedPtr<std::vector<cppgc::Persistent<CXFA_Node>>>>
+        restorer_;
+  };
+
   CXFA_Node* GetEventTarget() const { return target_; }
   CXFA_EventParam* GetEventParam() const { return event_param_; }
 
@@ -152,7 +166,6 @@
   CFXJSE_Class* GetJseNormalClass() const { return js_class_; }
   CXFA_Document* GetDocument() const { return document_.Get(); }
 
-  void SetNodesOfRunScript(std::vector<cppgc::Persistent<CXFA_Node>>* pArray);
   void AddNodesOfRunScript(CXFA_Node* pNode);
 
   void SetRunAtType(XFA_AttributeValue eRunAt) { run_at_type_ = eRunAt; }

xfa/fxfa/parser/cxfa_node.cpp

diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index f7e2475..7c0e3e4 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -2934,11 +2934,15 @@
       context, pEventParam->targeted_ ? this : nullptr, pEventParam);
   context->SetRunAtType(script->GetRunAt());
 
+  // Avoid wasted work in non-calculate events by not attaching a ref node
+  // array. Always attaching ref nodes would be functionally equivalent, but
+  // every node whose value is read during a non-calculate script would be
+  // pinned until ExecuteBoolScript() exits.
+  const bool needs_ref_nodes = pEventParam->type_ == XFA_EVENT_InitCalculate ||
+                               pEventParam->type_ == XFA_EVENT_Calculate;
   std::vector<cppgc::Persistent<CXFA_Node>> refNodes;
-  if (pEventParam->type_ == XFA_EVENT_InitCalculate ||
-      pEventParam->type_ == XFA_EVENT_Calculate) {
-    context->SetNodesOfRunScript(&refNodes);
-  }
+  CFXJSE_Engine::ScriptNodeArrayScope node_array_scope(
+      context, needs_ref_nodes ? &refNodes : nullptr);
 
   CFXJSE_Context::ExecutionResult exec_result;
   {
@@ -2988,7 +2992,6 @@
       }
     }
   }
-  context->SetNodesOfRunScript(nullptr);
 
   return {iRet, !value.IsEmpty() && fxv8::IsBoolean(value) &&
                     fxv8::ReentrantToBooleanHelper(isolate, value)};