[Merge to 54] Fixup CPDFSDK_PageView and CPDF_Page interactions.
There are several issues when CPDFSDK_PageView and CPDF_Page interact,
especially around deletion. This Cl fixes up several places where things
go wrong working with these objects.
BUG=chromium:632709
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/2340523002 .
Review-Url: https://codereview.chromium.org/2319663002
diff --git a/fpdfsdk/fpdfview.cpp b/fpdfsdk/fpdfview.cpp
index dee71ac..1c0af1d 100644
--- a/fpdfsdk/fpdfview.cpp
+++ b/fpdfsdk/fpdfview.cpp
@@ -673,9 +673,20 @@
#else // PDF_ENABLE_XFA
CPDFSDK_PageView* pPageView =
static_cast<CPDFSDK_PageView*>(pPage->GetView());
- if (pPageView && pPageView->IsLocked()) {
- pPageView->TakeOverPage();
- return;
+ if (pPageView) {
+ if (pPageView->IsLocked()) {
+ pPageView->TakePageOwnership();
+ return;
+ }
+
+ bool owned = pPageView->OwnsPage();
+ // This will delete the |pPageView| object. We must cleanup the PageView
+ // first because it will attempt to reset the View on the |pPage| during
+ // destruction.
+ pPageView->GetSDKDocument()->RemovePageView(pPage);
+ // If the page was owned then the pageview will have deleted the page.
+ if (owned)
+ return;
}
delete pPage;
#endif // PDF_ENABLE_XFA
diff --git a/fpdfsdk/fsdk_mgr.cpp b/fpdfsdk/fsdk_mgr.cpp
index 9017e20..1011b92 100644
--- a/fpdfsdk/fsdk_mgr.cpp
+++ b/fpdfsdk/fsdk_mgr.cpp
@@ -299,7 +299,7 @@
return nullptr;
auto it = m_pageMap.find(pTempPage);
- return it->second;
+ return it != m_pageMap.end() ? it->second : nullptr;
}
void CPDFSDK_Document::ProcJavascriptFun() {
@@ -482,7 +482,7 @@
m_pSDKDoc(pSDKDoc),
m_CaptureWidget(nullptr),
#ifndef PDF_ENABLE_XFA
- m_bTakeOverPage(FALSE),
+ m_bOwnsPage(false),
#endif // PDF_ENABLE_XFA
m_bEnterWidget(FALSE),
m_bExitWidget(FALSE),
@@ -505,6 +505,13 @@
}
CPDFSDK_PageView::~CPDFSDK_PageView() {
+#ifndef PDF_ENABLE_XFA
+ // The call to |ReleaseAnnot| can cause the page pointed to by |m_page| to
+ // be freed, which will cause issues if we try to cleanup the pageview pointer
+ // in |m_page|. So, reset the pageview pointer before doing anything else.
+ m_page->SetView(nullptr);
+#endif // PDF_ENABLE_XFA
+
CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray)
@@ -512,11 +519,10 @@
m_fxAnnotArray.clear();
m_pAnnotList.reset();
+
#ifndef PDF_ENABLE_XFA
- m_page->SetView(nullptr);
- if (m_bTakeOverPage) {
+ if (m_bOwnsPage)
delete m_page;
- }
#endif // PDF_ENABLE_XFA
}
diff --git a/fpdfsdk/include/fsdk_mgr.h b/fpdfsdk/include/fsdk_mgr.h
index a0adf04..cf86e38 100644
--- a/fpdfsdk/include/fsdk_mgr.h
+++ b/fpdfsdk/include/fsdk_mgr.h
@@ -607,7 +607,8 @@
void SetLock(FX_BOOL bLocked) { m_bLocked = bLocked; }
FX_BOOL IsLocked() { return m_bLocked; }
#ifndef PDF_ENABLE_XFA
- void TakeOverPage() { m_bTakeOverPage = TRUE; }
+ bool OwnsPage() const { return m_bOwnsPage; }
+ void TakePageOwnership() { m_bOwnsPage = true; }
#endif // PDF_ENABLE_XFA
private:
@@ -622,7 +623,7 @@
CPDFSDK_Annot* m_CaptureWidget;
#else // PDF_ENABLE_XFA
CPDFSDK_Widget* m_CaptureWidget;
- FX_BOOL m_bTakeOverPage;
+ bool m_bOwnsPage;
#endif // PDF_ENABLE_XFA
FX_BOOL m_bEnterWidget;
FX_BOOL m_bExitWidget;
diff --git a/fpdfsdk/javascript/Field.cpp b/fpdfsdk/javascript/Field.cpp
index 5c279d6..0a184d8 100644
--- a/fpdfsdk/javascript/Field.cpp
+++ b/fpdfsdk/javascript/Field.cpp
@@ -260,11 +260,12 @@
FX_BOOL bChangeMark,
FX_BOOL bResetAP,
FX_BOOL bRefresh) {
- std::vector<CPDFSDK_Widget*> widgets;
CPDFSDK_InterForm* pInterForm = pDocument->GetInterForm();
- pInterForm->GetWidgets(pFormField, &widgets);
if (bResetAP) {
+ std::vector<CPDFSDK_Widget*> widgets;
+ pInterForm->GetWidgets(pFormField, &widgets);
+
int nFieldType = pFormField->GetFieldType();
if (nFieldType == FIELDTYPE_COMBOBOX || nFieldType == FIELDTYPE_TEXTFIELD) {
for (CPDFSDK_Widget* pWidget : widgets) {
@@ -284,6 +285,12 @@
}
if (bRefresh) {
+ // Refresh the widget list. The calls in |bResetAP| may have caused widgets
+ // to be removed from the list. We need to call |GetWidgets| again to be
+ // sure none of the widgets have been deleted.
+ std::vector<CPDFSDK_Widget*> widgets;
+ pInterForm->GetWidgets(pFormField, &widgets);
+
for (CPDFSDK_Widget* pWidget : widgets) {
CPDFSDK_Document* pDoc = pWidget->GetInterForm()->GetDocument();
pDoc->UpdateAllViews(nullptr, pWidget);
