diff options
author | Aron Budea <aron.budea@collabora.com> | 2017-04-03 02:21:28 +0200 |
---|---|---|
committer | Andras Timar <andras.timar@collabora.com> | 2017-04-07 07:53:09 +0200 |
commit | f75be566b13e6361f9792fb227d6cd2b839e1fb6 (patch) | |
tree | 87af1b50ff215f018855e75945b37f1db1e438bf /vcl | |
parent | 5ec306f3fb4b8965c4aa0aa14ec09a135471571e (diff) |
tdf#104686: do not crash if Menu has been somehow disposed
The rare crashes in MenuFloatingWindow::ImplGetStartY() and
MenuFloatingWindow::ImplScroll(bool) likely happen because
of a disposed Menu.
Let's guard against invalid accesses.
Change-Id: Ie31240abbc48c06edd40d0a95f319725cdb3db16
Reviewed-on: https://gerrit.libreoffice.org/36026
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/36151
Reviewed-by: Aron Budea <aron.budea@collabora.com>
(cherry picked from commit 9fcb6cb86893b991ceb6395fbabba63c962f59db)
Diffstat (limited to 'vcl')
-rw-r--r-- | vcl/source/window/menu.cxx | 4 | ||||
-rw-r--r-- | vcl/source/window/menufloatingwindow.cxx | 74 | ||||
-rw-r--r-- | vcl/source/window/menuitemlist.cxx | 7 | ||||
-rw-r--r-- | vcl/source/window/menuitemlist.hxx | 1 |
4 files changed, 56 insertions, 30 deletions
diff --git a/vcl/source/window/menu.cxx b/vcl/source/window/menu.cxx index 854affea713f..0852b953442a 100644 --- a/vcl/source/window/menu.cxx +++ b/vcl/source/window/menu.cxx @@ -175,9 +175,11 @@ void Menu::dispose() bKilled = true; - delete pItemList; + pItemList->Clear(); delete pLogo; + pLogo = nullptr; delete mpLayoutData; + mpLayoutData = nullptr; // Native-support: destroy SalMenu ImplSetSalMenu( nullptr ); diff --git a/vcl/source/window/menufloatingwindow.cxx b/vcl/source/window/menufloatingwindow.cxx index 366aeed1ad1d..6bf2fde56e3e 100644 --- a/vcl/source/window/menufloatingwindow.cxx +++ b/vcl/source/window/menufloatingwindow.cxx @@ -152,6 +152,12 @@ long MenuFloatingWindow::ImplGetStartY() const long nY = 0; if( pMenu ) { + // avoid crash if somehow menu got disposed, and MenuItemList is empty (workaround for tdf#104686) + if ( nFirstEntry > 0 && !pMenu->GetItemList()->GetDataFromPos(nFirstEntry - 1) ) + { + return 0; + } + for ( sal_uInt16 n = 0; n < nFirstEntry; n++ ) nY += pMenu->GetItemList()->GetDataFromPos( n )->aSz.Height(); nY -= pMenu->GetTitleHeight(); @@ -606,45 +612,55 @@ void MenuFloatingWindow::ImplScroll( bool bUp ) nFirstEntry = pMenu->ImplGetPrevVisible( nFirstEntry ); SAL_WARN_IF( nFirstEntry == ITEMPOS_INVALID, "vcl", "Scroll?!" ); - long nScrollEntryHeight = pMenu->GetItemList()->GetDataFromPos( nFirstEntry )->aSz.Height(); - - if ( !bScrollDown ) + // avoid crash if somehow menu got disposed, and MenuItemList is empty (workaround for tdf#104686) + const auto pItemData = pMenu->GetItemList()->GetDataFromPos( nFirstEntry ); + if ( pItemData ) { - bScrollDown = true; - Invalidate(); - } + long nScrollEntryHeight = pItemData->aSz.Height(); - if ( pMenu->ImplGetPrevVisible( nFirstEntry ) == ITEMPOS_INVALID ) - { - bScrollUp = false; - Invalidate(); - } + if ( !bScrollDown ) + { + bScrollDown = true; + Invalidate(); + } + + if ( pMenu->ImplGetPrevVisible( nFirstEntry ) == ITEMPOS_INVALID ) + { + bScrollUp = false; + Invalidate(); + } - Scroll( 0, nScrollEntryHeight, ImplCalcClipRegion( false ).GetBoundRect(), ScrollFlags::Clip ); + Scroll( 0, nScrollEntryHeight, ImplCalcClipRegion( false ).GetBoundRect(), ScrollFlags::Clip ); + } } else if ( bScrollDown && !bUp ) { - long nScrollEntryHeight = pMenu->GetItemList()->GetDataFromPos( nFirstEntry )->aSz.Height(); + // avoid crash if somehow menu got disposed, and MenuItemList is empty (workaround for tdf#104686) + const auto pItemData = pMenu->GetItemList()->GetDataFromPos( nFirstEntry ); + if ( pItemData ) + { + long nScrollEntryHeight = pItemData->aSz.Height(); - nFirstEntry = pMenu->ImplGetNextVisible( nFirstEntry ); - SAL_WARN_IF( nFirstEntry == ITEMPOS_INVALID, "vcl", "Scroll?!" ); + nFirstEntry = pMenu->ImplGetNextVisible( nFirstEntry ); + SAL_WARN_IF( nFirstEntry == ITEMPOS_INVALID, "vcl", "Scroll?!" ); - if ( !bScrollUp ) - { - bScrollUp = true; - Invalidate(); - } + if ( !bScrollUp ) + { + bScrollUp = true; + Invalidate(); + } - long nHeight = GetOutputSizePixel().Height(); - sal_uInt16 nLastVisible; - static_cast<PopupMenu*>(pMenu.get())->ImplCalcVisEntries( nHeight, nFirstEntry, &nLastVisible ); - if ( pMenu->ImplGetNextVisible( nLastVisible ) == ITEMPOS_INVALID ) - { - bScrollDown = false; - Invalidate(); - } + long nHeight = GetOutputSizePixel().Height(); + sal_uInt16 nLastVisible; + static_cast<PopupMenu*>(pMenu.get())->ImplCalcVisEntries( nHeight, nFirstEntry, &nLastVisible ); + if ( pMenu->ImplGetNextVisible( nLastVisible ) == ITEMPOS_INVALID ) + { + bScrollDown = false; + Invalidate(); + } - Scroll( 0, -nScrollEntryHeight, ImplCalcClipRegion( false ).GetBoundRect(), ScrollFlags::Clip ); + Scroll( 0, -nScrollEntryHeight, ImplCalcClipRegion( false ).GetBoundRect(), ScrollFlags::Clip ); + } } Invalidate(); diff --git a/vcl/source/window/menuitemlist.cxx b/vcl/source/window/menuitemlist.cxx index 44757a4778ba..30bd874d9763 100644 --- a/vcl/source/window/menuitemlist.cxx +++ b/vcl/source/window/menuitemlist.cxx @@ -135,6 +135,13 @@ void MenuItemList::Remove( size_t nPos ) } } +void MenuItemList::Clear() +{ + for (MenuItemData* i : maItemList) + delete i; + maItemList.resize(0); +} + MenuItemData* MenuItemList::GetData( sal_uInt16 nSVId, size_t& rPos ) const { for( size_t i = 0, n = maItemList.size(); i < n; ++i ) diff --git a/vcl/source/window/menuitemlist.hxx b/vcl/source/window/menuitemlist.hxx index 89befc106e28..556a5a049597 100644 --- a/vcl/source/window/menuitemlist.hxx +++ b/vcl/source/window/menuitemlist.hxx @@ -116,6 +116,7 @@ public: ); void InsertSeparator(const OString &rIdent, size_t nPos); void Remove( size_t nPos ); + void Clear(); MenuItemData* GetData( sal_uInt16 nSVId, size_t& rPos ) const; MenuItemData* GetData( sal_uInt16 nSVId ) const |