Blob Blame History Raw
From 4757c4173b9c093536d91ddbb0800b92e0842cc7 Mon Sep 17 00:00:00 2001
From: Michael Stahl <mstahl@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@redhat.com>
Reviewed-by: Caolán McNamara <caolanm@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