Explicitly track CXFA_List and its subclasses outside constructor.
Allows for easier migration to other ownership models. Removes an
instance of the std::unique_ptr<T>(this) anti-pattern. Also makes
obvious the answer to an old TODO() about who owns a newly-created
list.
-- slight shuffling of cxfjse_engine.h methods avoids a blank line.
-- take engine directly from CFX_V8 runtime where possible.
-- remove uninstantiated CFXA_Deltas class.
Change-Id: Ifaaefb34f07a5355d87162748fdb5ba861fe8246
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/71590
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/xfa/cfxjse_engine.cpp b/fxjs/xfa/cfxjse_engine.cpp
index 5db71c0..7145ac4 100644
--- a/fxjs/xfa/cfxjse_engine.cpp
+++ b/fxjs/xfa/cfxjse_engine.cpp
@@ -771,8 +771,9 @@
return nNodes > 0;
}
-void CFXJSE_Engine::AddToCacheList(std::unique_ptr<CXFA_List> pList) {
+CXFA_List* CFXJSE_Engine::AddToCacheList(std::unique_ptr<CXFA_List> pList) {
m_CacheList.push_back(std::move(pList));
+ return m_CacheList.back().get();
}
CFXJSE_Value* CFXJSE_Engine::GetOrCreateJSBindingFromMap(CXFA_Object* pObject) {
diff --git a/fxjs/xfa/cfxjse_engine.h b/fxjs/xfa/cfxjse_engine.h
index 0eec31f..5b5c01e 100644
--- a/fxjs/xfa/cfxjse_engine.h
+++ b/fxjs/xfa/cfxjse_engine.h
@@ -87,12 +87,14 @@
CFXJSE_Value* GetOrCreateJSBindingFromMap(CXFA_Object* pObject);
void RemoveJSBindingFromMap(CXFA_Object* pObject);
- void AddToCacheList(std::unique_ptr<CXFA_List> pList);
+ // Takes ownership of |pList|, returns unowned pointer to it.
+ CXFA_List* AddToCacheList(std::unique_ptr<CXFA_List> pList);
+
CXFA_Object* GetThisObject() const { return m_pThisObject.Get(); }
+ CFXJSE_Class* GetJseNormalClass() const { return m_pJsClass.Get(); }
void SetNodesOfRunScript(std::vector<CXFA_Node*>* pArray);
void AddNodesOfRunScript(CXFA_Node* pNode);
- CFXJSE_Class* GetJseNormalClass() const { return m_pJsClass.Get(); }
void SetRunAtType(XFA_AttributeValue eRunAt) { m_eRunAtType = eRunAt; }
bool IsRunAtClient() { return m_eRunAtType != XFA_AttributeValue::Server; }
diff --git a/fxjs/xfa/cjx_container.cpp b/fxjs/xfa/cjx_container.cpp
index 20220be..52672c7 100644
--- a/fxjs/xfa/cjx_container.cpp
+++ b/fxjs/xfa/cjx_container.cpp
@@ -39,7 +39,8 @@
CFX_V8* runtime,
const std::vector<v8::Local<v8::Value>>& params) {
auto* pEngine = static_cast<CFXJSE_Engine*>(runtime);
+ auto* pList = static_cast<CXFA_ArrayNodeList*>(pEngine->AddToCacheList(
+ std::make_unique<CXFA_ArrayNodeList>(GetDocument())));
return CJS_Result::Success(pEngine->NewXFAObject(
- new CXFA_ArrayNodeList(GetDocument()),
- GetDocument()->GetScriptContext()->GetJseNormalClass()->GetTemplate()));
+ pList, pEngine->GetJseNormalClass()->GetTemplate()));
}
diff --git a/fxjs/xfa/cjx_form.cpp b/fxjs/xfa/cjx_form.cpp
index f2b3a7d..901bfa5 100644
--- a/fxjs/xfa/cjx_form.cpp
+++ b/fxjs/xfa/cjx_form.cpp
@@ -41,16 +41,14 @@
if (params.size() != 1)
return CJS_Result::Failure(JSMessage::kParamError);
- CXFA_Node* pDataNode =
- ToNode(static_cast<CFXJSE_Engine*>(runtime)->ToXFAObject(params[0]));
+ CFXJSE_Engine* pEngine = static_cast<CFXJSE_Engine*>(runtime);
+ CXFA_Node* pDataNode = ToNode(pEngine->ToXFAObject(params[0]));
if (!pDataNode)
return CJS_Result::Failure(JSMessage::kValueError);
- CXFA_ArrayNodeList* pFormNodes = new CXFA_ArrayNodeList(GetDocument());
- CFXJSE_Value* value =
- GetDocument()->GetScriptContext()->GetOrCreateJSBindingFromMap(
- pFormNodes);
-
+ auto* pFormNodes = static_cast<CXFA_ArrayNodeList*>(pEngine->AddToCacheList(
+ std::make_unique<CXFA_ArrayNodeList>(GetDocument())));
+ CFXJSE_Value* value = pEngine->GetOrCreateJSBindingFromMap(pFormNodes);
return CJS_Result::Success(
value->DirectGetValue().Get(runtime->GetIsolate()));
}
diff --git a/fxjs/xfa/cjx_layoutpseudomodel.cpp b/fxjs/xfa/cjx_layoutpseudomodel.cpp
index 8c9216e..32e777f 100644
--- a/fxjs/xfa/cjx_layoutpseudomodel.cpp
+++ b/fxjs/xfa/cjx_layoutpseudomodel.cpp
@@ -369,15 +369,16 @@
if (!pNotify)
return CJS_Result::Success();
+ CFXJSE_Engine* pEngine = static_cast<CFXJSE_Engine*>(runtime);
auto* pDocLayout = CXFA_LayoutProcessor::FromDocument(GetDocument());
- auto pArrayNodeList = std::make_unique<CXFA_ArrayNodeList>(GetDocument());
+ auto* pArrayNodeList =
+ static_cast<CXFA_ArrayNodeList*>(pEngine->AddToCacheList(
+ std::make_unique<CXFA_ArrayNodeList>(GetDocument())));
pArrayNodeList->SetArrayNodeList(
GetObjArray(pDocLayout, iIndex, wsType, bOnPageArea));
- // TODO(dsinclair): Who owns the array once we release it? Won't this leak?
- return CJS_Result::Success(static_cast<CFXJSE_Engine*>(runtime)->NewXFAObject(
- pArrayNodeList.release(),
- GetDocument()->GetScriptContext()->GetJseNormalClass()->GetTemplate()));
+ return CJS_Result::Success(pEngine->NewXFAObject(
+ pArrayNodeList, pEngine->GetJseNormalClass()->GetTemplate()));
}
CJS_Result CJX_LayoutPseudoModel::absPageCount(
diff --git a/fxjs/xfa/cjx_tree.cpp b/fxjs/xfa/cjx_tree.cpp
index 3bef3bf..145a27e 100644
--- a/fxjs/xfa/cjx_tree.cpp
+++ b/fxjs/xfa/cjx_tree.cpp
@@ -136,8 +136,9 @@
}
CFXJSE_Engine* pScriptContext = GetDocument()->GetScriptContext();
- CXFA_AttachNodeList* pNodeList =
- new CXFA_AttachNodeList(GetDocument(), GetXFANode());
+ auto* pNodeList =
+ static_cast<CXFA_AttachNodeList*>(pScriptContext->AddToCacheList(
+ std::make_unique<CXFA_AttachNodeList>(GetDocument(), GetXFANode())));
pValue->SetHostObject(pNodeList->JSObject(),
pScriptContext->GetJseNormalClass());
}
@@ -209,7 +210,9 @@
CFXJSE_Engine* pScriptContext = GetDocument()->GetScriptContext();
pScriptContext->ResolveObjects(refNode, wsExpression.AsStringView(),
&resolveNodeRS, dwFlag, nullptr);
- CXFA_ArrayNodeList* pNodeList = new CXFA_ArrayNodeList(GetDocument());
+ auto* pNodeList =
+ static_cast<CXFA_ArrayNodeList*>(pScriptContext->AddToCacheList(
+ std::make_unique<CXFA_ArrayNodeList>(GetDocument())));
if (resolveNodeRS.dwFlags == XFA_ResolveNode_RSType_Nodes) {
for (auto& pObject : resolveNodeRS.objects) {
if (pObject->IsNode())
diff --git a/xfa/fxfa/parser/BUILD.gn b/xfa/fxfa/parser/BUILD.gn
index c6f6d55..754bdcf 100644
--- a/xfa/fxfa/parser/BUILD.gn
+++ b/xfa/fxfa/parser/BUILD.gn
@@ -187,8 +187,6 @@
"cxfa_delete.h",
"cxfa_delta.cpp",
"cxfa_delta.h",
- "cxfa_deltas.cpp",
- "cxfa_deltas.h",
"cxfa_desc.cpp",
"cxfa_desc.h",
"cxfa_destination.cpp",
diff --git a/xfa/fxfa/parser/cxfa_deltas.cpp b/xfa/fxfa/parser/cxfa_deltas.cpp
deleted file mode 100644
index 7dd48a0..0000000
--- a/xfa/fxfa/parser/cxfa_deltas.cpp
+++ /dev/null
@@ -1,16 +0,0 @@
-// Copyright 2017 PDFium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-
-#include "xfa/fxfa/parser/cxfa_deltas.h"
-
-#include <memory>
-
-#include "fxjs/xfa/cjx_list.h"
-
-CXFA_Deltas::CXFA_Deltas(CXFA_Document* doc)
- : CXFA_List(doc, std::make_unique<CJX_List>(this)) {}
-
-CXFA_Deltas::~CXFA_Deltas() = default;
diff --git a/xfa/fxfa/parser/cxfa_deltas.h b/xfa/fxfa/parser/cxfa_deltas.h
deleted file mode 100644
index 4c00e9e..0000000
--- a/xfa/fxfa/parser/cxfa_deltas.h
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2017 PDFium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-
-#ifndef XFA_FXFA_PARSER_CXFA_DELTAS_H_
-#define XFA_FXFA_PARSER_CXFA_DELTAS_H_
-
-#include "xfa/fxfa/parser/cxfa_list.h"
-
-class CXFA_Deltas : public CXFA_List {
- public:
- explicit CXFA_Deltas(CXFA_Document* doc);
- ~CXFA_Deltas() override;
-};
-
-#endif // XFA_FXFA_PARSER_CXFA_DELTAS_H_
diff --git a/xfa/fxfa/parser/cxfa_list.cpp b/xfa/fxfa/parser/cxfa_list.cpp
index 035266d..bbbb509 100644
--- a/xfa/fxfa/parser/cxfa_list.cpp
+++ b/xfa/fxfa/parser/cxfa_list.cpp
@@ -24,9 +24,6 @@
XFA_ObjectType objectType,
XFA_Element eType,
std::unique_ptr<CJX_Object> obj)
- : CXFA_Object(pDocument, objectType, eType, std::move(obj)) {
- m_pDocument->GetScriptContext()->AddToCacheList(
- std::unique_ptr<CXFA_List>(this));
-}
+ : CXFA_Object(pDocument, objectType, eType, std::move(obj)) {}
CXFA_List::~CXFA_List() = default;
diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 0d49f4f..184f962 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -122,7 +122,6 @@
#include "xfa/fxfa/parser/cxfa_defaultui.h"
#include "xfa/fxfa/parser/cxfa_delete.h"
#include "xfa/fxfa/parser/cxfa_delta.h"
-#include "xfa/fxfa/parser/cxfa_deltas.h"
#include "xfa/fxfa/parser/cxfa_desc.h"
#include "xfa/fxfa/parser/cxfa_destination.h"
#include "xfa/fxfa/parser/cxfa_digestmethod.h"
