Garbage collect CXFA_CalcData.
Also move it to CJX_Object, where it belongs, and nest.
-- simplify some logic along the way.
Bug: pdfium:1563
Change-Id: Ib929a93da97c18e17d25277d5b636544324f598b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73790
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/xfa/cjx_object.cpp b/fxjs/xfa/cjx_object.cpp
index ba3ebb1..7abc68c 100644
--- a/fxjs/xfa/cjx_object.cpp
+++ b/fxjs/xfa/cjx_object.cpp
@@ -14,6 +14,7 @@
#include "core/fxcrt/xml/cfx_xmlelement.h"
#include "core/fxcrt/xml/cfx_xmltext.h"
#include "fxjs/cjs_result.h"
+#include "fxjs/gc/container_trace.h"
#include "fxjs/xfa/cfxjse_engine.h"
#include "fxjs/xfa/cfxjse_value.h"
#include "fxjs/xfa/cjx_boolean.h"
@@ -134,6 +135,7 @@
void CJX_Object::Trace(cppgc::Visitor* visitor) const {
visitor->Trace(object_);
visitor->Trace(layout_item_);
+ visitor->Trace(calc_data_);
}
bool CJX_Object::DynamicTypeIs(TypeTag eType) const {
@@ -1018,12 +1020,9 @@
if (!pDstModule)
return;
- bool bNeedMove = true;
- if (pDstModule->GetElementType() != GetXFAObject()->GetElementType())
- bNeedMove = false;
+ if (pDstModule->GetElementType() == GetXFAObject()->GetElementType())
+ ToNode(pDstModule)->JSObject()->TakeCalcDataFrom(this);
- if (bNeedMove)
- ToNode(pDstModule)->JSObject()->SetCalcData(ReleaseCalcData());
if (!pDstModule->IsNodeV())
return;
@@ -1070,12 +1069,17 @@
GetXFANode()->SendAttributeChangeMessage(eAttr, bScriptModify);
}
-void CJX_Object::SetCalcData(std::unique_ptr<CXFA_CalcData> data) {
- calc_data_ = std::move(data);
+CJX_Object::CalcData* CJX_Object::GetOrCreateCalcData(cppgc::Heap* heap) {
+ if (!calc_data_) {
+ calc_data_ =
+ cppgc::MakeGarbageCollected<CalcData>(heap->GetAllocationHandle());
+ }
+ return calc_data_;
}
-std::unique_ptr<CXFA_CalcData> CJX_Object::ReleaseCalcData() {
- return std::move(calc_data_);
+void CJX_Object::TakeCalcDataFrom(CJX_Object* that) {
+ calc_data_ = that->calc_data_;
+ that->calc_data_ = nullptr;
}
void CJX_Object::ScriptAttributeString(CFXJSE_Value* pValue,
@@ -1510,3 +1514,11 @@
void CJX_Object::ScriptSubmitFormatMode(CFXJSE_Value* pValue,
bool bSetting,
XFA_Attribute eAttribute) {}
+
+CJX_Object::CalcData::CalcData() = default;
+
+CJX_Object::CalcData::~CalcData() = default;
+
+void CJX_Object::CalcData::Trace(cppgc::Visitor* visitor) const {
+ ContainerTrace(visitor, m_Globals);
+}
diff --git a/fxjs/xfa/cjx_object.h b/fxjs/xfa/cjx_object.h
index c3cdbf5..35206e4 100644
--- a/fxjs/xfa/cjx_object.h
+++ b/fxjs/xfa/cjx_object.h
@@ -27,7 +27,6 @@
class CFXJSE_Value;
class CFX_V8;
class CJX_Object;
-class CXFA_CalcData;
class CXFA_Document;
class CXFA_LayoutItem;
class CXFA_Node;
@@ -100,6 +99,19 @@
Xfa,
};
+ class CalcData : public cppgc::GarbageCollected<CalcData> {
+ public:
+ CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED;
+ ~CalcData();
+
+ void Trace(cppgc::Visitor* visitor) const;
+
+ std::vector<cppgc::Member<CXFA_Node>> m_Globals;
+
+ private:
+ CalcData();
+ };
+
CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED;
~CJX_Object() override;
@@ -214,9 +226,9 @@
void MergeAllData(CXFA_Object* pDstModule);
- void SetCalcData(std::unique_ptr<CXFA_CalcData> data);
- CXFA_CalcData* GetCalcData() const { return calc_data_.get(); }
- std::unique_ptr<CXFA_CalcData> ReleaseCalcData();
+ CalcData* GetCalcData() const { return calc_data_; }
+ CalcData* GetOrCreateCalcData(cppgc::Heap* heap);
+ void TakeCalcDataFrom(CJX_Object* that);
void ThrowInvalidPropertyException() const;
void ThrowArgumentMismatchException() const;
@@ -268,8 +280,8 @@
cppgc::Member<CXFA_Object> object_;
cppgc::Member<CXFA_LayoutItem> layout_item_;
+ cppgc::Member<CalcData> calc_data_;
std::unique_ptr<XFA_MAPMODULEDATA> map_module_data_;
- std::unique_ptr<CXFA_CalcData> calc_data_;
std::map<ByteString, CJX_MethodCall> method_specs_;
size_t calc_recursion_count_ = 0;
};
diff --git a/xfa/fxfa/cxfa_ffdocview.cpp b/xfa/fxfa/cxfa_ffdocview.cpp
index 0663015..ceeda58 100644
--- a/xfa/fxfa/cxfa_ffdocview.cpp
+++ b/xfa/fxfa/cxfa_ffdocview.cpp
@@ -528,11 +528,11 @@
}
void CXFA_FFDocView::AddCalculateNodeNotify(CXFA_Node* pNodeChange) {
- CXFA_CalcData* pGlobalData = pNodeChange->JSObject()->GetCalcData();
+ CJX_Object::CalcData* pGlobalData = pNodeChange->JSObject()->GetCalcData();
if (!pGlobalData)
return;
- for (auto* pResult : pGlobalData->m_Globals) {
+ for (auto pResult : pGlobalData->m_Globals) {
if (!pResult->HasRemovedChildren() && pResult->IsWidgetReady())
AddCalculateNode(pResult);
}
diff --git a/xfa/fxfa/cxfa_ffwidget.cpp b/xfa/fxfa/cxfa_ffwidget.cpp
index 77ca30c..408dbe4 100644
--- a/xfa/fxfa/cxfa_ffwidget.cpp
+++ b/xfa/fxfa/cxfa_ffwidget.cpp
@@ -224,10 +224,6 @@
return GetFFWidget(ToContentLayoutItem(pLayoutItem));
}
-CXFA_CalcData::CXFA_CalcData() = default;
-
-CXFA_CalcData::~CXFA_CalcData() = default;
-
CXFA_FFWidget::CXFA_FFWidget(CXFA_Node* node) : m_pNode(node) {}
CXFA_FFWidget::~CXFA_FFWidget() = default;
diff --git a/xfa/fxfa/cxfa_ffwidget.h b/xfa/fxfa/cxfa_ffwidget.h
index 637d7a7..023f09d 100644
--- a/xfa/fxfa/cxfa_ffwidget.h
+++ b/xfa/fxfa/cxfa_ffwidget.h
@@ -60,14 +60,6 @@
void XFA_RectWithoutMargin(CFX_RectF* rt, const CXFA_Margin* margin);
CXFA_FFWidget* XFA_GetWidgetFromLayoutItem(CXFA_LayoutItem* pLayoutItem);
-class CXFA_CalcData {
- public:
- CXFA_CalcData();
- ~CXFA_CalcData();
-
- std::vector<CXFA_Node*> m_Globals;
-};
-
class CXFA_FFWidget : public cppgc::GarbageCollected<CXFA_FFWidget>,
public CFWL_Widget::AdapterIface {
CPPGC_USING_PRE_FINALIZER(CXFA_FFWidget, PreFinalize);
diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 52f5cc9..212d7b0 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -2818,11 +2818,8 @@
if (pRefNode == this)
continue;
- CXFA_CalcData* pGlobalData = pRefNode->JSObject()->GetCalcData();
- if (!pGlobalData) {
- pRefNode->JSObject()->SetCalcData(std::make_unique<CXFA_CalcData>());
- pGlobalData = pRefNode->JSObject()->GetCalcData();
- }
+ CJX_Object::CalcData* pGlobalData =
+ pRefNode->JSObject()->GetOrCreateCalcData(pDoc->GetHeap());
if (!pdfium::Contains(pGlobalData->m_Globals, this))
pGlobalData->m_Globals.push_back(this);
}
