Retain layout items rather than raw widgets in CFXA_TabParam.
Prevent them from popping out of existence while we might be iterating
over them.
-- forward-declare CXFA_TabParam in header file.
Bug: chromium:1109108
Change-Id: I0e97cf3a98861e004808c124f5cfdffb3abf27fd
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/71890
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
index d74ac22..63a5114 100644
--- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp
+++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
@@ -737,6 +737,10 @@
return false;
}
+ // Might have been destroyed by Annot_OnKillFocus().
+ if (!pFocusAnnot)
+ return false;
+
if (pFocusAnnot->GetAnnotSubtype() == CPDF_Annot::Subtype::WIDGET) {
CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pFocusAnnot.Get());
FormFieldType fieldType = pWidget->GetFieldType();
diff --git a/testing/resources/javascript/xfa_specific/bug_1109108.evt b/testing/resources/javascript/xfa_specific/bug_1109108.evt
new file mode 100644
index 0000000..7d58927
--- /dev/null
+++ b/testing/resources/javascript/xfa_specific/bug_1109108.evt
@@ -0,0 +1 @@
+keycode,9
diff --git a/testing/resources/javascript/xfa_specific/bug_1109108.in b/testing/resources/javascript/xfa_specific/bug_1109108.in
new file mode 100644
index 0000000..550112e
--- /dev/null
+++ b/testing/resources/javascript/xfa_specific/bug_1109108.in
@@ -0,0 +1,64 @@
+{{header}}
+{{include ../../xfa_catalog_1_0.fragment}}
+{{include ../../xfa_object_2_0.fragment}}
+{{include ../../xfa_preamble_3_0.fragment}}
+{{include ../../xfa_config_4_0.fragment}}
+{{object 5 0}} <<
+ {{streamlen}}
+>>
+stream
+<template>
+ <subform name="s1">
+ <subform name="s5">
+ <field name="field_28">
+ <calculate>
+ <script contentType="application/x-javascript">
+ var f=xfa.resolveNode("xfa.form.s1.s5.s6.field_35");
+ f.relevant ="print";
+ </script>
+ </calculate>
+ <ui>
+ <textEdit/>
+ </ui>
+ <value>
+ <text>CLGT.</text>
+ </value>
+ </field>
+ <subform name="s6">
+ <field name="field_35">
+ <ui>
+ <choiceList open="onEntry">
+ <border>
+ <edge/>
+ </border>
+ </choiceList>
+ </ui>
+ <items save="1">
+ <text>apples</text>
+ <text>bananas</text>
+ <text>pears</text>
+ </items>
+ <value>
+ <text>apples</text>
+ </value>
+ <event activity="change">
+ <script contentType="application/x-javascript">
+ xfa.template.remerge();
+ xfa.host.openList(this);
+ app.alert('remerged');
+ </script>
+ </event>
+ </field>
+ </subform>
+ </subform>
+ </subform>
+</template>
+endstream
+endobj
+{{include ../../xfa_locale_6_0.fragment}}
+{{include ../../xfa_postamble_7_0.fragment}}
+{{include ../../xfa_pages_8_0.fragment}}
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/javascript/xfa_specific/bug_1109108_expected.txt b/testing/resources/javascript/xfa_specific/bug_1109108_expected.txt
new file mode 100644
index 0000000..86c4845
--- /dev/null
+++ b/testing/resources/javascript/xfa_specific/bug_1109108_expected.txt
@@ -0,0 +1,2 @@
+Alert: remerged
+Alert: remerged
diff --git a/xfa/fxfa/cxfa_ffpageview.cpp b/xfa/fxfa/cxfa_ffpageview.cpp
index 996d8e5..47b5fdd 100644
--- a/xfa/fxfa/cxfa_ffpageview.cpp
+++ b/xfa/fxfa/cxfa_ffpageview.cpp
@@ -114,6 +114,30 @@
} // namespace
+class CXFA_TabParam {
+ public:
+ CXFA_TabParam() = default;
+ explicit CXFA_TabParam(CXFA_FFWidget* pWidget)
+ : m_pItem(pWidget->GetLayoutItem()) {}
+
+ ~CXFA_TabParam() = default;
+
+ CXFA_FFWidget* GetWidget() const { return m_pItem->GetFFWidget(); }
+ const std::vector<RetainPtr<CXFA_ContentLayoutItem>>& GetChildren() const {
+ return m_Children;
+ }
+ void ClearChildren() { m_Children.clear(); }
+ void AppendTabParam(CXFA_TabParam* pParam) {
+ m_Children.push_back(pParam->m_pItem);
+ m_Children.insert(m_Children.end(), pParam->m_Children.begin(),
+ pParam->m_Children.end());
+ }
+
+ private:
+ RetainPtr<CXFA_ContentLayoutItem> m_pItem;
+ std::vector<RetainPtr<CXFA_ContentLayoutItem>> m_Children;
+};
+
CXFA_FFPageView::CXFA_FFPageView(CXFA_FFDocView* pDocView, CXFA_Node* pPageArea)
: m_pPageArea(pPageArea), m_pDocView(pDocView) {}
@@ -225,16 +249,6 @@
return pWidget;
}
-void CXFA_TabParam::AppendTabParam(CXFA_TabParam* pParam) {
- m_Children.push_back(pParam->GetWidget());
- m_Children.insert(m_Children.end(), pParam->GetChildren().begin(),
- pParam->GetChildren().end());
-}
-
-void CXFA_TabParam::ClearChildren() {
- m_Children.clear();
-}
-
CXFA_FFTabOrderPageWidgetIterator::CXFA_FFTabOrderPageWidgetIterator(
CXFA_FFPageView* pPageView,
uint32_t dwFilter)
@@ -434,14 +448,14 @@
CXFA_FFTabOrderPageWidgetIterator::CreateSpaceOrderLayoutItems() {
std::vector<RetainPtr<CXFA_ContentLayoutItem>> items;
CXFA_LayoutItemIterator sIterator(m_pPageViewLayout.Get());
- auto pParam = std::make_unique<CXFA_TabParam>(nullptr);
+ auto pParam = std::make_unique<CXFA_TabParam>();
bool bCurrentItem = false;
bool bContentArea = false;
OrderContainer(&sIterator, nullptr, pParam.get(), &bCurrentItem,
&bContentArea, false);
items.reserve(pParam->GetChildren().size());
- for (CXFA_FFWidget* widget : pParam->GetChildren())
- items.emplace_back(widget->GetLayoutItem());
+ for (const auto& layout_item : pParam->GetChildren())
+ items.push_back(layout_item);
sIterator.Reset();
bCurrentItem = false;
@@ -449,8 +463,9 @@
pParam->ClearChildren();
OrderContainer(&sIterator, nullptr, pParam.get(), &bCurrentItem,
&bContentArea, true);
- for (CXFA_FFWidget* widget : pParam->GetChildren())
- items.emplace_back(widget->GetLayoutItem());
+ for (const auto& layout_item : pParam->GetChildren())
+ items.push_back(layout_item);
+
return items;
}
@@ -466,7 +481,3 @@
}
return pWidget;
}
-
-CXFA_TabParam::CXFA_TabParam(CXFA_FFWidget* pWidget) : m_pWidget(pWidget) {}
-
-CXFA_TabParam::~CXFA_TabParam() = default;
diff --git a/xfa/fxfa/cxfa_ffpageview.h b/xfa/fxfa/cxfa_ffpageview.h
index f2fab62..ed22572 100644
--- a/xfa/fxfa/cxfa_ffpageview.h
+++ b/xfa/fxfa/cxfa_ffpageview.h
@@ -19,6 +19,7 @@
class CXFA_FFWidget;
class CXFA_FFDocView;
+class CXFA_TabParam;
class CXFA_FFPageView : public Observable {
public:
@@ -65,21 +66,6 @@
const bool m_bIgnoreRelevant;
};
-class CXFA_TabParam {
- public:
- explicit CXFA_TabParam(CXFA_FFWidget* pWidget);
- ~CXFA_TabParam();
-
- void AppendTabParam(CXFA_TabParam* pParam);
- void ClearChildren();
- CXFA_FFWidget* GetWidget() const { return m_pWidget.Get(); }
- const std::vector<CXFA_FFWidget*>& GetChildren() const { return m_Children; }
-
- private:
- UnownedPtr<CXFA_FFWidget> const m_pWidget;
- std::vector<CXFA_FFWidget*> m_Children;
-};
-
class CXFA_FFTabOrderPageWidgetIterator final : public IXFA_WidgetIterator {
public:
CXFA_FFTabOrderPageWidgetIterator(CXFA_FFPageView* pPageView,
