From 55469aed5acffcce3259d37418ba9e8b8e60d801 Mon Sep 17 00:00:00 2001 From: Henrique Nakashima Date: Wed, 4 Oct 2017 11:08:45 -0400 Subject: Fix UAF in SetVisible(). SetVisible() may be called during Destroy() which may be called during SetVisible(). This fixes the latest in a family of bugs that happen after an instance is freed by code triggered by JS code while it's executing a method. The CL has a lot of protection for many of these points where JS may be executed and potentially destroy objects. The return types of many methods that may execute JS have been changed to bool, indicating whether the instance is still alive after the call. Bug: chromium:770148 Change-Id: If5a9db4d8d6aac10f4dd6b645922bb96c116684d Reviewed-on: https://pdfium-review.googlesource.com/15190 Reviewed-by: dsinclair Commit-Queue: Henrique Nakashima --- fpdfsdk/pwl/cpwl_appstream.cpp | 3 +- fpdfsdk/pwl/cpwl_caret.cpp | 25 +++++------ fpdfsdk/pwl/cpwl_caret.h | 4 +- fpdfsdk/pwl/cpwl_combo_box.cpp | 83 ++++++++++++++++++++++++++--------- fpdfsdk/pwl/cpwl_combo_box.h | 6 ++- fpdfsdk/pwl/cpwl_edit.cpp | 21 +++++---- fpdfsdk/pwl/cpwl_edit.h | 2 +- fpdfsdk/pwl/cpwl_edit_ctrl.cpp | 27 ++++++++---- fpdfsdk/pwl/cpwl_edit_ctrl.h | 4 +- fpdfsdk/pwl/cpwl_list_box.cpp | 6 ++- fpdfsdk/pwl/cpwl_list_box.h | 2 +- fpdfsdk/pwl/cpwl_scroll_bar.cpp | 97 ++++++++++++++++++++++++++++++----------- fpdfsdk/pwl/cpwl_scroll_bar.h | 6 ++- fpdfsdk/pwl/cpwl_wnd.cpp | 63 ++++++++++++++++++-------- fpdfsdk/pwl/cpwl_wnd.h | 17 +++++--- 15 files changed, 251 insertions(+), 115 deletions(-) (limited to 'fpdfsdk/pwl') diff --git a/fpdfsdk/pwl/cpwl_appstream.cpp b/fpdfsdk/pwl/cpwl_appstream.cpp index 6a19efb906..7d7f172f2f 100644 --- a/fpdfsdk/pwl/cpwl_appstream.cpp +++ b/fpdfsdk/pwl/cpwl_appstream.cpp @@ -696,7 +696,8 @@ ByteString GenerateIconAppStream(CPDF_IconFit& fit, icon.Create(cp); icon.SetIconFit(&fit); icon.SetPDFStream(pIconStream); - icon.Move(rcIcon, false, false); + if (!icon.Move(rcIcon, false, false)) + return ByteString(); ByteString sAlias = icon.GetImageAlias(); if (sAlias.GetLength() <= 0) diff --git a/fpdfsdk/pwl/cpwl_caret.cpp b/fpdfsdk/pwl/cpwl_caret.cpp index c58f019c4c..77a768afe0 100644 --- a/fpdfsdk/pwl/cpwl_caret.cpp +++ b/fpdfsdk/pwl/cpwl_caret.cpp @@ -83,7 +83,7 @@ void CPWL_Caret::SetCaret(bool bVisible, EndTimer(); CPWL_Wnd::SetVisible(false); // Note, |this| may no longer be viable at this point. If more work needs - // to be done, add an observer. + // to be done, check the return value of SetVisible(). return; } @@ -93,15 +93,13 @@ void CPWL_Caret::SetCaret(bool bVisible, EndTimer(); BeginTimer(PWL_CARET_FLASHINTERVAL); - ObservedPtr observer(this); - CPWL_Wnd::SetVisible(true); - if (!observer) + if (!CPWL_Wnd::SetVisible(true)) return; m_bFlash = true; Move(m_rcInvalid, false, true); // Note, |this| may no longer be viable at this point. If more work needs - // to be done, add an observer. + // to be done, check the return value of Move(). return; } @@ -113,15 +111,12 @@ void CPWL_Caret::SetCaret(bool bVisible, m_bFlash = true; Move(m_rcInvalid, false, true); // Note, |this| may no longer be viable at this point. If more work - // needs to be done, add an observer. + // needs to be done, check the return value of Move(). } -void CPWL_Caret::InvalidateRect(CFX_FloatRect* pRect) { +bool CPWL_Caret::InvalidateRect(CFX_FloatRect* pRect) { if (!pRect) { - CPWL_Wnd::InvalidateRect(nullptr); - // Note, |this| may no longer be viable at this point. If more work needs - // to be done, add an observer. - return; + return CPWL_Wnd::InvalidateRect(nullptr); } CFX_FloatRect rcRefresh = *pRect; @@ -131,7 +126,9 @@ void CPWL_Caret::InvalidateRect(CFX_FloatRect* pRect) { } rcRefresh.top += 1; rcRefresh.bottom -= 1; - CPWL_Wnd::InvalidateRect(&rcRefresh); - // Note, |this| may no longer be viable at this point. If more work needs - // to be done, add an observer. + return CPWL_Wnd::InvalidateRect(&rcRefresh); +} + +bool CPWL_Caret::SetVisible(bool bVisible) { + return true; } diff --git a/fpdfsdk/pwl/cpwl_caret.h b/fpdfsdk/pwl/cpwl_caret.h index fb0fa8978c..d60a96419b 100644 --- a/fpdfsdk/pwl/cpwl_caret.h +++ b/fpdfsdk/pwl/cpwl_caret.h @@ -18,8 +18,8 @@ class CPWL_Caret : public CPWL_Wnd { ByteString GetClassName() const override; void DrawThisAppearance(CFX_RenderDevice* pDevice, const CFX_Matrix& mtUser2Device) override; - void InvalidateRect(CFX_FloatRect* pRect) override; - void SetVisible(bool bVisible) override {} + bool InvalidateRect(CFX_FloatRect* pRect) override; + bool SetVisible(bool bVisible) override; void TimerProc() override; void SetCaret(bool bVisible, diff --git a/fpdfsdk/pwl/cpwl_combo_box.cpp b/fpdfsdk/pwl/cpwl_combo_box.cpp index 3ebccca263..92b01658f2 100644 --- a/fpdfsdk/pwl/cpwl_combo_box.cpp +++ b/fpdfsdk/pwl/cpwl_combo_box.cpp @@ -174,7 +174,9 @@ void CPWL_ComboBox::SetFocus() { } void CPWL_ComboBox::KillFocus() { - SetPopup(false); + if (!SetPopup(false)) + return; + CPWL_Wnd::KillFocus(); } @@ -316,7 +318,9 @@ void CPWL_ComboBox::CreateListBox(const CreateParams& cp) { m_pList->Create(lcp); } -void CPWL_ComboBox::RePosChildWnd() { +bool CPWL_ComboBox::RePosChildWnd() { + ObservedPtr thisObserved(this); + const CFX_FloatRect rcClient = GetClientRect(); if (m_bPopup) { const float fOldWindowHeight = m_rcOldWindow.Height(); @@ -338,34 +342,57 @@ void CPWL_ComboBox::RePosChildWnd() { rcList.bottom += fOldWindowHeight; } - if (m_pButton) + if (m_pButton) { m_pButton->Move(rcButton, true, false); + if (!thisObserved) + return false; + } - if (m_pEdit) + if (m_pEdit) { m_pEdit->Move(rcEdit, true, false); + if (!thisObserved) + return false; + } if (m_pList) { - m_pList->SetVisible(true); - m_pList->Move(rcList, true, false); + if (!m_pList->SetVisible(true) || !thisObserved) + return false; + + if (!m_pList->Move(rcList, true, false) || !thisObserved) + return false; + m_pList->ScrollToListItem(m_nSelectItem); + if (!thisObserved) + return false; } - return; + return true; } CFX_FloatRect rcButton = rcClient; rcButton.left = std::max(rcButton.right - kDefaultButtonWidth, rcClient.left); - if (m_pButton) + if (m_pButton) { m_pButton->Move(rcButton, true, false); + if (!thisObserved) + return false; + } CFX_FloatRect rcEdit = rcClient; rcEdit.right = std::max(rcButton.left - 1.0f, rcEdit.left); - if (m_pEdit) + if (m_pEdit) { m_pEdit->Move(rcEdit, true, false); + if (!thisObserved) + return false; + } - if (m_pList) + if (m_pList) { m_pList->SetVisible(false); + if (!thisObserved) + return false; + } + + return true; } void CPWL_ComboBox::SelectAll() { @@ -377,27 +404,30 @@ CFX_FloatRect CPWL_ComboBox::GetFocusRect() const { return CFX_FloatRect(); } -void CPWL_ComboBox::SetPopup(bool bPopup) { +bool CPWL_ComboBox::SetPopup(bool bPopup) { if (!m_pList) - return; + return true; if (bPopup == m_bPopup) - return; + return true; float fListHeight = m_pList->GetContentRect().Height(); if (!IsFloatBigger(fListHeight, 0.0f)) - return; + return true; if (!bPopup) { m_bPopup = bPopup; - Move(m_rcOldWindow, true, true); - return; + return Move(m_rcOldWindow, true, true); } if (!m_pFillerNotify) - return; + return true; + + ObservedPtr thisObserved(this); #ifdef PDF_ENABLE_XFA if (m_pFillerNotify->OnPopupPreOpen(GetAttachedData(), 0)) - return; + return !!thisObserved; + if (!thisObserved) + return false; #endif // PDF_ENABLE_XFA float fBorderWidth = m_pList->GetBorderWidth() * 2; @@ -411,7 +441,7 @@ void CPWL_ComboBox::SetPopup(bool bPopup) { m_pFillerNotify->QueryWherePopup(GetAttachedData(), fPopupMin, fPopupMax, &bBottom, &fPopupRet); if (!IsFloatBigger(fPopupRet, 0.0f)) - return; + return true; m_rcOldWindow = CPWL_Wnd::GetWindowRect(); m_bPopup = bPopup; @@ -423,10 +453,16 @@ void CPWL_ComboBox::SetPopup(bool bPopup) { else rcWindow.top += fPopupRet; - Move(rcWindow, true, true); + if (!Move(rcWindow, true, true)) + return false; + #ifdef PDF_ENABLE_XFA m_pFillerNotify->OnPopupPostOpen(GetAttachedData(), 0); + if (!thisObserved) + return false; #endif // PDF_ENABLE_XFA + + return !!thisObserved; } bool CPWL_ComboBox::OnKeyDown(uint16_t nChar, uint32_t nFlag) { @@ -505,8 +541,11 @@ bool CPWL_ComboBox::OnChar(uint16_t nChar, uint32_t nFlag) { } void CPWL_ComboBox::NotifyLButtonDown(CPWL_Wnd* child, const CFX_PointF& pos) { - if (child == m_pButton) + if (child == m_pButton) { SetPopup(!m_bPopup); + // Note, |this| may no longer be viable at this point. If more work needs to + // be done, check the return value of SetPopup(). + } } void CPWL_ComboBox::NotifyLButtonUp(CPWL_Wnd* child, const CFX_PointF& pos) { @@ -517,6 +556,8 @@ void CPWL_ComboBox::NotifyLButtonUp(CPWL_Wnd* child, const CFX_PointF& pos) { SelectAll(); m_pEdit->SetFocus(); SetPopup(false); + // Note, |this| may no longer be viable at this point. If more work needs to + // be done, check the return value of SetPopup(). } bool CPWL_ComboBox::IsPopup() const { diff --git a/fpdfsdk/pwl/cpwl_combo_box.h b/fpdfsdk/pwl/cpwl_combo_box.h index 20744df169..6db721369b 100644 --- a/fpdfsdk/pwl/cpwl_combo_box.h +++ b/fpdfsdk/pwl/cpwl_combo_box.h @@ -56,7 +56,7 @@ class CPWL_ComboBox : public CPWL_Wnd { void NotifyLButtonDown(CPWL_Wnd* child, const CFX_PointF& pos) override; void NotifyLButtonUp(CPWL_Wnd* child, const CFX_PointF& pos) override; void CreateChildWnd(const CreateParams& cp) override; - void RePosChildWnd() override; + bool RePosChildWnd() override; CFX_FloatRect GetFocusRect() const override; void SetFocus() override; void KillFocus() override; @@ -85,7 +85,9 @@ class CPWL_ComboBox : public CPWL_Wnd { void CreateEdit(const CreateParams& cp); void CreateButton(const CreateParams& cp); void CreateListBox(const CreateParams& cp); - void SetPopup(bool bPopup); + + // Returns |true| iff this instance is still allocated. + bool SetPopup(bool bPopup); UnownedPtr m_pEdit; UnownedPtr m_pButton; diff --git a/fpdfsdk/pwl/cpwl_edit.cpp b/fpdfsdk/pwl/cpwl_edit.cpp index 662c69e369..650db5438b 100644 --- a/fpdfsdk/pwl/cpwl_edit.cpp +++ b/fpdfsdk/pwl/cpwl_edit.cpp @@ -79,13 +79,18 @@ void CPWL_Edit::SetText(const WideString& csText) { m_pEdit->SetText(swText); } -void CPWL_Edit::RePosChildWnd() { +bool CPWL_Edit::RePosChildWnd() { if (CPWL_ScrollBar* pVSB = GetVScrollBar()) { CFX_FloatRect rcWindow = m_rcOldWindow; CFX_FloatRect rcVScroll = CFX_FloatRect(rcWindow.right, rcWindow.bottom, rcWindow.right + PWL_SCROLLBAR_WIDTH, rcWindow.top); + + ObservedPtr thisObserved(this); + pVSB->Move(rcVScroll, true, false); + if (!thisObserved) + return false; } if (m_pEditCaret && !HasFlag(PES_TEXTOVERFLOW)) { @@ -98,7 +103,7 @@ void CPWL_Edit::RePosChildWnd() { m_pEditCaret->SetClipRect(rect); } - CPWL_EditCtrl::RePosChildWnd(); + return CPWL_EditCtrl::RePosChildWnd(); } CFX_FloatRect CPWL_Edit::GetClientRect() const { @@ -290,8 +295,8 @@ bool CPWL_Edit::OnLButtonDown(const CFX_PointF& point, uint32_t nFlag) { CPWL_Wnd::OnLButtonDown(point, nFlag); if (HasFlag(PES_TEXTOVERFLOW) || ClientHitTest(point)) { - if (m_bMouseDown) - InvalidateRect(nullptr); + if (m_bMouseDown && !InvalidateRect(nullptr)) + return true; m_bMouseDown = true; SetCapture(); @@ -355,17 +360,15 @@ void CPWL_Edit::OnKillFocus() { if (!observed_ptr) return; - Move(m_rcOldWindow, true, true); + if (!Move(m_rcOldWindow, true, true)) + return; } - if (!observed_ptr) - return; m_pEdit->SelectNone(); if (!observed_ptr) return; - SetCaret(false, CFX_PointF(), CFX_PointF()); - if (!observed_ptr) + if (!SetCaret(false, CFX_PointF(), CFX_PointF())) return; SetCharSet(FX_CHARSET_ANSI); diff --git a/fpdfsdk/pwl/cpwl_edit.h b/fpdfsdk/pwl/cpwl_edit.h index d093f44d84..9c6f5e8da1 100644 --- a/fpdfsdk/pwl/cpwl_edit.h +++ b/fpdfsdk/pwl/cpwl_edit.h @@ -49,7 +49,7 @@ class CPWL_Edit : public CPWL_EditCtrl { // CPWL_EditCtrl ByteString GetClassName() const override; void OnCreated() override; - void RePosChildWnd() override; + bool RePosChildWnd() override; CFX_FloatRect GetClientRect() const override; void DrawThisAppearance(CFX_RenderDevice* pDevice, const CFX_Matrix& mtUser2Device) override; diff --git a/fpdfsdk/pwl/cpwl_edit_ctrl.cpp b/fpdfsdk/pwl/cpwl_edit_ctrl.cpp index f0fc408771..2fe6e28c59 100644 --- a/fpdfsdk/pwl/cpwl_edit_ctrl.cpp +++ b/fpdfsdk/pwl/cpwl_edit_ctrl.cpp @@ -67,8 +67,9 @@ void CPWL_EditCtrl::ReplaceSelection(const WideString& text) { m_pEdit->InsertText(text, FX_CHARSET_Default); } -void CPWL_EditCtrl::RePosChildWnd() { +bool CPWL_EditCtrl::RePosChildWnd() { m_pEdit->SetPlateRect(GetClientRect()); + return true; } void CPWL_EditCtrl::SetScrollInfo(const PWL_SCROLL_INFO& info) { @@ -264,8 +265,8 @@ bool CPWL_EditCtrl::OnLButtonDown(const CFX_PointF& point, uint32_t nFlag) { CPWL_Wnd::OnLButtonDown(point, nFlag); if (ClientHitTest(point)) { - if (m_bMouseDown) - InvalidateRect(nullptr); + if (m_bMouseDown && !InvalidateRect(nullptr)) + return true; m_bMouseDown = true; SetCapture(); @@ -307,6 +308,8 @@ void CPWL_EditCtrl::SetEditCaret(bool bVisible) { GetCaretInfo(&ptHead, &ptFoot); SetCaret(bVisible, ptHead, ptFoot); + // Note, |this| may no longer be viable at this point. If more work needs to + // be done, check the return value of SetCaret(). } void CPWL_EditCtrl::GetCaretInfo(CFX_PointF* ptHead, CFX_PointF* ptFoot) const { @@ -327,15 +330,21 @@ void CPWL_EditCtrl::GetCaretInfo(CFX_PointF* ptHead, CFX_PointF* ptFoot) const { } } -void CPWL_EditCtrl::SetCaret(bool bVisible, +bool CPWL_EditCtrl::SetCaret(bool bVisible, const CFX_PointF& ptHead, const CFX_PointF& ptFoot) { - if (m_pEditCaret) { - if (!IsFocused() || m_pEdit->IsSelected()) - bVisible = false; + if (!m_pEditCaret) + return true; - m_pEditCaret->SetCaret(bVisible, ptHead, ptFoot); - } + if (!IsFocused() || m_pEdit->IsSelected()) + bVisible = false; + + ObservedPtr thisObserved(this); + m_pEditCaret->SetCaret(bVisible, ptHead, ptFoot); + if (!thisObserved) + return false; + + return true; } WideString CPWL_EditCtrl::GetText() const { diff --git a/fpdfsdk/pwl/cpwl_edit_ctrl.h b/fpdfsdk/pwl/cpwl_edit_ctrl.h index 37ac320d8f..31140e3038 100644 --- a/fpdfsdk/pwl/cpwl_edit_ctrl.h +++ b/fpdfsdk/pwl/cpwl_edit_ctrl.h @@ -56,14 +56,14 @@ class CPWL_EditCtrl : public CPWL_Wnd { void SetScrollPosition(float pos) override; void ScrollWindowVertically(float pos) override; void CreateChildWnd(const CreateParams& cp) override; - void RePosChildWnd() override; + bool RePosChildWnd() override; void SetFontSize(float fFontSize) override; float GetFontSize() const override; void SetCursor() override; WideString GetSelectedText() override; void ReplaceSelection(const WideString& text) override; - void SetCaret(bool bVisible, + bool SetCaret(bool bVisible, const CFX_PointF& ptHead, const CFX_PointF& ptFoot); diff --git a/fpdfsdk/pwl/cpwl_list_box.cpp b/fpdfsdk/pwl/cpwl_list_box.cpp index c8f7be995d..a7c02e4692 100644 --- a/fpdfsdk/pwl/cpwl_list_box.cpp +++ b/fpdfsdk/pwl/cpwl_list_box.cpp @@ -250,10 +250,12 @@ void CPWL_ListBox::KillFocus() { CPWL_Wnd::KillFocus(); } -void CPWL_ListBox::RePosChildWnd() { - CPWL_Wnd::RePosChildWnd(); +bool CPWL_ListBox::RePosChildWnd() { + if (!CPWL_Wnd::RePosChildWnd()) + return false; m_pList->SetPlateRect(GetListRect()); + return true; } bool CPWL_ListBox::OnNotifySelectionChanged(bool bKeyDown, uint32_t nFlag) { diff --git a/fpdfsdk/pwl/cpwl_list_box.h b/fpdfsdk/pwl/cpwl_list_box.h index fec4d63a0c..7cfdd18e93 100644 --- a/fpdfsdk/pwl/cpwl_list_box.h +++ b/fpdfsdk/pwl/cpwl_list_box.h @@ -61,7 +61,7 @@ class CPWL_ListBox : public CPWL_Wnd { void SetScrollInfo(const PWL_SCROLL_INFO& info) override; void SetScrollPosition(float pos) override; void ScrollWindowVertically(float pos) override; - void RePosChildWnd() override; + bool RePosChildWnd() override; CFX_FloatRect GetFocusRect() const override; void SetFontSize(float fFontSize) override; float GetFontSize() const override; diff --git a/fpdfsdk/pwl/cpwl_scroll_bar.cpp b/fpdfsdk/pwl/cpwl_scroll_bar.cpp index e50f778943..58a874531f 100644 --- a/fpdfsdk/pwl/cpwl_scroll_bar.cpp +++ b/fpdfsdk/pwl/cpwl_scroll_bar.cpp @@ -344,7 +344,7 @@ void CPWL_ScrollBar::OnDestroy() { CPWL_Wnd::OnDestroy(); } -void CPWL_ScrollBar::RePosChildWnd() { +bool CPWL_ScrollBar::RePosChildWnd() { CFX_FloatRect rcClient = GetClientRect(); CFX_FloatRect rcMinButton, rcMaxButton; float fBWidth = 0; @@ -367,7 +367,8 @@ void CPWL_ScrollBar::RePosChildWnd() { rcMaxButton = CFX_FloatRect(rcClient.right - fBWidth, rcClient.bottom, rcClient.right, rcClient.top); } else { - SetVisible(false); + if (!SetVisible(false)) + return false; } } break; @@ -389,17 +390,31 @@ void CPWL_ScrollBar::RePosChildWnd() { CFX_FloatRect(rcClient.left, rcClient.bottom, rcClient.right, rcClient.bottom + fBWidth); } else { - SetVisible(false); + if (!SetVisible(false)) + return false; } } break; } - if (m_pMinButton) + ObservedPtr thisObserved(this); + + if (m_pMinButton) { m_pMinButton->Move(rcMinButton, true, false); - if (m_pMaxButton) + if (!thisObserved) + return false; + } + + if (m_pMaxButton) { m_pMaxButton->Move(rcMaxButton, true, false); - MovePosButton(false); + if (!thisObserved) + return false; + } + + if (!MovePosButton(false)) + return false; + + return true; } void CPWL_ScrollBar::DrawThisAppearance(CFX_RenderDevice* pDevice, @@ -428,7 +443,8 @@ bool CPWL_ScrollBar::OnLButtonDown(const CFX_PointF& point, uint32_t nFlag) { if (HasFlag(PWS_AUTOTRANSPARENT)) { if (GetTransparency() != 255) { SetTransparency(255); - InvalidateRect(nullptr); + if (!InvalidateRect(nullptr)) + return true; } } @@ -459,13 +475,15 @@ bool CPWL_ScrollBar::OnLButtonDown(const CFX_PointF& point, uint32_t nFlag) { if (rcMinArea.Contains(point)) { m_sData.SubBig(); - MovePosButton(true); + if (!MovePosButton(true)) + return true; NotifyScrollWindow(); } if (rcMaxArea.Contains(point)) { m_sData.AddBig(); - MovePosButton(true); + if (!MovePosButton(true)) + return true; NotifyScrollWindow(); } } @@ -479,7 +497,8 @@ bool CPWL_ScrollBar::OnLButtonUp(const CFX_PointF& point, uint32_t nFlag) { if (HasFlag(PWS_AUTOTRANSPARENT)) { if (GetTransparency() != PWL_SCROLLBAR_TRANSPARENCY) { SetTransparency(PWL_SCROLLBAR_TRANSPARENCY); - InvalidateRect(nullptr); + if (!InvalidateRect(nullptr)) + return true; } } @@ -560,7 +579,10 @@ void CPWL_ScrollBar::CreateButtons(const CreateParams& cp) { if (!m_pPosButton) { m_pPosButton = new CPWL_SBButton(m_sbType, PSBT_POS); - m_pPosButton->SetVisible(false); + + ObservedPtr thisObserved(this); + if (!m_pPosButton->SetVisible(false) || !thisObserved) + return; m_pPosButton->Create(scp); } } @@ -575,24 +597,37 @@ float CPWL_ScrollBar::GetScrollBarWidth() const { void CPWL_ScrollBar::SetScrollRange(float fMin, float fMax, float fClientWidth) { - if (m_pPosButton) { - m_sData.SetScrollRange(fMin, fMax); - m_sData.SetClientWidth(fClientWidth); + if (!m_pPosButton) + return; - if (IsFloatSmaller(m_sData.ScrollRange.GetWidth(), 0.0f)) { - m_pPosButton->SetVisible(false); - } else { - m_pPosButton->SetVisible(true); - MovePosButton(true); - } + m_sData.SetScrollRange(fMin, fMax); + m_sData.SetClientWidth(fClientWidth); + + ObservedPtr thisObserved(this); + + if (IsFloatSmaller(m_sData.ScrollRange.GetWidth(), 0.0f)) { + m_pPosButton->SetVisible(false); + // Note, |this| may no longer be viable at this point. If more work needs + // to be done, check thisObserved. + return; } + + if (!m_pPosButton->SetVisible(true) || !thisObserved) + return; + + 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(). } void CPWL_ScrollBar::SetScrollPos(float fPos) { float fOldPos = m_sData.fScrollPos; m_sData.SetPos(fPos); - if (!IsFloatEqual(m_sData.fScrollPos, fOldPos)) + if (!IsFloatEqual(m_sData.fScrollPos, fOldPos)) { 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(). + } } void CPWL_ScrollBar::SetScrollStep(float fBigStep, float fSmallStep) { @@ -600,7 +635,7 @@ void CPWL_ScrollBar::SetScrollStep(float fBigStep, float fSmallStep) { m_sData.SetSmallStep(fSmallStep); } -void CPWL_ScrollBar::MovePosButton(bool bRefresh) { +bool CPWL_ScrollBar::MovePosButton(bool bRefresh) { ASSERT(m_pMinButton); ASSERT(m_pMaxButton); @@ -648,13 +683,20 @@ void CPWL_ScrollBar::MovePosButton(bool bRefresh) { break; } + ObservedPtr thisObserved(this); + m_pPosButton->Move(rcPosButton, true, bRefresh); + if (!thisObserved) + return false; } + + return true; } void CPWL_ScrollBar::OnMinButtonLBDown(const CFX_PointF& point) { m_sData.SubSmall(); - MovePosButton(true); + if (!MovePosButton(true)) + return; NotifyScrollWindow(); m_bMinOrMax = true; @@ -669,7 +711,8 @@ void CPWL_ScrollBar::OnMinButtonMouseMove(const CFX_PointF& point) {} void CPWL_ScrollBar::OnMaxButtonLBDown(const CFX_PointF& point) { m_sData.AddSmall(); - MovePosButton(true); + if (!MovePosButton(true)) + return; NotifyScrollWindow(); m_bMinOrMax = false; @@ -758,7 +801,8 @@ void CPWL_ScrollBar::OnPosButtonMouseMove(const CFX_PointF& point) { } if (!IsFloatEqual(fOldScrollPos, m_sData.fScrollPos)) { - MovePosButton(true); + if (!MovePosButton(true)) + return; if (m_bNotifyForever) NotifyScrollWindow(); @@ -875,7 +919,8 @@ void CPWL_ScrollBar::TimerProc() { m_sData.AddSmall(); if (sTemp != m_sData) { - MovePosButton(true); + if (!MovePosButton(true)) + return; NotifyScrollWindow(); } } diff --git a/fpdfsdk/pwl/cpwl_scroll_bar.h b/fpdfsdk/pwl/cpwl_scroll_bar.h index 1e71e3b90f..69217323ff 100644 --- a/fpdfsdk/pwl/cpwl_scroll_bar.h +++ b/fpdfsdk/pwl/cpwl_scroll_bar.h @@ -124,7 +124,7 @@ class CPWL_ScrollBar : public CPWL_Wnd { ByteString GetClassName() const override; void OnCreate(CreateParams* pParamsToAdjust) override; void OnDestroy() override; - void RePosChildWnd() override; + bool RePosChildWnd() override; void DrawThisAppearance(CFX_RenderDevice* pDevice, const CFX_Matrix& mtUser2Device) override; bool OnLButtonDown(const CFX_PointF& point, uint32_t nFlag) override; @@ -145,7 +145,9 @@ class CPWL_ScrollBar : public CPWL_Wnd { protected: void SetScrollRange(float fMin, float fMax, float fClientWidth); void SetScrollPos(float fPos); - void MovePosButton(bool bRefresh); + + // Returns |true| iff this instance is still allocated. + 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 d7eb853c8f..124065cdf9 100644 --- a/fpdfsdk/pwl/cpwl_wnd.cpp +++ b/fpdfsdk/pwl/cpwl_wnd.cpp @@ -175,7 +175,9 @@ void CPWL_Wnd::Create(const CreateParams& cp) { CreateChildWnd(ccp); m_bVisible = HasFlag(PWS_VISIBLE); OnCreated(); - RePosChildWnd(); + if (!RePosChildWnd()) + return; + m_bCreated = true; } @@ -201,7 +203,8 @@ void CPWL_Wnd::Destroy() { if (m_bCreated) { m_pVScrollBar = nullptr; for (auto it = m_Children.rbegin(); it != m_Children.rend(); ++it) { - if (CPWL_Wnd* pChild = *it) { + CPWL_Wnd* pChild = *it; + if (pChild) { *it = nullptr; pChild->Destroy(); delete pChild; @@ -216,9 +219,9 @@ void CPWL_Wnd::Destroy() { m_Children.clear(); } -void CPWL_Wnd::Move(const CFX_FloatRect& rcNew, bool bReset, bool bRefresh) { +bool CPWL_Wnd::Move(const CFX_FloatRect& rcNew, bool bReset, bool bRefresh) { if (!IsValid()) - return; + return true; CFX_FloatRect rcOld = GetWindowRect(); m_rcWindow = rcNew; @@ -227,21 +230,23 @@ void CPWL_Wnd::Move(const CFX_FloatRect& rcNew, bool bReset, bool bRefresh) { if (bReset) { if (rcOld.left != rcNew.left || rcOld.right != rcNew.right || rcOld.top != rcNew.top || rcOld.bottom != rcNew.bottom) { - RePosChildWnd(); + if (!RePosChildWnd()) + return false; } } - if (bRefresh) - InvalidateRectMove(rcOld, rcNew); + if (bRefresh && !InvalidateRectMove(rcOld, rcNew)) + return false; m_CreationParams.rcRectWnd = m_rcWindow; + return true; } -void CPWL_Wnd::InvalidateRectMove(const CFX_FloatRect& rcOld, +bool CPWL_Wnd::InvalidateRectMove(const CFX_FloatRect& rcOld, const CFX_FloatRect& rcNew) { CFX_FloatRect rcUnion = rcOld; rcUnion.Union(rcNew); - InvalidateRect(&rcUnion); + return InvalidateRect(&rcUnion); } void CPWL_Wnd::DrawAppearance(CFX_RenderDevice* pDevice, @@ -289,9 +294,10 @@ void CPWL_Wnd::DrawChildAppearance(CFX_RenderDevice* pDevice, } } -void CPWL_Wnd::InvalidateRect(CFX_FloatRect* pRect) { +bool CPWL_Wnd::InvalidateRect(CFX_FloatRect* pRect) { + ObservedPtr thisObserved(this); if (!IsValid()) - return; + return true; CFX_FloatRect rcRefresh = pRect ? *pRect : GetWindowRect(); @@ -310,8 +316,12 @@ void CPWL_Wnd::InvalidateRect(CFX_FloatRect* pRect) { if (CPDFSDK_Widget* widget = static_cast( m_CreationParams.pAttachedWidget.Get())) { pSH->InvalidateRect(widget, rcWin); + if (!thisObserved) + return false; } } + + return true; } #define PWL_IMPLEMENT_KEY_METHOD(key_method_name) \ @@ -563,19 +573,29 @@ const CPWL_Wnd* CPWL_Wnd::GetRootWnd() const { return pParent ? pParent->GetRootWnd() : this; } -void CPWL_Wnd::SetVisible(bool bVisible) { +bool CPWL_Wnd::SetVisible(bool bVisible) { if (!IsValid()) - return; + return true; + + ObservedPtr thisObserved(this); for (auto* pChild : m_Children) { - if (pChild) + if (pChild) { pChild->SetVisible(bVisible); + if (!thisObserved) + return false; + } } + if (bVisible != m_bVisible) { m_bVisible = bVisible; - RePosChildWnd(); - InvalidateRect(nullptr); + if (!RePosChildWnd()) + return false; + + if (!InvalidateRect(nullptr)) + return false; } + return true; } void CPWL_Wnd::SetClipRect(const CFX_FloatRect& rect) { @@ -591,10 +611,10 @@ bool CPWL_Wnd::IsReadOnly() const { return HasFlag(PWS_READONLY); } -void CPWL_Wnd::RePosChildWnd() { +bool CPWL_Wnd::RePosChildWnd() { CPWL_ScrollBar* pVSB = GetVScrollBar(); if (!pVSB) - return; + return true; CFX_FloatRect rcContent = GetWindowRect(); if (!rcContent.IsEmpty()) { @@ -605,7 +625,14 @@ void CPWL_Wnd::RePosChildWnd() { CFX_FloatRect rcVScroll = CFX_FloatRect(rcContent.right - PWL_SCROLLBAR_WIDTH, rcContent.bottom, rcContent.right - 1.0f, rcContent.top); + + ObservedPtr thisObserved(this); + pVSB->Move(rcVScroll, true, false); + if (!thisObserved) + return false; + + return true; } void CPWL_Wnd::CreateChildWnd(const CreateParams& cp) {} diff --git a/fpdfsdk/pwl/cpwl_wnd.h b/fpdfsdk/pwl/cpwl_wnd.h index c37fa2fff3..38d4ce0918 100644 --- a/fpdfsdk/pwl/cpwl_wnd.h +++ b/fpdfsdk/pwl/cpwl_wnd.h @@ -151,7 +151,9 @@ class CPWL_Wnd : public CPWL_TimerHandler, public Observable { ~CPWL_Wnd() override; virtual ByteString GetClassName() const; - virtual void InvalidateRect(CFX_FloatRect* pRect); + + // Returns |true| iff this instance is still allocated. + virtual bool InvalidateRect(CFX_FloatRect* pRect); virtual bool OnKeyDown(uint16_t nChar, uint32_t nFlag); virtual bool OnChar(uint16_t nChar, uint32_t nFlag); @@ -173,7 +175,9 @@ class CPWL_Wnd : public CPWL_TimerHandler, public Observable { virtual void SetFocus(); virtual void KillFocus(); virtual void SetCursor(); - virtual void SetVisible(bool bVisible); + + // Returns |true| iff this instance is still allocated. + virtual bool SetVisible(bool bVisible); virtual void SetFontSize(float fFontSize); virtual float GetFontSize() const; @@ -186,7 +190,7 @@ class CPWL_Wnd : public CPWL_TimerHandler, public Observable { void InvalidateProvider(ProviderIface* provider); void Create(const CreateParams& cp); void Destroy(); - void Move(const CFX_FloatRect& rcNew, bool bReset, bool bRefresh); + bool Move(const CFX_FloatRect& rcNew, bool bReset, bool bRefresh); void SetCapture(); void ReleaseCapture(); @@ -252,7 +256,9 @@ class CPWL_Wnd : public CPWL_TimerHandler, public Observable { CFX_SystemHandler* GetSystemHandler() const override; virtual void CreateChildWnd(const CreateParams& cp); - virtual void RePosChildWnd(); + + // Returns |true| iff this instance is still allocated. + virtual bool RePosChildWnd(); virtual void DrawThisAppearance(CFX_RenderDevice* pDevice, const CFX_Matrix& mtUser2Device); @@ -269,7 +275,8 @@ class CPWL_Wnd : public CPWL_TimerHandler, public Observable { bool IsValid() const { return m_bCreated; } const CreateParams& GetCreationParams() const { return m_CreationParams; } - void InvalidateRectMove(const CFX_FloatRect& rcOld, + // Returns |true| iff this instance is still allocated. + bool InvalidateRectMove(const CFX_FloatRect& rcOld, const CFX_FloatRect& rcNew); bool IsWndCaptureMouse(const CPWL_Wnd* pWnd) const; -- cgit v1.2.3