Mark some dangerious pwl methods [[nodiscard]]
These may have destroyed |this| if false is returned. Logic changes
indicate there may be places where pointers dangle as a result. Null
them as possible while pondering other remedies.
Change-Id: Iea8d5dbec587fbec444ee73f765150cc83751094
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/107713
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/pwl/cpwl_caret.cpp b/fpdfsdk/pwl/cpwl_caret.cpp
index fe29388..47f7864 100644
--- a/fpdfsdk/pwl/cpwl_caret.cpp
+++ b/fpdfsdk/pwl/cpwl_caret.cpp
@@ -76,7 +76,7 @@
return;
m_pTimer.reset();
- CPWL_Wnd::SetVisible(false);
+ (void)CPWL_Wnd::SetVisible(false);
// Note, |this| may no longer be viable at this point. If more work needs
// to be done, check the return value of SetVisible().
return;
diff --git a/fpdfsdk/pwl/cpwl_combo_box.cpp b/fpdfsdk/pwl/cpwl_combo_box.cpp
index e3a144b..a691db1 100644
--- a/fpdfsdk/pwl/cpwl_combo_box.cpp
+++ b/fpdfsdk/pwl/cpwl_combo_box.cpp
@@ -277,7 +277,10 @@
}
if (m_pList) {
- m_pList->SetVisible(false);
+ if (!m_pList->SetVisible(false)) {
+ m_pList = nullptr; // Gone, dangling even.
+ return false;
+ }
if (!this_observed) {
return false;
}
diff --git a/fpdfsdk/pwl/cpwl_edit.cpp b/fpdfsdk/pwl/cpwl_edit.cpp
index c3803bc..f079a2f 100644
--- a/fpdfsdk/pwl/cpwl_edit.cpp
+++ b/fpdfsdk/pwl/cpwl_edit.cpp
@@ -238,12 +238,15 @@
ObservedPtr<CPWL_Edit> observed_ptr(this);
CPWL_ScrollBar* pScroll = GetVScrollBar();
if (pScroll && pScroll->IsVisible()) {
- pScroll->SetVisible(false);
- if (!observed_ptr)
+ if (!pScroll->SetVisible(false)) {
return;
-
- if (!Move(m_rcOldWindow, true, true))
+ }
+ if (!observed_ptr) {
return;
+ }
+ if (!Move(m_rcOldWindow, true, true)) {
+ return;
+ }
}
m_pEditImpl->SelectNone();
diff --git a/fpdfsdk/pwl/cpwl_edit_impl.cpp b/fpdfsdk/pwl/cpwl_edit_impl.cpp
index 50e2c42..37ec306 100644
--- a/fpdfsdk/pwl/cpwl_edit_impl.cpp
+++ b/fpdfsdk/pwl/cpwl_edit_impl.cpp
@@ -1232,9 +1232,12 @@
if (!m_bNotifyFlag) {
AutoRestorer<bool> restorer(&m_bNotifyFlag);
m_bNotifyFlag = true;
- if (std::vector<CFX_FloatRect>* pRects = m_Refresh.GetRefreshRects()) {
- for (auto& rect : *pRects)
- m_pNotify->InvalidateRect(&rect);
+ std::vector<CFX_FloatRect>* pRects = m_Refresh.GetRefreshRects();
+ for (auto& rect : *pRects) {
+ if (!m_pNotify->InvalidateRect(&rect)) {
+ m_pNotify = nullptr; // Gone, dangling even.
+ break;
+ }
}
}
}
@@ -1302,7 +1305,9 @@
AutoRestorer<bool> restorer(&m_bNotifyFlag);
m_bNotifyFlag = true;
CFX_FloatRect rcRefresh = VTToEdit(rcWord);
- m_pNotify->InvalidateRect(&rcRefresh);
+ if (!m_pNotify->InvalidateRect(&rcRefresh)) {
+ m_pNotify = nullptr; // Gone, dangling even.
+ }
}
}
} else {
@@ -1316,7 +1321,9 @@
AutoRestorer<bool> restorer(&m_bNotifyFlag);
m_bNotifyFlag = true;
CFX_FloatRect rcRefresh = VTToEdit(rcLine);
- m_pNotify->InvalidateRect(&rcRefresh);
+ if (!m_pNotify->InvalidateRect(&rcRefresh)) {
+ m_pNotify = nullptr; // Gone, dangling even.
+ }
}
}
diff --git a/fpdfsdk/pwl/cpwl_list_box.cpp b/fpdfsdk/pwl/cpwl_list_box.cpp
index e4874d5..5adb004 100644
--- a/fpdfsdk/pwl/cpwl_list_box.cpp
+++ b/fpdfsdk/pwl/cpwl_list_box.cpp
@@ -271,15 +271,13 @@
Info.fContentMax - Info.fContentMin) ||
FXSYS_IsFloatEqual(Info.fPlateWidth,
Info.fContentMax - Info.fContentMin)) {
- if (pScroll->IsVisible()) {
- pScroll->SetVisible(false);
+ if (pScroll->IsVisible() && pScroll->SetVisible(false)) {
RePosChildWnd();
}
- } else {
- if (!pScroll->IsVisible()) {
- pScroll->SetVisible(true);
- RePosChildWnd();
- }
+ return;
+ }
+ if (!pScroll->IsVisible() && pScroll->SetVisible(true)) {
+ RePosChildWnd();
}
}
@@ -287,8 +285,8 @@
SetScrollPosition(fy);
}
-void CPWL_ListBox::OnInvalidateRect(const CFX_FloatRect& rect) {
- InvalidateRect(&rect);
+bool CPWL_ListBox::OnInvalidateRect(const CFX_FloatRect& rect) {
+ return InvalidateRect(&rect);
}
void CPWL_ListBox::Select(int32_t nItemIndex) {
diff --git a/fpdfsdk/pwl/cpwl_list_box.h b/fpdfsdk/pwl/cpwl_list_box.h
index 49d9363..5ee56b0 100644
--- a/fpdfsdk/pwl/cpwl_list_box.h
+++ b/fpdfsdk/pwl/cpwl_list_box.h
@@ -52,7 +52,7 @@
float fSmallStep,
float fBigStep) override;
void OnSetScrollPosY(float fy) override;
- void OnInvalidateRect(const CFX_FloatRect& pRect) override;
+ [[nodiscard]] bool OnInvalidateRect(const CFX_FloatRect& pRect) override;
bool OnNotifySelectionChanged(bool bKeyDown, Mask<FWL_EVENTFLAG> nFlag);
diff --git a/fpdfsdk/pwl/cpwl_list_ctrl.cpp b/fpdfsdk/pwl/cpwl_list_ctrl.cpp
index 046e772..d7e3b15 100644
--- a/fpdfsdk/pwl/cpwl_list_ctrl.cpp
+++ b/fpdfsdk/pwl/cpwl_list_ctrl.cpp
@@ -356,26 +356,30 @@
}
void CPWL_ListCtrl::InvalidateItem(int32_t nItemIndex) {
- if (m_pNotify) {
- if (nItemIndex == -1) {
- if (!m_bNotifyFlag) {
- m_bNotifyFlag = true;
- CFX_FloatRect rcRefresh = m_rcPlate;
- m_pNotify->OnInvalidateRect(rcRefresh);
- m_bNotifyFlag = false;
+ if (!m_pNotify) {
+ return;
+ }
+ if (nItemIndex == -1) {
+ if (!m_bNotifyFlag) {
+ m_bNotifyFlag = true;
+ CFX_FloatRect rcRefresh = m_rcPlate;
+ if (!m_pNotify->OnInvalidateRect(rcRefresh)) {
+ m_pNotify = nullptr; // Gone, dangling even.
}
- } else {
- if (!m_bNotifyFlag) {
- m_bNotifyFlag = true;
- CFX_FloatRect rcRefresh = GetItemRect(nItemIndex);
- rcRefresh.left -= 1.0f;
- rcRefresh.right += 1.0f;
- rcRefresh.bottom -= 1.0f;
- rcRefresh.top += 1.0f;
-
- m_pNotify->OnInvalidateRect(rcRefresh);
- m_bNotifyFlag = false;
+ m_bNotifyFlag = false;
+ }
+ } else {
+ if (!m_bNotifyFlag) {
+ m_bNotifyFlag = true;
+ CFX_FloatRect rcRefresh = GetItemRect(nItemIndex);
+ rcRefresh.left -= 1.0f;
+ rcRefresh.right += 1.0f;
+ rcRefresh.bottom -= 1.0f;
+ rcRefresh.top += 1.0f;
+ if (!m_pNotify->OnInvalidateRect(rcRefresh)) {
+ m_pNotify = nullptr; // Gone, dangling even.
}
+ m_bNotifyFlag = false;
}
}
}
diff --git a/fpdfsdk/pwl/cpwl_list_ctrl.h b/fpdfsdk/pwl/cpwl_list_ctrl.h
index 3f36354..c278b81 100644
--- a/fpdfsdk/pwl/cpwl_list_ctrl.h
+++ b/fpdfsdk/pwl/cpwl_list_ctrl.h
@@ -31,7 +31,9 @@
float fSmallStep,
float fBigStep) = 0;
virtual void OnSetScrollPosY(float fy) = 0;
- virtual void OnInvalidateRect(const CFX_FloatRect& rect) = 0;
+
+ // Returns true if `this` is still allocated.
+ [[nodiscard]] virtual bool OnInvalidateRect(const CFX_FloatRect& rect) = 0;
};
CPWL_ListCtrl();
diff --git a/fpdfsdk/pwl/cpwl_scroll_bar.cpp b/fpdfsdk/pwl/cpwl_scroll_bar.cpp
index 1483191..da98ee9 100644
--- a/fpdfsdk/pwl/cpwl_scroll_bar.cpp
+++ b/fpdfsdk/pwl/cpwl_scroll_bar.cpp
@@ -338,7 +338,7 @@
m_sData.SetClientWidth(fClientWidth);
if (FXSYS_IsFloatSmaller(m_sData.ScrollRange.GetWidth(), 0.0f)) {
- m_pPosButton->SetVisible(false);
+ (void)m_pPosButton->SetVisible(false);
// Note, |this| may no longer be viable at this point. If more work needs
// to be done, check this_observed.
return;
@@ -348,7 +348,7 @@
return;
}
- MovePosButton(true);
+ (void)MovePosButton(true);
// Note, |this| may no longer be viable at this point. If more work needs
// to be done, check the return value of MovePosButton().
}
@@ -357,7 +357,7 @@
float fOldPos = m_sData.fScrollPos;
m_sData.SetPos(fPos);
if (!FXSYS_IsFloatEqual(m_sData.fScrollPos, fOldPos)) {
- MovePosButton(true);
+ (void)MovePosButton(true);
// Note, |this| may no longer be viable at this point. If more work needs
// to be done, check the return value of MovePosButton().
}
diff --git a/fpdfsdk/pwl/cpwl_scroll_bar.h b/fpdfsdk/pwl/cpwl_scroll_bar.h
index 6cca8fb..91e7c29 100644
--- a/fpdfsdk/pwl/cpwl_scroll_bar.h
+++ b/fpdfsdk/pwl/cpwl_scroll_bar.h
@@ -124,7 +124,7 @@
void SetScrollPos(float fPos);
// Returns |true| iff this instance is still allocated.
- bool MovePosButton(bool bRefresh);
+ [[nodiscard]] bool MovePosButton(bool bRefresh);
void SetScrollStep(float fBigStep, float fSmallStep);
void NotifyScrollWindow();
CFX_FloatRect GetScrollArea() const;
diff --git a/fpdfsdk/pwl/cpwl_wnd.cpp b/fpdfsdk/pwl/cpwl_wnd.cpp
index 2716fde..8d14656 100644
--- a/fpdfsdk/pwl/cpwl_wnd.cpp
+++ b/fpdfsdk/pwl/cpwl_wnd.cpp
@@ -557,7 +557,9 @@
ObservedPtr<CPWL_Wnd> this_observed(this);
for (const auto& pChild : m_Children) {
- pChild->SetVisible(bVisible);
+ if (!pChild->SetVisible(bVisible)) {
+ return false;
+ }
if (!this_observed) {
return false;
}
diff --git a/fpdfsdk/pwl/cpwl_wnd.h b/fpdfsdk/pwl/cpwl_wnd.h
index 752fed1..eb22934 100644
--- a/fpdfsdk/pwl/cpwl_wnd.h
+++ b/fpdfsdk/pwl/cpwl_wnd.h
@@ -138,7 +138,7 @@
virtual ~CPWL_Wnd();
// Returns |true| iff this instance is still allocated.
- virtual bool InvalidateRect(const CFX_FloatRect* pRect);
+ [[nodiscard]] virtual bool InvalidateRect(const CFX_FloatRect* pRect);
virtual bool OnKeyDown(FWL_VKEYCODE nKeyCode, Mask<FWL_EVENTFLAG> nFlag);
virtual bool OnChar(uint16_t nChar, Mask<FWL_EVENTFLAG> nFlag);
@@ -165,7 +165,7 @@
virtual void SetCursor();
// Returns |true| iff this instance is still allocated.
- virtual bool SetVisible(bool bVisible);
+ [[nodiscard]] virtual bool SetVisible(bool bVisible);
virtual void SetFontSize(float fFontSize);
virtual float GetFontSize() const;
@@ -224,7 +224,7 @@
virtual void CreateChildWnd(const CreateParams& cp);
// Returns |true| iff this instance is still allocated.
- virtual bool RePosChildWnd();
+ [[nodiscard]] virtual bool RePosChildWnd();
virtual void DrawThisAppearance(CFX_RenderDevice* pDevice,
const CFX_Matrix& mtUser2Device);
@@ -248,8 +248,8 @@
CPWL_ScrollBar* GetVScrollBar() const;
// Returns |true| iff this instance is still allocated.
- bool InvalidateRectMove(const CFX_FloatRect& rcOld,
- const CFX_FloatRect& rcNew);
+ [[nodiscard]] bool InvalidateRectMove(const CFX_FloatRect& rcOld,
+ const CFX_FloatRect& rcNew);
void SetCapture();
void ReleaseCapture();
