Make public IndirectObjectHolder orphan tracking feature.
Use it to persist formfield /V and /RV objects.
Bug: chromium:901654
Change-Id: I22cabd64cbe0e91c6d9febb7fcdc070b4762ea10
Reviewed-on: https://pdfium-review.googlesource.com/c/45111
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
index 4c15a9e..32ba87f 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
@@ -90,9 +90,7 @@
return false;
pObj->SetObjNum(objnum);
- if (obj_holder)
- m_OrphanObjs.push_back(std::move(obj_holder));
-
+ AddOrphan(std::move(obj_holder));
obj_holder = std::move(pObj);
m_LastObjNum = std::max(m_LastObjNum, objnum);
return true;
@@ -105,3 +103,9 @@
m_IndirectObjs.erase(it);
}
+
+void CPDF_IndirectObjectHolder::AddOrphan(
+ std::unique_ptr<CPDF_Object> pObject) {
+ if (pObject)
+ m_OrphanObjs.push_back(std::move(pObject));
+}
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.h b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
index 57374b9..1d95db1 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
@@ -55,6 +55,11 @@
uint32_t objnum,
std::unique_ptr<CPDF_Object> pObj);
+ // Takes ownership of |pObj|, persist it for life of the indirect object
+ // holder (typically so that unowned pointers to it remain valid). No-op
+ // if |pObj| is NULL.
+ void AddOrphan(std::unique_ptr<CPDF_Object> pObj);
+
uint32_t GetLastObjNum() const { return m_LastObjNum; }
void SetLastObjNum(uint32_t objnum) { m_LastObjNum = objnum; }
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index 61cdf4c..0872317 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -235,12 +235,15 @@
if (!pClone)
return false;
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
m_pDict->SetFor("V", std::move(pClone));
- if (pRV)
+ if (pRV) {
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
m_pDict->SetFor("RV", pDV->Clone());
+ }
} else {
- m_pDict->RemoveFor("V");
- m_pDict->RemoveFor("RV");
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
}
if (notify == NotificationOption::kNotify)
NotifyAfterValueChange();
@@ -381,11 +384,15 @@
int iIndex = FindOptionValue(csValue);
if (iIndex < 0) {
ByteString bsEncodeText = PDF_EncodeText(csValue);
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor(key));
m_pDict->SetNewFor<CPDF_String>(key, bsEncodeText, false);
- if (m_Type == kRichText && !bDefault)
+ if (m_Type == kRichText && !bDefault) {
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
m_pDict->SetNewFor<CPDF_String>("RV", bsEncodeText, false);
- m_pDict->RemoveFor("I");
+ }
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
} else {
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor(key));
m_pDict->SetNewFor<CPDF_String>(key, PDF_EncodeText(csValue), false);
if (!bDefault) {
ClearSelection(NotificationOption::kDoNotNotify);
@@ -501,8 +508,8 @@
if (!NotifyListOrComboBoxBeforeChange(csValue))
return false;
}
- m_pDict->RemoveFor("V");
- m_pDict->RemoveFor("I");
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
if (notify == NotificationOption::kNotify)
NotifyListOrComboBoxAfterChange();
return true;
@@ -589,7 +596,7 @@
SelectOption(index, false, NotificationOption::kDoNotNotify);
if (pValue->IsString()) {
if (pValue->GetUnicodeText() == opt_value)
- m_pDict->RemoveFor("V");
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
} else if (pValue->IsArray()) {
auto pArray = pdfium::MakeUnique<CPDF_Array>();
for (int i = 0; i < CountOptions(); i++) {
@@ -598,12 +605,14 @@
pArray->AddNew<CPDF_String>(PDF_EncodeText(opt_value), false);
}
}
- if (pArray->size() > 0)
+ if (pArray->size() > 0) {
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
m_pDict->SetFor("V", std::move(pArray));
+ }
}
} else {
- m_pDict->RemoveFor("V");
- m_pDict->RemoveFor("I");
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
}
}
}
@@ -850,7 +859,7 @@
if (bSelected)
pArray->AddNew<CPDF_Number>(iOptIndex);
if (pArray->IsEmpty())
- m_pDict->RemoveFor("I");
+ m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
}
if (notify == NotificationOption::kNotify)
NotifyListOrComboBoxAfterChange();
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index da0adb2..4180bd7 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -404,6 +404,25 @@
UnloadPage(page);
}
+TEST_F(FPDFFormFillEmbeddertest, BUG_901654) {
+ EmbedderTestTimerHandlingDelegate delegate;
+ SetDelegate(&delegate);
+
+ EXPECT_TRUE(OpenDocument("bug_901654.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+ DoOpenActions();
+ delegate.AdvanceTime(4000);
+
+ // Simulate a repaint.
+ {
+ ScopedFPDFBitmap bitmap(FPDFBitmap_Create(512, 512, 0));
+ FPDF_RenderPageBitmap_Start(bitmap.get(), page, 0, 0, 512, 512, 0, 0,
+ nullptr);
+ }
+ UnloadPage(page);
+}
+
class DoURIActionBlockedDelegate final : public EmbedderTest::Delegate {
public:
void DoURIAction(FPDF_BYTESTRING uri) override {
diff --git a/testing/resources/bug_901654.in b/testing/resources/bug_901654.in
new file mode 100644
index 0000000..703e354
--- /dev/null
+++ b/testing/resources/bug_901654.in
@@ -0,0 +1,73 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /OpenAction 8 0 R
+>>
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 600 700]
+ /Resources 5 0 R
+ /Contents 4 0 R
+ /Annots [6 0 R]
+>>
+endobj
+{{object 4 0}} <<
+ >>
+stream
+ /V
+ scn
+ BT
+ /F1 20 Tf
+ 50 50 Td
+ (Test) Tj
+ ET
+endstream
+endobj
+{{object 5 0}} <<
+ /Pattern 6 0 R
+ /Font <<F1 7 0 R>>
+ >>
+endobj
+{{object 6 0}} <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txt1)
+ /F 4
+ /Rect [200 200 400 400]
+ /V <</PatternType 2 /Shading <</Test (a)>> >>
+>>
+endobj
+{{object 7 0}} <<
+ /Type /Font
+ /Subtype /Type1
+ /BaseFont /Helvetica
+>>
+endobj
+{{object 8 0}} <<
+ /Type /Action
+ /S /JavaScript
+ /JS 9 0 R
+>>
+endobj
+{{object 9 0}} <<>>
+stream
+function run()
+{
+ this.getField('txt1').value='a';
+}
+app.setTimeOut('run()',3000);
+endstream
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_901654.pdf b/testing/resources/bug_901654.pdf
new file mode 100644
index 0000000..a98fe8b
--- /dev/null
+++ b/testing/resources/bug_901654.pdf
@@ -0,0 +1,89 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /OpenAction 8 0 R
+>>
+2 0 obj <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+3 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 600 700]
+ /Resources 5 0 R
+ /Contents 4 0 R
+ /Annots [6 0 R]
+>>
+endobj
+4 0 obj <<
+ >>
+stream
+ /V
+ scn
+ BT
+ /F1 20 Tf
+ 50 50 Td
+ (Test) Tj
+ ET
+endstream
+endobj
+5 0 obj <<
+ /Pattern 6 0 R
+ /Font <<F1 7 0 R>>
+ >>
+endobj
+6 0 obj <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txt1)
+ /F 4
+ /Rect [200 200 400 400]
+ /V <</PatternType 2 /Shading <</Test (a)>> >>
+>>
+endobj
+7 0 obj <<
+ /Type /Font
+ /Subtype /Type1
+ /BaseFont /Helvetica
+>>
+endobj
+8 0 obj <<
+ /Type /Action
+ /S /JavaScript
+ /JS 9 0 R
+>>
+endobj
+9 0 obj <<>>
+stream
+function run()
+{
+ this.getField('txt1').value='a';
+}
+app.setTimeOut('run()',3000);
+endstream
+endobj
+xref
+0 10
+0000000000 65535 f
+0000000015 00000 n
+0000000081 00000 n
+0000000144 00000 n
+0000000276 00000 n
+0000000375 00000 n
+0000000438 00000 n
+0000000596 00000 n
+0000000672 00000 n
+0000000738 00000 n
+trailer <<
+ /Root 1 0 R
+ /Size 10
+>>
+startxref
+859
+%%EOF
