[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - sd/source
Caolán McNamara (via logerrit)
logerrit at kemper.freedesktop.org
Tue Sep 29 09:54:27 UTC 2020
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 01eefecf1eca39969e1b7f04ca80204105452979
Author: Caolán McNamara <caolanm at redhat.com>
AuthorDate: Thu Sep 24 14:00:35 2020 +0100
Commit: Michael Stahl <michael.stahl at cib.de>
CommitDate: Tue Sep 29 11:53:54 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/+/103326
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl at cib.de>
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 74bbe395d92d..d253bce66b4a 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 ca99e3f7a214..cd7534a89893 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();
More information about the Libreoffice-commits
mailing list