[libreoffice/f21] Resolves: rhbz#1136013 ExternalToolEdit crash
Michael Stahl
mstahl at fedoraproject.org
Mon Jan 19 12:45:23 UTC 2015
commit c037cf101add37cbc749c08373d8a22cc51f34f3
Author: Michael Stahl <mstahl at redhat.com>
Date: Mon Jan 19 13:45:40 2015 +0100
Resolves: rhbz#1136013 ExternalToolEdit crash
...3-svx-try-to-make-the-ExternalToolEdit-no.patch | 531 ++++++++++++++++++++
libreoffice.spec | 1 +
2 files changed, 532 insertions(+), 0 deletions(-)
---
diff --git a/0001-rhbz-1136013-svx-try-to-make-the-ExternalToolEdit-no.patch b/0001-rhbz-1136013-svx-try-to-make-the-ExternalToolEdit-no.patch
new file mode 100644
index 0000000..a31ba7d
--- /dev/null
+++ b/0001-rhbz-1136013-svx-try-to-make-the-ExternalToolEdit-no.patch
@@ -0,0 +1,531 @@
+From 4757c4173b9c093536d91ddbb0800b92e0842cc7 Mon Sep 17 00:00:00 2001
+From: Michael Stahl <mstahl at redhat.com>
+Date: Fri, 16 Jan 2015 23:56:09 +0100
+Subject: [PATCH] rhbz#1136013: svx: try to make the ExternalToolEdit not crash
+ all the time
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+This thing was starting a timer that re-starts itself forever, and when
+the file it was watching changed, it would just assume the drawing
+objects were still there (and the document, for that matter...)
+
+(cherry picked from commit 5f6bdce0c0ac687f418821ce328f2987bf340cda)
+
+Conflicts:
+ sc/source/ui/drawfunc/graphsh.cxx
+ sd/source/ui/inc/DrawViewShell.hxx
+ sd/source/ui/view/drviews2.cxx
+ svx/source/core/extedit.cxx
+ sw/source/core/uibase/shells/grfsh.cxx
+
+Converted to C++98.
+
+Change-Id: I35f187f0828097a05618dc1733dce819fc6bffc6
+Reviewed-on: https://gerrit.libreoffice.org/13994
+Tested-by: Caolán McNamara <caolanm at redhat.com>
+Reviewed-by: Caolán McNamara <caolanm at redhat.com>
+---
+ include/svx/extedit.hxx | 42 +++++++++++----
+ sc/source/ui/drawfunc/graphsh.cxx | 32 ++---------
+ sc/source/ui/inc/graphsh.hxx | 4 ++
+ sd/source/ui/inc/DrawViewShell.hxx | 4 ++
+ sd/source/ui/view/drviews2.cxx | 35 ++----------
+ sd/source/ui/view/drviewsa.cxx | 1 +
+ svx/source/core/extedit.cxx | 97 +++++++++++++++++++++++++++-------
+ sw/source/core/uibase/inc/grfsh.hxx | 5 ++
+ sw/source/core/uibase/shells/grfsh.cxx | 50 ++++++++++++------
+ 9 files changed, 167 insertions(+), 103 deletions(-)
+
+diff --git a/include/svx/extedit.hxx b/include/svx/extedit.hxx
+index dc0c489..eba4794 100644
+--- a/include/svx/extedit.hxx
++++ b/include/svx/extedit.hxx
+@@ -10,30 +10,52 @@
+ #ifndef INCLUDED_SVX_EXTEDIT_HXX
+ #define INCLUDED_SVX_EXTEDIT_HXX
+
+-#include <svtools/grfmgr.hxx>
+-#include <osl/file.hxx>
+-#include <osl/process.h>
+-#include <vcl/graph.hxx>
+-#include <vcl/timer.hxx>
+ #include <svx/svxdllapi.h>
++#include <svl/lstner.hxx>
++#include <rtl/ustring.hxx>
++#include <memory>
++#include <boost/scoped_ptr.hpp>
++
++class Graphic;
++class GraphicObject;
++class FileChangedChecker;
+
+ class SVX_DLLPUBLIC ExternalToolEdit
+ {
+-public:
+- GraphicObject* m_pGraphicObject;
++protected:
+ OUString m_aFileName;
+
++ ::boost::scoped_ptr<FileChangedChecker> m_pChecker;
++
++public:
++
+ ExternalToolEdit();
+ virtual ~ExternalToolEdit();
+
+ virtual void Update( Graphic& aGraphic ) = 0;
+- void Edit( GraphicObject *pGraphic );
++ void Edit(GraphicObject const*const pGraphic);
+
+- DECL_LINK( StartListeningEvent, void *pEvent );
++ void StartListeningEvent();
+
+- static void threadWorker( void *pThreadData );
+ static void HandleCloseEvent( ExternalToolEdit* pData );
+ };
+
++class FmFormView;
++class SdrObject;
++
++class SVX_DLLPUBLIC SdrExternalToolEdit
++ : public ExternalToolEdit
++ , public SfxListener
++{
++private:
++ FmFormView * m_pView;
++ SdrObject * m_pObj;
++
++ SAL_DLLPRIVATE virtual void Update(Graphic&) SAL_OVERRIDE;
++ SAL_DLLPRIVATE virtual void Notify(SfxBroadcaster&, const SfxHint&) SAL_OVERRIDE;
++
++public:
++ SdrExternalToolEdit(FmFormView * pView, SdrObject * pObj);
++};
+
+ #endif
+diff --git a/sc/source/ui/drawfunc/graphsh.cxx b/sc/source/ui/drawfunc/graphsh.cxx
+index 67e3ca4..ae967cd 100644
+--- a/sc/source/ui/drawfunc/graphsh.cxx
++++ b/sc/source/ui/drawfunc/graphsh.cxx
+@@ -38,31 +38,6 @@
+ #define ScGraphicShell
+ #include "scslots.hxx"
+
+-class ScExternalToolEdit : public ExternalToolEdit
+-{
+- FmFormView* m_pView;
+- SdrObject* m_pObj;
+-
+-public:
+- ScExternalToolEdit ( FmFormView* pView, SdrObject* pObj ) :
+- m_pView (pView),
+- m_pObj (pObj)
+- {}
+-
+- virtual void Update( Graphic& aGraphic ) SAL_OVERRIDE
+- {
+- SdrPageView* pPageView = m_pView->GetSdrPageView();
+- if( pPageView )
+- {
+- SdrGrafObj* pNewObj = (SdrGrafObj*) m_pObj->Clone();
+- OUString aStr = m_pView->GetDescriptionOfMarkedObjects() + " External Edit";
+- m_pView->BegUndo( aStr );
+- pNewObj->SetGraphicObject( aGraphic );
+- m_pView->ReplaceObjectAtView( m_pObj, *pPageView, pNewObj );
+- m_pView->EndUndo();
+- }
+- }
+-};
+
+ SFX_IMPL_INTERFACE(ScGraphicShell, ScDrawShell, ScResId(SCSTR_GRAPHICSHELL))
+
+@@ -187,9 +162,10 @@ void ScGraphicShell::ExecuteExternalEdit( SfxRequest& )
+
+ if( pObj && pObj->ISA( SdrGrafObj ) && ( (SdrGrafObj*) pObj )->GetGraphicType() == GRAPHIC_BITMAP )
+ {
+- GraphicObject aGraphicObject( ( (SdrGrafObj*) pObj )->GetGraphicObject() );
+- ScExternalToolEdit* aExternalToolEdit = new ScExternalToolEdit( pView, pObj );
+- aExternalToolEdit->Edit( &aGraphicObject );
++ GraphicObject aGraphicObject( static_cast<SdrGrafObj*>(pObj)->GetGraphicObject() );
++ m_ExternalEdits.push_back( boost::shared_ptr<SdrExternalToolEdit>(
++ new SdrExternalToolEdit(pView, pObj)));
++ m_ExternalEdits.back()->Edit( &aGraphicObject );
+ }
+ }
+
+diff --git a/sc/source/ui/inc/graphsh.hxx b/sc/source/ui/inc/graphsh.hxx
+index 866d527..677bd72 100644
+--- a/sc/source/ui/inc/graphsh.hxx
++++ b/sc/source/ui/inc/graphsh.hxx
+@@ -24,7 +24,9 @@
+ #include "shellids.hxx"
+ #include <sfx2/module.hxx>
+ #include <svx/svdmark.hxx>
++#include <boost/shared_ptr.hpp>
+
++class SdrExternalToolEdit;
+ class ScViewData;
+
+ #include "drawsh.hxx"
+@@ -36,6 +38,8 @@ public:
+ SFX_DECL_INTERFACE(SCID_GRAPHIC_SHELL)
+
+ private:
++ std::vector<boost::shared_ptr<SdrExternalToolEdit> > m_ExternalEdits;
++
+ /// SfxInterface initializer.
+ static void InitInterface_Impl();
+
+diff --git a/sd/source/ui/inc/DrawViewShell.hxx b/sd/source/ui/inc/DrawViewShell.hxx
+index 96c5c9c..a7b33d1 100644
+--- a/sd/source/ui/inc/DrawViewShell.hxx
++++ b/sd/source/ui/inc/DrawViewShell.hxx
+@@ -30,8 +30,10 @@
+ #include <com/sun/star/lang/XEventListener.hpp>
+ #include <com/sun/star/scanner/XScannerManager2.hpp>
+ #include <unotools/caserotate.hxx>
++#include <boost/shared_ptr.hpp>
+
+ class SdPage;
++class SdrExternalToolEdit;
+ class DrawDocShell;
+ class TabBar;
+ class SdrObject;
+@@ -501,6 +503,8 @@ private:
+
+ ::std::auto_ptr< AnnotationManager > mpAnnotationManager;
+ ::std::auto_ptr< ViewOverlayManager > mpViewOverlayManager;
++
++ std::vector<boost::shared_ptr<SdrExternalToolEdit> > m_ExternalEdits;
+ };
+
+
+diff --git a/sd/source/ui/view/drviews2.cxx b/sd/source/ui/view/drviews2.cxx
+index 384a6c7..688432e 100644
+--- a/sd/source/ui/view/drviews2.cxx
++++ b/sd/source/ui/view/drviews2.cxx
+@@ -186,33 +186,6 @@ using namespace ::com::sun::star::uno;
+
+ namespace sd {
+
+-class SdExternalToolEdit : public ExternalToolEdit
+-{
+- FmFormView* m_pView;
+- SdrObject* m_pObj;
+-
+-public:
+- SdExternalToolEdit ( FmFormView* pView, SdrObject* pObj ) :
+- m_pView (pView),
+- m_pObj (pObj)
+- {}
+-
+- virtual void Update( Graphic& aGraphic ) SAL_OVERRIDE
+- {
+- SdrPageView* pPageView = m_pView->GetSdrPageView();
+- if( pPageView )
+- {
+- SdrGrafObj* pNewObj = (SdrGrafObj*) m_pObj->Clone();
+- OUString aStr = m_pView->GetDescriptionOfMarkedObjects();
+- aStr += " External Edit";
+- m_pView->BegUndo( aStr );
+- pNewObj->SetGraphicObject( aGraphic );
+- m_pView->ReplaceObjectAtView( m_pObj, *pPageView, pNewObj );
+- m_pView->EndUndo();
+- }
+- }
+-};
+-
+ /**
+ * SfxRequests for temporary actions
+ */
+@@ -1001,9 +974,11 @@ void DrawViewShell::FuTemporary(SfxRequest& rReq)
+ SdrObject* pObj = rMarkList.GetMark( 0 )->GetMarkedSdrObj();
+ if( pObj && pObj->ISA( SdrGrafObj ) && ( (SdrGrafObj*) pObj )->GetGraphicType() == GRAPHIC_BITMAP )
+ {
+- GraphicObject aGraphicObject( ( (SdrGrafObj*) pObj )->GetGraphicObject() );
+- SdExternalToolEdit* aExternalToolEdit = new SdExternalToolEdit( mpDrawView, pObj );
+- aExternalToolEdit->Edit( &aGraphicObject );
++ GraphicObject aGraphicObject( static_cast<SdrGrafObj*>(pObj)->GetGraphicObject() );
++ m_ExternalEdits.push_back(
++ boost::shared_ptr<SdrExternalToolEdit>(
++ new SdrExternalToolEdit(mpDrawView, pObj)));
++ m_ExternalEdits.back()->Edit( &aGraphicObject );
+ }
+ }
+ Cancel();
+diff --git a/sd/source/ui/view/drviewsa.cxx b/sd/source/ui/view/drviewsa.cxx
+index 8e1e09d..e26759e 100644
+--- a/sd/source/ui/view/drviewsa.cxx
++++ b/sd/source/ui/view/drviewsa.cxx
+@@ -45,6 +45,7 @@
+ #include <svx/fmshell.hxx>
+ #include <svtools/cliplistener.hxx>
+ #include <svx/float3d.hxx>
++#include <svx/extedit.hxx>
+ #include <svx/sidebar/SelectionAnalyzer.hxx>
+ #include "helpids.h"
+
+diff --git a/svx/source/core/extedit.cxx b/svx/source/core/extedit.cxx
+index 24e93f9..fba280d 100644
+--- a/svx/source/core/extedit.cxx
++++ b/svx/source/core/extedit.cxx
+@@ -7,15 +7,21 @@
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
++#include <svx/extedit.hxx>
++
+ #include <vcl/svapp.hxx>
+ #include <vcl/graph.hxx>
+ #include <vcl/cvtgrf.hxx>
+ #include <vcl/graphicfilter.hxx>
+ #include <svx/xoutbmp.hxx>
+-#include <svx/extedit.hxx>
+ #include <svx/graphichelper.hxx>
++#include <svx/svdpagv.hxx>
++#include <svx/svdograf.hxx>
++#include <svx/fmview.hxx>
++#include <svtools/grfmgr.hxx>
+ #include <sfx2/viewfrm.hxx>
+ #include <sfx2/bindings.hxx>
++#include <salhelper/thread.hxx>
+ #include <osl/file.hxx>
+ #include <osl/thread.hxx>
+ #include <osl/process.h>
+@@ -32,7 +38,6 @@ using namespace css::uno;
+ using namespace css::system;
+
+ ExternalToolEdit::ExternalToolEdit()
+- : m_pGraphicObject(NULL)
+ {
+ }
+
+@@ -57,33 +62,40 @@ void ExternalToolEdit::HandleCloseEvent(ExternalToolEdit* pData)
+ }
+ }
+
+-IMPL_LINK (ExternalToolEdit, StartListeningEvent, void*, pEvent)
++void ExternalToolEdit::StartListeningEvent()
+ {
+ //Start an event listener implemented via VCL timeout
+- ExternalToolEdit* pData = ( ExternalToolEdit* )pEvent;
+-
+- new FileChangedChecker(pData->m_aFileName, ::boost::bind(&HandleCloseEvent, pData));
+-
+- return 0;
++ assert(!m_pChecker.get());
++ m_pChecker.reset(new FileChangedChecker(
++ m_aFileName, ::boost::bind(&HandleCloseEvent, this)));
+ }
+
+-void ExternalToolEdit::threadWorker(void* pThreadData)
++// self-destructing thread to make shell execute async
++class ExternalToolEditThread
++ : public ::salhelper::Thread
+ {
+- ExternalToolEdit* pData = (ExternalToolEdit*) pThreadData;
++private:
++ OUString const m_aFileName;
++
++ virtual void execute() SAL_OVERRIDE;
+
+- // Make an asynchronous call to listen to the event of temporary image file
+- // getting changed
+- Application::PostUserEvent( LINK( NULL, ExternalToolEdit, StartListeningEvent ), pThreadData);
++public:
++ ExternalToolEditThread(OUString const& rFileName)
++ : ::salhelper::Thread("ExternalToolEdit")
++ , m_aFileName(rFileName)
++ {}
++};
+
++void ExternalToolEditThread::execute()
++{
+ Reference<XSystemShellExecute> xSystemShellExecute(
+ SystemShellExecute::create( ::comphelper::getProcessComponentContext() ) );
+- xSystemShellExecute->execute( pData->m_aFileName, OUString(), SystemShellExecuteFlags::URIS_ONLY );
++ xSystemShellExecute->execute(m_aFileName, OUString(), SystemShellExecuteFlags::URIS_ONLY);
+ }
+
+-void ExternalToolEdit::Edit( GraphicObject* pGraphicObject )
++void ExternalToolEdit::Edit(GraphicObject const*const pGraphicObject)
+ {
+ //Get the graphic from the GraphicObject
+- m_pGraphicObject = pGraphicObject;
+ const Graphic aGraphic = pGraphicObject->GetGraphic();
+
+ //get the Preferred File Extension for this graphic
+@@ -116,8 +128,57 @@ void ExternalToolEdit::Edit( GraphicObject* pGraphicObject )
+
+ //Create a thread
+
+- // Create the data that is needed by the thread later
+- osl_createThread(ExternalToolEdit::threadWorker, this);
++ rtl::Reference<ExternalToolEditThread> const pThread(
++ new ExternalToolEditThread(m_aFileName));
++ pThread->launch();
++
++ StartListeningEvent();
++}
++
++SdrExternalToolEdit::SdrExternalToolEdit(
++ FmFormView *const pView, SdrObject *const pObj)
++ : m_pView(pView)
++ , m_pObj(pObj)
++{
++ assert(m_pObj && m_pView);
++ StartListening(*m_pObj->GetModel());
++}
++
++
++void SdrExternalToolEdit::Notify(SfxBroadcaster & rBC, SfxHint const& rHint)
++{
++ SdrHint const*const pSdrHint(dynamic_cast<SdrHint const*>(&rHint));
++ if (pSdrHint
++ && (HINT_MODELCLEARED == pSdrHint->GetKind()
++ || (pSdrHint->GetObject() == m_pObj
++ && HINT_OBJREMOVED == pSdrHint->GetKind())))
++ {
++ m_pView = 0;
++ m_pObj = 0;
++ m_pChecker.reset(); // avoid modifying deleted object
++ EndListening(rBC);
++ }
++}
++
++void SdrExternalToolEdit::Update(Graphic & rGraphic)
++{
++ assert(m_pObj && m_pView); // timer should be deleted by Notify() too
++ SdrPageView *const pPageView = m_pView->GetSdrPageView();
++ if (pPageView)
++ {
++ SdrGrafObj *const pNewObj(static_cast<SdrGrafObj*>(m_pObj->Clone()));
++ assert(pNewObj);
++ OUString const description =
++ m_pView->GetDescriptionOfMarkedObjects() + " External Edit";
++ m_pView->BegUndo(description);
++ pNewObj->SetGraphicObject(rGraphic);
++ // set to new object before ReplaceObjectAtView() so that Notify() will
++ // not delete the running timer and crash
++ SdrObject *const pOldObj = m_pObj;
++ m_pObj = pNewObj;
++ m_pView->ReplaceObjectAtView(pOldObj, *pPageView, pNewObj);
++ m_pView->EndUndo();
++ }
+ }
+
+ /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
+diff --git a/sw/source/core/uibase/inc/grfsh.hxx b/sw/source/core/uibase/inc/grfsh.hxx
+index bfbdaed..48f8afe 100644
+--- a/sw/source/core/uibase/inc/grfsh.hxx
++++ b/sw/source/core/uibase/inc/grfsh.hxx
+@@ -20,9 +20,13 @@
+ #define INCLUDED_SW_SOURCE_CORE_UIBASE_INC_GRFSH_HXX
+
+ #include "frmsh.hxx"
++#include <boost/shared_ptr.hpp>
+
+ class SwGrfShell: public SwBaseShell
+ {
++ class SwExternalToolEdit;
++ std::vector<boost::shared_ptr<SwExternalToolEdit> > m_ExternalEdits;
++
+ public:
+ SFX_DECL_INTERFACE(SW_GRFSHELL)
+
+@@ -39,6 +43,7 @@ public:
+ void GetAttrStateForRotation(SfxItemSet& rRequest);
+
+ SwGrfShell(SwView &rView);
++ virtual ~SwGrfShell();
+ };
+
+ #endif
+diff --git a/sw/source/core/uibase/shells/grfsh.cxx b/sw/source/core/uibase/shells/grfsh.cxx
+index e81d1bf..1c4042d 100644
+--- a/sw/source/core/uibase/shells/grfsh.cxx
++++ b/sw/source/core/uibase/shells/grfsh.cxx
+@@ -74,26 +74,37 @@
+ #include "swslots.hxx"
+
+ #include "swabstdlg.hxx"
++#include <unocrsr.hxx>
++#include <boost/scoped_ptr.hpp>
+
+ #define TOOLBOX_NAME "colorbar"
+
+-namespace
++class SwGrfShell::SwExternalToolEdit
++ : public ExternalToolEdit
+ {
+- class SwExternalToolEdit : public ExternalToolEdit
++private:
++ SwWrtShell *const m_pShell;
++ ::boost::scoped_ptr<SwUnoCrsr> const m_pCursor;
++
++public:
++ SwExternalToolEdit(SwWrtShell *const pShell)
++ : m_pShell(pShell)
++ , m_pCursor( // need only Point, must point to SwGrfNode
++ pShell->GetDoc()->CreateUnoCrsr(
++ *pShell->GetCurrentShellCursor().GetPoint()))
+ {
+- SwWrtShell* m_pShell;
+-
+- public:
+- SwExternalToolEdit ( SwWrtShell* pShell ) :
+- m_pShell (pShell)
+- {}
++ }
+
+- virtual void Update( Graphic& aGraphic ) SAL_OVERRIDE
+- {
+- m_pShell->ReRead(OUString(), OUString(), (const Graphic*) &aGraphic);
+- }
+- };
+-}
++ virtual void Update(Graphic & rGraphic) SAL_OVERRIDE
++ {
++ DBG_TESTSOLARMUTEX();
++ m_pShell->Push();
++ m_pShell->GetCurrentShellCursor().DeleteMark();
++ *m_pShell->GetCurrentShellCursor().GetPoint() = *m_pCursor->GetPoint();
++ m_pShell->ReRead(OUString(), OUString(), &rGraphic);
++ m_pShell->Pop();
++ }
++};
+
+ SFX_IMPL_INTERFACE(SwGrfShell, SwBaseShell, SW_RES(STR_SHELLNAME_GRAPHIC))
+
+@@ -179,11 +190,12 @@ void SwGrfShell::Execute(SfxRequest &rReq)
+ {
+ // When the graphic is selected to be opened via some external tool
+ // for advanced editing
+- GraphicObject *pGraphicObject = (GraphicObject *) rSh.GetGraphicObj();
++ GraphicObject const*const pGraphicObject(rSh.GetGraphicObj());
+ if(0 != pGraphicObject)
+ {
+- SwExternalToolEdit* externalToolEdit = new SwExternalToolEdit( &rSh );
+- externalToolEdit->Edit ( pGraphicObject );
++ m_ExternalEdits.push_back(boost::shared_ptr<SwExternalToolEdit>(
++ new SwExternalToolEdit(&rSh)));
++ m_ExternalEdits.back()->Edit(pGraphicObject);
+ }
+ }
+ break;
+@@ -884,6 +896,10 @@ void SwGrfShell::GetAttrStateForRotation(SfxItemSet &rSet)
+ SetGetStateSet( 0 );
+ }
+
++SwGrfShell::~SwGrfShell()
++{
++}
++
+ SwGrfShell::SwGrfShell(SwView &_rView) :
+ SwBaseShell(_rView)
+ {
+--
+2.1.0
+
diff --git a/libreoffice.spec b/libreoffice.spec
index 013fd37..cbf0d83 100644
--- a/libreoffice.spec
+++ b/libreoffice.spec
@@ -2343,6 +2343,7 @@ update-desktop-database %{_datadir}/applications &> /dev/null || :
%changelog
* Mon Jan 19 2015 Caolán McNamara <caolanm at redhat.com> - 1:4.3.5.2-12-UNBUILT
- if we change the keys we have to resort based on the new keys
+- Resolves: rhbz#1136013 ExternalToolEdit crash
* Fri Jan 16 2015 Eike Rathke <erack at redhat.com> - 1:4.3.5.2-11
- Resolves: rhbz#1171828 fdo#86978 append formula cells to track instead of tree
More information about the scm-commits
mailing list