sd/source/ui/inc/PresentationViewShell.hxx | 3 +++
sd/source/ui/inc/slideshow.hxx | 3 ++-
sd/source/ui/slideshow/slideshow.cxx | 20 ++++++++------------
sd/source/ui/view/presvish.cxx | 29 +++++++++++++++++++++++++++--
sd/source/ui/view/viewshel.cxx | 7 +++++--
5 files changed, 45 insertions(+), 17 deletions(-)
New commits:
commit 582a73abd2e5b51d8a21af2997395cd4db707a80
Author: Caolán McNamara <caol...@redhat.com>
AuthorDate: Thu Sep 24 14:00:35 2020 +0100
Commit: Caolán McNamara <caol...@redhat.com>
CommitDate: Fri Sep 25 09:57:10 2020 +0200
tdf#64711 pointer deleted out from under std::shared_ptr
the "end" call will close the shell, which is deleted directly so the local
shared_ptr remains
pointed to something which is already deleted.
So:
a) Have activate return true/false to indicate the failure and require the
caller to call
'end' in response to failure.
b) A bunch of stuff in the call-stack expects the ViewShell not to get
deleted while they are
calling it, so additionally launch that 'end' to happen in the next event
loop.
==2838867== Invalid read of size 8
==2838867== at 0x32F4B83C: sd::ViewShellBase::GetDocShell() const
(ViewShellBase.hxx:97)
==2838867== by 0x335BBCFC: sd::ViewShell::GetDocSh() const
(viewshel.cxx:1389)
==2838867== by 0x3357CAC1:
sd::PresentationViewShell::~PresentationViewShell() (presvish.cxx:73)
==2838867== by 0x330AA34B: void
__gnu_cxx::new_allocator<sd::PresentationViewShell>::destroy<sd::PresentationViewShell>(sd::PresentationViewShell*)
(new_allocator.h:156)
==2838867== by 0x330AA2DF: void
std::allocator_traits<std::allocator<sd::PresentationViewShell>
>::destroy<sd::PresentationViewShell>(std::allocator<sd::PresentationViewShell>&,
sd::PresentationViewShell*) (alloc_traits.h:531)
==2838867== by 0x330AA0BE:
std::_Sp_counted_ptr_inplace<sd::PresentationViewShell,
std::allocator<sd::PresentationViewShell>,
(__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:560)
==2838867== by 0x32D10D33:
std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release()
(shared_ptr_base.h:158)
==2838867== by 0x32D10C79:
std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count()
(shared_ptr_base.h:733)
==2838867== by 0x330A03BD: std::__shared_ptr<sd::PresentationViewShell,
(__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1183)
==2838867== by 0x3309F847:
std::shared_ptr<sd::PresentationViewShell>::~shared_ptr() (shared_ptr.h:121)
==2838867== by 0x3320E1E8: sd::SlideShow::activate(sd::ViewShellBase&)
(slideshow.cxx:999)
==2838867== by 0x3357CF33: sd::PresentationViewShell::Activate(bool)
(presvish.cxx:118)
==2838867== Address 0x2fb5f320 is 256 bytes inside a block of size 272
free'd
==2838867== at 0x483BEDD: operator delete(void*)
(vg_replace_malloc.c:584)
==2838867== by 0x33466537:
sd::PresentationViewShellBase::~PresentationViewShellBase()
(PresentationViewShellBase.cxx:82)
==2838867== by 0x823076C: SfxViewFrame::ReleaseObjectShell_Impl()
(viewfrm.cxx:1113)
==2838867== by 0x8234B1C: SfxViewFrame::~SfxViewFrame()
(viewfrm.cxx:1652)
==2838867== by 0x8234FEB: SfxViewFrame::~SfxViewFrame()
(viewfrm.cxx:1646)
==2838867== by 0x8230D6C: SfxViewFrame::Close() (viewfrm.cxx:1165)
==2838867== by 0x81E4217: SfxFrame::DoClose_Impl() (frame.cxx:142)
==2838867== by 0x821709A: SfxBaseController::dispose()
(sfxbasecontroller.cxx:982)
==2838867== by 0x3337A59F: sd::DrawController::dispose()
(DrawController.cxx:164)
==2838867== by 0x6F35CC0: (anonymous
namespace)::XFrameImpl::setComponent(com::sun::star::uno::Reference<com::sun::star::awt::XWindow>
const&, com::sun::star::uno::Reference<com::sun::star::frame::XController>
const&) (frame.cxx:1485)
==2838867== by 0x6F3834E: (anonymous
namespace)::XFrameImpl::close(unsigned char) (frame.cxx:1692)
==2838867== by 0x81E3CFA: SfxFrame::DoClose() (frame.cxx:108)
==2838867== by 0x823802C: SfxViewFrame::DoClose() (viewfrm.cxx:2525)
==2838867== by 0x3320B971: sd::SlideShow::end() (slideshow.cxx:696)
==2838867== by 0x3320E1C2: sd::SlideShow::activate(sd::ViewShellBase&)
(slideshow.cxx:995)
==2838867== by 0x3357CF33: sd::PresentationViewShell::Activate(bool)
(presvish.cxx:118)
Change-Id: Ida91228b7584491c9a5dc9c0b3f76ce63218a92a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103319
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caol...@redhat.com>
diff --git a/sd/source/ui/inc/PresentationViewShell.hxx
b/sd/source/ui/inc/PresentationViewShell.hxx
index c0020685c02e..7adbc6631461 100644
--- a/sd/source/ui/inc/PresentationViewShell.hxx
+++ b/sd/source/ui/inc/PresentationViewShell.hxx
@@ -58,9 +58,12 @@ protected:
private:
::tools::Rectangle maOldVisArea;
+ ImplSVEvent* mnAbortSlideShowEvent;
virtual void Activate (bool bIsMDIActivate) override;
virtual void Paint (const ::tools::Rectangle& rRect, ::sd::Window* pWin)
override;
+
+ DECL_LINK(AbortSlideShowHdl, void*, void);
};
} // end of namespace sd
diff --git a/sd/source/ui/inc/slideshow.hxx b/sd/source/ui/inc/slideshow.hxx
index 90a45ac826ab..9b6087a5d311 100644
--- a/sd/source/ui/inc/slideshow.hxx
+++ b/sd/source/ui/inc/slideshow.hxx
@@ -157,7 +157,8 @@ public:
// events
void resize( const Size &rSize );
- void activate(ViewShellBase& rBase);
+ // return false if the activate failed. callers should call end in
response to failre
+ bool activate(ViewShellBase& rBase);
void deactivate();
void paint();
diff --git a/sd/source/ui/slideshow/slideshow.cxx
b/sd/source/ui/slideshow/slideshow.cxx
index 98c465d49eb6..cd10ad249eed 100644
--- a/sd/source/ui/slideshow/slideshow.cxx
+++ b/sd/source/ui/slideshow/slideshow.cxx
@@ -972,7 +972,7 @@ void SlideShow::resize( const Size &rSize )
mxController->resize( rSize );
}
-void SlideShow::activate( ViewShellBase& rBase )
+bool SlideShow::activate( ViewShellBase& rBase )
{
if( (mpFullScreenViewShellBase == &rBase) && !mxController.is() )
{
@@ -984,23 +984,19 @@ void SlideShow::activate( ViewShellBase& rBase )
CreateController( pShell.get(), pShell->GetView(),
rBase.GetViewWindow() );
- if( mxController->startShow(mxCurrentSettings.get()) )
- {
- pShell->Resize();
- // Defer the sd::ShowWindow's GrabFocus to here. so that the
accessible event can be fired correctly.
- pShell->GetActiveWindow()->GrabFocus();
- }
- else
- {
- end();
- return;
- }
+ if (!mxController->startShow(mxCurrentSettings.get()))
+ return false;
+
+ pShell->Resize();
+ // Defer the sd::ShowWindow's GrabFocus to here. so that the
accessible event can be fired correctly.
+ pShell->GetActiveWindow()->GrabFocus();
}
}
if( mxController.is() )
mxController->activate();
+ return true;
}
void SlideShow::deactivate()
diff --git a/sd/source/ui/view/presvish.cxx b/sd/source/ui/view/presvish.cxx
index e324b5803f7b..34a789f4dd18 100644
--- a/sd/source/ui/view/presvish.cxx
+++ b/sd/source/ui/view/presvish.cxx
@@ -61,7 +61,8 @@ void PresentationViewShell::InitInterface_Impl()
PresentationViewShell::PresentationViewShell( ViewShellBase& rViewShellBase,
vcl::Window* pParentWindow, FrameView* pFrameView)
-: DrawViewShell( rViewShellBase, pParentWindow, PageKind::Standard, pFrameView)
+ : DrawViewShell(rViewShellBase, pParentWindow, PageKind::Standard,
pFrameView)
+ , mnAbortSlideShowEvent(nullptr)
{
if( GetDocSh() && GetDocSh()->GetCreateMode() ==
SfxObjectCreateMode::EMBEDDED )
maOldVisArea = GetDocSh()->GetVisArea( ASPECT_CONTENT );
@@ -70,6 +71,9 @@ PresentationViewShell::PresentationViewShell( ViewShellBase&
rViewShellBase, vcl
PresentationViewShell::~PresentationViewShell()
{
+ if (mnAbortSlideShowEvent)
+ Application::RemoveUserEvent(mnAbortSlideShowEvent);
+
if( GetDocSh() && GetDocSh()->GetCreateMode() ==
SfxObjectCreateMode::EMBEDDED && !maOldVisArea.IsEmpty() )
GetDocSh()->SetVisArea( maOldVisArea );
}
@@ -102,6 +106,14 @@ VclPtr<SvxRuler>
PresentationViewShell::CreateVRuler(::sd::Window*)
return nullptr;
}
+IMPL_LINK_NOARG(PresentationViewShell, AbortSlideShowHdl, void*, void)
+{
+ mnAbortSlideShowEvent = nullptr;
+ rtl::Reference<SlideShow>
xSlideShow(SlideShow::GetSlideShow(GetViewShellBase()));
+ if (xSlideShow.is())
+ xSlideShow->end();
+}
+
void PresentationViewShell::Activate( bool bIsMDIActivate )
{
DrawViewShell::Activate( bIsMDIActivate );
@@ -115,7 +127,20 @@ void PresentationViewShell::Activate( bool bIsMDIActivate )
rtl::Reference< SlideShow > xSlideShow( SlideShow::GetSlideShow(
GetViewShellBase() ) );
if( xSlideShow.is() )
- xSlideShow->activate(GetViewShellBase());
+ {
+ bool bSuccess = xSlideShow->activate(GetViewShellBase());
+ if (!bSuccess)
+ {
+ /* tdf#64711 PresentationViewShell is deleted by 'end' due to
end closing
+ the object shell. So if we call xSlideShow->end during
Activate there are
+ a lot of places in the call stack of Activate which
understandable don't
+ expect this ViewShell to be deleted during use. Defer to
the next event
+ loop the abort of the slideshow
+ */
+ if (!mnAbortSlideShowEvent)
+ mnAbortSlideShowEvent =
Application::PostUserEvent(LINK(this, PresentationViewShell,
AbortSlideShowHdl));
+ }
+ }
if( HasCurrentFunction() )
GetCurrentFunction()->Activate();
diff --git a/sd/source/ui/view/viewshel.cxx b/sd/source/ui/view/viewshel.cxx
index ec1dd6d27b5a..a5d1cb22f140 100644
--- a/sd/source/ui/view/viewshel.cxx
+++ b/sd/source/ui/view/viewshel.cxx
@@ -320,8 +320,11 @@ void ViewShell::Activate(bool bIsMDIActivate)
rBindings.Invalidate( SID_3D_STATE, true );
rtl::Reference< SlideShow > xSlideShow( SlideShow::GetSlideShow(
GetViewShellBase() ) );
- if(xSlideShow.is() && xSlideShow->isRunning() )
- xSlideShow->activate(GetViewShellBase());
+ if (xSlideShow.is() && xSlideShow->isRunning())
+ {
+ bool bSuccess = xSlideShow->activate(GetViewShellBase());
+ assert(bSuccess && "can only return false with a
PresentationViewShell"); (void)bSuccess;
+ }
if(HasCurrentFunction())
GetCurrentFunction()->Activate();
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
