rhbz#1136013: svx: try to make the ExternalToolEdit not crash all the time

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...)

Change-Id: I35f187f0828097a05618dc1733dce819fc6bffc6

include/svx/extedit.hxx

@@ -10,30 +10,51 @@
 #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>
+
+class Graphic;
+class GraphicObject;
+class FileChangedChecker;
 
 class SVX_DLLPUBLIC ExternalToolEdit
 {
-public:
-    GraphicObject* m_pGraphicObject;
+protected:
     OUString m_aFileName;
 
+    ::std::unique_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

sc/source/ui/drawfunc/graphsh.cxx

@@ -38,32 +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 = static_cast<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)
 
 void ScGraphicShell::InitInterface_Impl()
@@ -188,8 +162,9 @@ void ScGraphicShell::ExecuteExternalEdit( SfxRequest& )
         if( pObj && pObj->ISA( SdrGrafObj ) && static_cast<SdrGrafObj*>(pObj)->GetGraphicType() == GRAPHIC_BITMAP )
         {
             GraphicObject aGraphicObject( static_cast<SdrGrafObj*>(pObj)->GetGraphicObject() );
-            ScExternalToolEdit* aExternalToolEdit = new ScExternalToolEdit( pView, pObj );
-            aExternalToolEdit->Edit( &aGraphicObject );
+            m_ExternalEdits.push_back( std::unique_ptr<SdrExternalToolEdit>(
+                        new SdrExternalToolEdit(pView, pObj)));
+            m_ExternalEdits.back()->Edit( &aGraphicObject );
         }
     }
 

sc/source/ui/inc/graphsh.hxx

@@ -25,6 +25,7 @@
 #include <sfx2/module.hxx>
 #include <svx/svdmark.hxx>
 
+class SdrExternalToolEdit;
 class ScViewData;
 
 #include "drawsh.hxx"
@@ -36,6 +37,8 @@ public:
     SFX_DECL_INTERFACE(SCID_GRAPHIC_SHELL)
 
 private:
+    std::vector<std::unique_ptr<SdrExternalToolEdit>> m_ExternalEdits;
+
     /// SfxInterface initializer.
     static void InitInterface_Impl();
 

sd/source/ui/inc/DrawViewShell.hxx

@@ -33,6 +33,7 @@
 
 class Outliner;
 class SdPage;
+class SdrExternalToolEdit;
 class DrawDocShell;
 class TabBar;
 class SdrObject;
@@ -507,6 +508,8 @@ private:
 
     ::std::unique_ptr< AnnotationManager > mpAnnotationManager;
     ::std::unique_ptr< ViewOverlayManager > mpViewOverlayManager;
+
+    std::vector<std::unique_ptr<SdrExternalToolEdit>> m_ExternalEdits;
 };
 
 } // end of namespace sd

sd/source/ui/view/drviews2.cxx

@@ -185,33 +185,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 = static_cast<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
  */
@@ -999,8 +972,10 @@ void DrawViewShell::FuTemporary(SfxRequest& rReq)
                 if( pObj && pObj->ISA( SdrGrafObj ) && static_cast<SdrGrafObj*>(pObj)->GetGraphicType() == GRAPHIC_BITMAP )
                 {
                     GraphicObject aGraphicObject( static_cast<SdrGrafObj*>(pObj)->GetGraphicObject() );
-                    SdExternalToolEdit* aExternalToolEdit = new SdExternalToolEdit( mpDrawView, pObj );
-                    aExternalToolEdit->Edit( &aGraphicObject );
+                    m_ExternalEdits.push_back(
+                        std::unique_ptr<SdrExternalToolEdit>(
+                            new SdrExternalToolEdit(mpDrawView, pObj)));
+                    m_ExternalEdits.back()->Edit( &aGraphicObject );
                 }
             }
             Cancel();

sd/source/ui/view/drviewsa.cxx

@@ -43,6 +43,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"
 

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>
@@ -33,7 +39,6 @@ using namespace css::uno;
 using namespace css::system;
 
 ExternalToolEdit::ExternalToolEdit()
-    : m_pGraphicObject(NULL)
 {
 }
 
@@ -56,35 +61,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
 {
-    osl_setThreadName("ExternalToolEdit");
+private:
+    OUString const m_aFileName;
 
-    ExternalToolEdit* pData = (ExternalToolEdit*) pThreadData;
+    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
@@ -117,8 +127,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: */

sw/source/uibase/inc/grfsh.hxx

@@ -23,6 +23,9 @@
 
 class SwGrfShell: public SwBaseShell
 {
+    class SwExternalToolEdit;
+    std::vector<std::unique_ptr<SwExternalToolEdit>> m_ExternalEdits;
+
 public:
     SFX_DECL_INTERFACE(SW_GRFSHELL)
 
@@ -39,6 +42,7 @@ public:
     void GetAttrStateForRotation(SfxItemSet& rRequest);
 
     SwGrfShell(SwView &rView);
+    virtual ~SwGrfShell();
 };
 
 #endif

sw/source/uibase/shells/grfsh.cxx

@@ -75,27 +75,37 @@
 #include <sfx2/msg.hxx>
 #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;
+    ::std::unique_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)
 
@@ -180,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(std::unique_ptr<SwExternalToolEdit>(
+                            new SwExternalToolEdit(&rSh)));
+                m_ExternalEdits.back()->Edit(pGraphicObject);
             }
         }
         break;
@@ -905,6 +916,10 @@ void SwGrfShell::GetAttrStateForRotation(SfxItemSet &rSet)
     SetGetStateSet( 0 );
 }
 
+SwGrfShell::~SwGrfShell()
+{
+}
+
 SwGrfShell::SwGrfShell(SwView &_rView) :
     SwBaseShell(_rView)
 {