[Libreoffice-commits] core.git: sw/inc sw/source

Michael Stahl mstahl at redhat.com
Fri Mar 24 13:39:55 UTC 2017


 sw/inc/accmap.hxx                         |    5 ++-
 sw/source/core/access/acccell.cxx         |    2 -
 sw/source/core/access/acccell.hxx         |    3 +-
 sw/source/core/access/acccontext.cxx      |   42 ++++++++++++++++++++++++++----
 sw/source/core/access/acccontext.hxx      |   12 ++++++--
 sw/source/core/access/accdoc.cxx          |   18 +++++++-----
 sw/source/core/access/accdoc.hxx          |    4 +-
 sw/source/core/access/accembedded.cxx     |    2 -
 sw/source/core/access/accembedded.hxx     |    2 -
 sw/source/core/access/accfootnote.cxx     |    2 -
 sw/source/core/access/accfootnote.hxx     |    2 -
 sw/source/core/access/accframebase.cxx    |    2 -
 sw/source/core/access/accframebase.hxx    |    2 -
 sw/source/core/access/accgraphic.cxx      |    2 -
 sw/source/core/access/accgraphic.hxx      |    2 -
 sw/source/core/access/accheaderfooter.cxx |    4 +-
 sw/source/core/access/accheaderfooter.hxx |    4 +-
 sw/source/core/access/accmap.cxx          |   35 +++++++++++++------------
 sw/source/core/access/accnotextframe.cxx  |    2 -
 sw/source/core/access/accnotextframe.hxx  |    2 -
 sw/source/core/access/accpage.cxx         |    2 -
 sw/source/core/access/accpage.hxx         |    3 +-
 sw/source/core/access/accpara.cxx         |    2 -
 sw/source/core/access/accpara.hxx         |    2 -
 sw/source/core/access/accpreview.cxx      |    4 +-
 sw/source/core/access/accpreview.hxx      |    2 -
 sw/source/core/access/acctable.cxx        |   12 +++++---
 sw/source/core/access/acctable.hxx        |    6 ++--
 sw/source/core/access/acctextframe.cxx    |    2 -
 sw/source/core/access/acctextframe.hxx    |    3 +-
 sw/source/core/inc/viewimp.hxx            |    4 ++
 sw/source/core/view/viewimp.cxx           |    8 ++---
 32 files changed, 127 insertions(+), 72 deletions(-)

New commits:
commit 0c2229dcab143925c6ad390e0735e2d98c3eecca
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Mar 24 13:04:32 2017 +0100

    tdf#58624 sw: fix ~SwAccessibleContext() use-after-free race
    
    As seen in JunitTest_toolkit_unoapi_1:
    
    Method doAccessibleAction() finished with state OK
    LOG> doAccessibleAction(): COMPLETED.OK
    debug:27272:12:  -SwAccessibleParagraph mutexwait 0x3fd9f50
    debug:27272:9:  SwAccessibleContext::Dispose 0x3872620 11SwRootFrame
    debug:27272:9:  SwAccessibleContext::DisposeChildren 0x4047c80 0x386d600 11SwPageFrame
    debug:27272:9:  SwAccessibleContext::DisposeChildren xAcc 0
    debug:27272:9:  SwAccessibleContext::DisposeChildren 0x4047c80 0x386cef0 11SwBodyFrame
    debug:27272:9:  SwAccessibleContext::DisposeChildren xAcc 0
    debug:27272:9:  SwAccessibleContext::DisposeChildren 0x4047c80 0x3878fe0 11SwTextFrame
    debug:27272:9:  SwAccessibleContext::DisposeChildren xAcc 0
    debug:27272:9:  SwAccessibleMap::RemoveContext erase 0x3872620
    debug:27272:9:  ~SwAccessibleMap: frame entry 0x3878fe0
    debug:27272:9:  ~SwAccessibleMap: mpFrameMap 0x3eb64a0
    debug:27272:9:  ~SwAccessibleMap: mpShapeMap 0
    soffice.bin: sw/source/core/access/accmap.cxx:1726: virtual SwAccessibleMap::~SwAccessibleMap(): Assertion `(!mpFrameMap || mpFrameMap->empty()) && "Frame map should be empty after disposing the root frame"'
    
    The problem here is that thread 12 is blocked on SolarMutex in
    ~SwAccessibleParagraph(), while thread 9 is in ~SwAccessibleMap().
    
    This means that in SwAccessibleContext::DisposeChildren(), the
    WeakReference to the SwAccessibleParagraph cannot create a
    uno::Reference because its reference count is 0, so
    SwAccessibleContext::Dispose() is not called on it and it remains
    in the SwAccessibleMap::mpFrameMap.
    
    This triggers the assert and later on ~SwAccessibleContext() would
    access the deleted SwAccessibleMap and crash.
    
    To fix this, introduce a weak reference from SwAccessibleContext to
    SwAccessibleMap; use a std::weak_ptr because that is not derived from
    OWeakObject.
    
    The weak_ptr is only used in the dtor ~SwAccessibleContext(); as
    long as the ref-count of SwAccessibleContext is > 0 it is guaranteed
    that the SwAccessibleContext::m_pMap is either null or valid
    as the recursive Dispose() will work fine.
    
    It is possible that additional temporary owning references could delay
    the destruction of SwAccessibleMap, and the order of destruction
    of Writer documents is very fragile, so rely on the SolarMutex lock
    to prevent that; the only shared_ptr that owns SwAccessibleMap while
    SolarMutex is not locked is the one in SwViewShellImp.
    
    (An alternative fix would be to represent the 3 lifecycle stages of
    SwAccessibleContext by adding a C++-pointer to the
    SwAccessibleMap::mpFrameMap, so that DisposeChildren() can, if
    the WeakReference is no longer valid due to ref-count 0, use the
    pointer and clear SwAccessibleContext::m_pMap - this and the
    corresponding call to SwAccessibleMap::RemoveContext() from
    ~SwAccessibleContext() under a mutex that is shared_ptr-owned by
    SwAccessibleMap and all SwAccessibleContext.)
    
    Change-Id: If2b44c79189e3b3d276491a5c57d5404bb2be71a

diff --git a/sw/inc/accmap.hxx b/sw/inc/accmap.hxx
index 9d155dcb2edb..d59852b671c2 100644
--- a/sw/inc/accmap.hxx
+++ b/sw/inc/accmap.hxx
@@ -30,10 +30,12 @@
 #include <svx/AccessibleControlShape.hxx>
 #include <svx/AccessibleShape.hxx>
 #include "fesh.hxx"
+#include <o3tl/typed_flags_set.hxx>
+
 #include <list>
 #include <vector>
+#include <memory>
 #include <set>
-#include <o3tl/typed_flags_set.hxx>
 
 class SwAccessibleParagraph;
 class SwViewShell;
@@ -88,6 +90,7 @@ namespace o3tl
 
 class SwAccessibleMap : public ::accessibility::IAccessibleViewForwarder,
                         public ::accessibility::IAccessibleParent
+                , public std::enable_shared_from_this<SwAccessibleMap>
 {
     mutable ::osl::Mutex maMutex;
     ::osl::Mutex maEventMutex;
diff --git a/sw/source/core/access/acccell.cxx b/sw/source/core/access/acccell.cxx
index 7d9b3c702934..a33276248b08 100644
--- a/sw/source/core/access/acccell.cxx
+++ b/sw/source/core/access/acccell.cxx
@@ -98,7 +98,7 @@ void SwAccessibleCell::GetStates( ::utl::AccessibleStateSetHelper& rStateSet )
     }
 }
 
-SwAccessibleCell::SwAccessibleCell( SwAccessibleMap *pInitMap,
+SwAccessibleCell::SwAccessibleCell(std::shared_ptr<SwAccessibleMap> const& pInitMap,
                                     const SwCellFrame *pCellFrame )
     : SwAccessibleContext( pInitMap, AccessibleRole::TABLE_CELL, pCellFrame )
     , m_aSelectionHelper( *this )
diff --git a/sw/source/core/access/acccell.hxx b/sw/source/core/access/acccell.hxx
index 213a4c1ff415..9f3472067b01 100644
--- a/sw/source/core/access/acccell.hxx
+++ b/sw/source/core/access/acccell.hxx
@@ -54,7 +54,8 @@ protected:
     virtual ~SwAccessibleCell() override;
 
 public:
-    SwAccessibleCell( SwAccessibleMap* pInitMap, const SwCellFrame *pCellFrame );
+    SwAccessibleCell(std::shared_ptr<SwAccessibleMap> const& pInitMap,
+                     const SwCellFrame *pCellFrame);
 
     virtual bool HasCursor() override;   // required by map to remember that object
 
diff --git a/sw/source/core/access/acccontext.cxx b/sw/source/core/access/acccontext.cxx
index 2914b270a1fc..f2288a51a3b6 100644
--- a/sw/source/core/access/acccontext.cxx
+++ b/sw/source/core/access/acccontext.cxx
@@ -406,8 +406,21 @@ void SwAccessibleContext::DisposeChildren(const SwFrame *pFrame,
                 xAccImpl = GetMap()->GetContextImpl( pLower, false );
             if( xAccImpl.is() )
                 xAccImpl->Dispose( bRecursive );
-            else if( bRecursive )
-                DisposeChildren(pLower, bRecursive, bCanSkipInvisible);
+            else
+            {
+                // it's possible that the xAccImpl *does* exist with a
+                // ref-count of 0 and blocked in its dtor in another thread -
+                // this call here could be from SwAccessibleMap dtor so
+                // remove it from any maps now!
+                GetMap()->RemoveContext(pLower);
+                // in this case the context will check with a weak_ptr
+                // that the map is still alive so it's not necessary
+                // to clear its m_pMap here.
+                if (bRecursive)
+                {
+                    DisposeChildren(pLower, bRecursive, bCanSkipInvisible);
+                }
+            }
         }
         else if ( rLower.GetDrawObject() )
         {
@@ -517,12 +530,13 @@ bool SwAccessibleContext::IsEditableState()
     return bRet;
 }
 
-SwAccessibleContext::SwAccessibleContext( SwAccessibleMap *const pMap,
+SwAccessibleContext::SwAccessibleContext(std::shared_ptr<SwAccessibleMap> const& pMap,
                                           sal_Int16 const nRole,
                                           const SwFrame *pF )
     : SwAccessibleFrame( pMap->GetVisArea().SVRect(), pF,
                          pMap->GetShell()->IsPreview() )
-    , m_pMap( pMap )
+    , m_pMap(pMap.get())
+    , m_wMap(pMap)
     , m_nClientId(0)
     , m_nRole(nRole)
     , m_isDisposing( false )
@@ -534,8 +548,16 @@ SwAccessibleContext::SwAccessibleContext( SwAccessibleMap *const pMap,
 
 SwAccessibleContext::~SwAccessibleContext()
 {
+    // must have for 2 reasons: 2. as long as this thread has SolarMutex
+    // another thread cannot destroy the SwAccessibleMap so our temporary
+    // taking a hard ref to SwAccessibleMap won't delay its destruction
     SolarMutexGuard aGuard;
-    RemoveFrameFromAccessibleMap();
+    // must check with weak_ptr that m_pMap is still alive
+    std::shared_ptr<SwAccessibleMap> pMap(m_wMap.lock());
+    if (m_isRegisteredAtAccessibleMap && GetFrame() && pMap)
+    {
+        pMap->RemoveContext( GetFrame() );
+    }
 }
 
 uno::Reference< XAccessibleContext > SAL_CALL
@@ -1049,6 +1071,7 @@ void SwAccessibleContext::Dispose(bool bRecursive, bool bCanSkipInvisible)
     RemoveFrameFromAccessibleMap();
     ClearFrame();
     m_pMap = nullptr;
+    m_wMap.reset();
 
     m_isDisposing = false;
 }
@@ -1406,8 +1429,17 @@ OUString SwAccessibleContext::GetResource( sal_uInt16 nResId,
     return sStr;
 }
 
+
+void SwAccessibleContext::ClearMapPointer()
+{
+    DBG_TESTSOLARMUTEX();
+    m_pMap = nullptr;
+    m_wMap.reset();
+}
+
 void SwAccessibleContext::RemoveFrameFromAccessibleMap()
 {
+    assert(m_refCount > 0); // must be alive to do this without using m_wMap
     if (m_isRegisteredAtAccessibleMap && GetFrame() && GetMap())
         GetMap()->RemoveContext( GetFrame() );
 }
diff --git a/sw/source/core/access/acccontext.hxx b/sw/source/core/access/acccontext.hxx
index dde2c82e4cde..d37bb8e7768d 100644
--- a/sw/source/core/access/acccontext.hxx
+++ b/sw/source/core/access/acccontext.hxx
@@ -29,6 +29,8 @@
 #include <cppuhelper/implbase.hxx>
 #include <cppuhelper/interfacecontainer.hxx>
 
+#include <memory>
+
 namespace vcl { class Window; }
 class SwAccessibleMap;
 class SwCursorShell;
@@ -70,6 +72,10 @@ private:
         css::accessibility::XAccessible > m_xWeakParent;
 
     SwAccessibleMap *m_pMap; // must be protected by solar mutex
+    /// note: the m_pMap is guaranteed to be valid until we hit the
+    /// dtor ~SwAccessibleContext, then m_wMap must be checked if it's still
+    /// alive, after locking SolarMutex (alternatively, Dispose clears m_pMap)
+    std::weak_ptr<SwAccessibleMap> m_wMap;
 
     sal_uInt32 m_nClientId;  // client id in the AccessibleEventNotifier queue
     sal_Int16 m_nRole;        // immutable outside constructor
@@ -161,7 +167,7 @@ protected:
     virtual void InvalidateFocus_();
 
 public:
-    void SetMap(SwAccessibleMap *const pMap) { m_pMap = pMap; }
+    void ClearMapPointer();
     void FireAccessibleEvent( css::accessibility::AccessibleEventObject& rEvent );
 
 protected:
@@ -192,8 +198,8 @@ protected:
     virtual ~SwAccessibleContext() override;
 
 public:
-    SwAccessibleContext( SwAccessibleMap *m_pMap, sal_Int16 nRole,
-                         const SwFrame *pFrame );
+    SwAccessibleContext( std::shared_ptr<SwAccessibleMap> const& pMap,
+                         sal_Int16 nRole, const SwFrame *pFrame );
 
     // XAccessible
 
diff --git a/sw/source/core/access/accdoc.cxx b/sw/source/core/access/accdoc.cxx
index 84e7298487f2..c332ea858d31 100644
--- a/sw/source/core/access/accdoc.cxx
+++ b/sw/source/core/access/accdoc.cxx
@@ -69,11 +69,12 @@ using lang::IndexOutOfBoundsException;
 // SwAccessibleDocumentBase: base class for SwAccessibleDocument and
 // SwAccessiblePreview
 
-SwAccessibleDocumentBase::SwAccessibleDocumentBase ( SwAccessibleMap *_pMap ) :
-    SwAccessibleContext( _pMap, AccessibleRole::DOCUMENT_TEXT,
-                         _pMap->GetShell()->GetLayout() ),
-    mxParent( _pMap->GetShell()->GetWin()->GetAccessibleParentWindow()->GetAccessible() ),
-    mpChildWin( nullptr )
+SwAccessibleDocumentBase::SwAccessibleDocumentBase(
+        std::shared_ptr<SwAccessibleMap> const& pMap)
+    : SwAccessibleContext(pMap, AccessibleRole::DOCUMENT_TEXT,
+                          pMap->GetShell()->GetLayout())
+    , mxParent(pMap->GetShell()->GetWin()->GetAccessibleParentWindow()->GetAccessible())
+    , mpChildWin(nullptr)
 {
 }
 
@@ -334,9 +335,10 @@ void SwAccessibleDocument::GetStates(
     rStateSet.AddState( AccessibleStateType::MANAGES_DESCENDANTS );
 }
 
-SwAccessibleDocument::SwAccessibleDocument ( SwAccessibleMap* pInitMap ) :
-    SwAccessibleDocumentBase( pInitMap ),
-    maSelectionHelper( *this )
+SwAccessibleDocument::SwAccessibleDocument(
+        std::shared_ptr<SwAccessibleMap> const& pInitMap)
+    : SwAccessibleDocumentBase(pInitMap)
+    , maSelectionHelper(*this)
 {
     SetName(pInitMap->GetDocName());
     vcl::Window *pWin = pInitMap->GetShell()->GetWin();
diff --git a/sw/source/core/access/accdoc.hxx b/sw/source/core/access/accdoc.hxx
index b1cd613d7ebd..9b1f2eae5963 100644
--- a/sw/source/core/access/accdoc.hxx
+++ b/sw/source/core/access/accdoc.hxx
@@ -42,7 +42,7 @@ protected:
     virtual ~SwAccessibleDocumentBase() override;
 
 public:
-    SwAccessibleDocumentBase( SwAccessibleMap* pInitMap );
+    SwAccessibleDocumentBase(std::shared_ptr<SwAccessibleMap> const& pInitMap);
 
     void SetVisArea();
 
@@ -108,7 +108,7 @@ protected:
     virtual ~SwAccessibleDocument() override;
 
 public:
-    SwAccessibleDocument( SwAccessibleMap* pInitMap );
+    SwAccessibleDocument(std::shared_ptr<SwAccessibleMap> const& pInitMap);
 
     DECL_LINK( WindowChildEventListener, VclWindowEvent&, void );
 
diff --git a/sw/source/core/access/accembedded.cxx b/sw/source/core/access/accembedded.cxx
index e0481fd46688..6d4fc0e4bd6c 100644
--- a/sw/source/core/access/accembedded.cxx
+++ b/sw/source/core/access/accembedded.cxx
@@ -36,7 +36,7 @@ using namespace ::com::sun::star::accessibility;
 const sal_Char sImplementationName[] = "com.sun.star.comp.Writer.SwAccessibleEmbeddedObject";
 
 SwAccessibleEmbeddedObject::SwAccessibleEmbeddedObject(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         const SwFlyFrame* pFlyFrame  ) :
     SwAccessibleNoTextFrame( pInitMap, AccessibleRole::EMBEDDED_OBJECT, pFlyFrame )
 {
diff --git a/sw/source/core/access/accembedded.hxx b/sw/source/core/access/accembedded.hxx
index 5d37eb4d455c..5506838fa64f 100644
--- a/sw/source/core/access/accembedded.hxx
+++ b/sw/source/core/access/accembedded.hxx
@@ -32,7 +32,7 @@ protected:
     virtual ~SwAccessibleEmbeddedObject() override;
 
 public:
-    SwAccessibleEmbeddedObject( SwAccessibleMap* pInitMap,
+    SwAccessibleEmbeddedObject(std::shared_ptr<SwAccessibleMap> const& pInitMap,
                                 const SwFlyFrame* pFlyFrame );
 
     // XInterface
diff --git a/sw/source/core/access/accfootnote.cxx b/sw/source/core/access/accfootnote.cxx
index 300b87495a69..5faefac2f2f1 100644
--- a/sw/source/core/access/accfootnote.cxx
+++ b/sw/source/core/access/accfootnote.cxx
@@ -40,7 +40,7 @@ const sal_Char sImplementationNameFootnote[] = "com.sun.star.comp.Writer.SwAcces
 const sal_Char sImplementationNameEndnote[] = "com.sun.star.comp.Writer.SwAccessibleEndnoteView";
 
 SwAccessibleFootnote::SwAccessibleFootnote(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         bool bIsEndnote,
         const SwFootnoteFrame *pFootnoteFrame ) :
     SwAccessibleContext( pInitMap,
diff --git a/sw/source/core/access/accfootnote.hxx b/sw/source/core/access/accfootnote.hxx
index 01970eb9e916..729cb7f5310f 100644
--- a/sw/source/core/access/accfootnote.hxx
+++ b/sw/source/core/access/accfootnote.hxx
@@ -32,7 +32,7 @@ protected:
     virtual ~SwAccessibleFootnote() override;
 
 public:
-    SwAccessibleFootnote( SwAccessibleMap* pInitMap,
+    SwAccessibleFootnote( std::shared_ptr<SwAccessibleMap> const& pInitMap,
                           bool bIsEndnote,
                           const SwFootnoteFrame *pFootnoteFrame );
 
diff --git a/sw/source/core/access/accframebase.cxx b/sw/source/core/access/accframebase.cxx
index 396cbb0571cf..8329ff586e6b 100644
--- a/sw/source/core/access/accframebase.cxx
+++ b/sw/source/core/access/accframebase.cxx
@@ -124,7 +124,7 @@ SwNodeType SwAccessibleFrameBase::GetNodeType( const SwFlyFrame *pFlyFrame )
 }
 
 SwAccessibleFrameBase::SwAccessibleFrameBase(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         sal_Int16 nInitRole,
         const SwFlyFrame* pFlyFrame  ) :
     SwAccessibleContext( pInitMap, nInitRole, pFlyFrame ),
diff --git a/sw/source/core/access/accframebase.hxx b/sw/source/core/access/accframebase.hxx
index 89ed1c06a8b1..ea6a1ba55097 100644
--- a/sw/source/core/access/accframebase.hxx
+++ b/sw/source/core/access/accframebase.hxx
@@ -48,7 +48,7 @@ protected:
     virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew) override;
 
 public:
-    SwAccessibleFrameBase( SwAccessibleMap* pInitMap,
+    SwAccessibleFrameBase(std::shared_ptr<SwAccessibleMap> const& pInitMap,
                            sal_Int16 nInitRole,
                            const SwFlyFrame *pFlyFrame );
 
diff --git a/sw/source/core/access/accgraphic.cxx b/sw/source/core/access/accgraphic.cxx
index 68952434684c..80b7d57e34a5 100644
--- a/sw/source/core/access/accgraphic.cxx
+++ b/sw/source/core/access/accgraphic.cxx
@@ -32,7 +32,7 @@ using namespace ::com::sun::star::uno;
 using namespace ::com::sun::star::accessibility;
 
 SwAccessibleGraphic::SwAccessibleGraphic(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         const SwFlyFrame* pFlyFrame  ) :
     SwAccessibleNoTextFrame( pInitMap, AccessibleRole::GRAPHIC, pFlyFrame )
 {
diff --git a/sw/source/core/access/accgraphic.hxx b/sw/source/core/access/accgraphic.hxx
index 038a3be708e2..dc21201bc039 100644
--- a/sw/source/core/access/accgraphic.hxx
+++ b/sw/source/core/access/accgraphic.hxx
@@ -28,7 +28,7 @@ protected:
     virtual ~SwAccessibleGraphic() override;
 
 public:
-    SwAccessibleGraphic( SwAccessibleMap* pInitMap,
+    SwAccessibleGraphic(std::shared_ptr<SwAccessibleMap> const& pInitMap,
                          const SwFlyFrame *pFlyFrame );
 
     // XServiceInfo
diff --git a/sw/source/core/access/accheaderfooter.cxx b/sw/source/core/access/accheaderfooter.cxx
index f9ecac61fc7a..f32a0b8b0d14 100644
--- a/sw/source/core/access/accheaderfooter.cxx
+++ b/sw/source/core/access/accheaderfooter.cxx
@@ -36,7 +36,7 @@ const sal_Char sImplementationNameHeader[] = "com.sun.star.comp.Writer.SwAccessi
 const sal_Char sImplementationNameFooter[] = "com.sun.star.comp.Writer.SwAccessibleFooterView";
 
 SwAccessibleHeaderFooter::SwAccessibleHeaderFooter(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         const SwHeaderFrame* pHdFrame    ) :
     SwAccessibleContext( pInitMap, AccessibleRole::HEADER, pHdFrame )
 {
@@ -47,7 +47,7 @@ SwAccessibleHeaderFooter::SwAccessibleHeaderFooter(
 }
 
 SwAccessibleHeaderFooter::SwAccessibleHeaderFooter(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         const SwFooterFrame* pFtFrame    ) :
     SwAccessibleContext( pInitMap, AccessibleRole::FOOTER, pFtFrame )
 {
diff --git a/sw/source/core/access/accheaderfooter.hxx b/sw/source/core/access/accheaderfooter.hxx
index ce7eaa406eb2..f948311c777f 100644
--- a/sw/source/core/access/accheaderfooter.hxx
+++ b/sw/source/core/access/accheaderfooter.hxx
@@ -31,9 +31,9 @@ protected:
     virtual ~SwAccessibleHeaderFooter() override;
 
 public:
-    SwAccessibleHeaderFooter( SwAccessibleMap* pInitMap,
+    SwAccessibleHeaderFooter( std::shared_ptr<SwAccessibleMap> const& pInitMap,
                               const SwHeaderFrame* pHdFrame );
-    SwAccessibleHeaderFooter( SwAccessibleMap* pInitMap,
+    SwAccessibleHeaderFooter( std::shared_ptr<SwAccessibleMap> const& pInitMap,
                               const SwFooterFrame* pFtFrame );
 
     // XAccessibleContext
diff --git a/sw/source/core/access/accmap.cxx b/sw/source/core/access/accmap.cxx
index 959652ef7484..5acee3a62889 100644
--- a/sw/source/core/access/accmap.cxx
+++ b/sw/source/core/access/accmap.cxx
@@ -1650,6 +1650,7 @@ SwAccessibleMap::SwAccessibleMap( SwViewShell *pSh ) :
 
 SwAccessibleMap::~SwAccessibleMap()
 {
+    DBG_TESTSOLARMUTEX();
     uno::Reference < XAccessible > xAcc;
     {
         osl::MutexGuard aGuard( maMutex );
@@ -1660,7 +1661,8 @@ SwAccessibleMap::~SwAccessibleMap()
             if( aIter != mpFrameMap->end() )
                 xAcc = (*aIter).second;
             if( !xAcc.is() )
-                xAcc = new SwAccessibleDocument( this );
+                assert(false); // let's hope this can't happen? the vcl::Window apparently owns the top-level
+                //xAcc = new SwAccessibleDocument(shared_from_this());
         }
     }
 
@@ -1678,7 +1680,8 @@ SwAccessibleMap::~SwAccessibleMap()
             if( xTmp.is() )
             {
                 SwAccessibleContext *pTmp = static_cast< SwAccessibleContext * >( xTmp.get() );
-                pTmp->SetMap(nullptr);
+                // TODO is this still needed
+                pTmp->ClearMapPointer();
             }
             ++aIter;
         }
@@ -1701,8 +1704,6 @@ SwAccessibleMap::~SwAccessibleMap()
                 ++aIter;
             }
         }
-        assert((!mpFrameMap || mpFrameMap->empty()) &&
-                "Frame map should be empty after disposing the root frame");
         if( mpShapeMap )
         {
             SwAccessibleShapeMap_Impl::iterator aIter = mpShapeMap->begin();
@@ -1718,6 +1719,8 @@ SwAccessibleMap::~SwAccessibleMap()
                 ++aIter;
             }
         }
+        assert((!mpFrameMap || mpFrameMap->empty()) &&
+                "Frame map should be empty after disposing the root frame");
         assert((!mpShapeMap || mpShapeMap->empty()) &&
                 "Object map should be empty after disposing the root frame");
 #endif
@@ -1781,9 +1784,9 @@ uno::Reference< XAccessible > SwAccessibleMap::GetDocumentView_(
         else
         {
             if( bPagePreview )
-                xAcc = new SwAccessiblePreview( this );
+                xAcc = new SwAccessiblePreview(shared_from_this());
             else
-                xAcc = new SwAccessibleDocument( this );
+                xAcc = new SwAccessibleDocument(shared_from_this());
 
             if( aIter != mpFrameMap->end() )
             {
@@ -1855,15 +1858,15 @@ uno::Reference< XAccessible> SwAccessibleMap::GetContext( const SwFrame *pFrame,
                 switch( pFrame->GetType() )
                 {
                 case SwFrameType::Txt:
-                    pAcc = new SwAccessibleParagraph( this,
+                    pAcc = new SwAccessibleParagraph(shared_from_this(),
                                     static_cast< const SwTextFrame& >( *pFrame ) );
                     break;
                 case SwFrameType::Header:
-                    pAcc = new SwAccessibleHeaderFooter( this,
+                    pAcc = new SwAccessibleHeaderFooter(shared_from_this(),
                                     static_cast< const SwHeaderFrame *>( pFrame ) );
                     break;
                 case SwFrameType::Footer:
-                    pAcc = new SwAccessibleHeaderFooter( this,
+                    pAcc = new SwAccessibleHeaderFooter(shared_from_this(),
                                     static_cast< const SwFooterFrame *>( pFrame ) );
                     break;
                 case SwFrameType::Ftn:
@@ -1872,7 +1875,7 @@ uno::Reference< XAccessible> SwAccessibleMap::GetContext( const SwFrame *pFrame,
                             static_cast < const SwFootnoteFrame * >( pFrame );
                         bool bIsEndnote =
                             SwAccessibleFootnote::IsEndnote( pFootnoteFrame );
-                        pAcc = new SwAccessibleFootnote( this, bIsEndnote,
+                        pAcc = new SwAccessibleFootnote(shared_from_this(), bIsEndnote,
                                     /*(bIsEndnote ? mnEndnote++ : mnFootnote++),*/
                                     pFootnoteFrame );
                     }
@@ -1884,29 +1887,29 @@ uno::Reference< XAccessible> SwAccessibleMap::GetContext( const SwFrame *pFrame,
                         switch( SwAccessibleFrameBase::GetNodeType( pFlyFrame ) )
                         {
                         case SwNodeType::Grf:
-                            pAcc = new SwAccessibleGraphic( this, pFlyFrame );
+                            pAcc = new SwAccessibleGraphic(shared_from_this(), pFlyFrame );
                             break;
                         case SwNodeType::Ole:
-                            pAcc = new SwAccessibleEmbeddedObject( this, pFlyFrame );
+                            pAcc = new SwAccessibleEmbeddedObject(shared_from_this(), pFlyFrame );
                             break;
                         default:
-                            pAcc = new SwAccessibleTextFrame( this, *pFlyFrame );
+                            pAcc = new SwAccessibleTextFrame(shared_from_this(), *pFlyFrame );
                             break;
                         }
                     }
                     break;
                 case SwFrameType::Cell:
-                    pAcc = new SwAccessibleCell( this,
+                    pAcc = new SwAccessibleCell(shared_from_this(),
                                     static_cast< const SwCellFrame *>( pFrame ) );
                     break;
                 case SwFrameType::Tab:
-                    pAcc = new SwAccessibleTable( this,
+                    pAcc = new SwAccessibleTable(shared_from_this(),
                                     static_cast< const SwTabFrame *>( pFrame ) );
                     break;
                 case SwFrameType::Page:
                     OSL_ENSURE( GetShell()->IsPreview(),
                                 "accessible page frames only in PagePreview" );
-                    pAcc = new SwAccessiblePage( this, pFrame );
+                    pAcc = new SwAccessiblePage(shared_from_this(), pFrame);
                     break;
                 default: break;
                 }
diff --git a/sw/source/core/access/accnotextframe.cxx b/sw/source/core/access/accnotextframe.cxx
index 623df89c74be..2ec43e900df7 100644
--- a/sw/source/core/access/accnotextframe.cxx
+++ b/sw/source/core/access/accnotextframe.cxx
@@ -57,7 +57,7 @@ const SwNoTextNode *SwAccessibleNoTextFrame::GetNoTextNode() const
 }
 
 SwAccessibleNoTextFrame::SwAccessibleNoTextFrame(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         sal_Int16 nInitRole,
         const SwFlyFrame* pFlyFrame  ) :
     SwAccessibleFrameBase( pInitMap, nInitRole, pFlyFrame ),
diff --git a/sw/source/core/access/accnotextframe.hxx b/sw/source/core/access/accnotextframe.hxx
index 746901455236..014effd850c0 100644
--- a/sw/source/core/access/accnotextframe.hxx
+++ b/sw/source/core/access/accnotextframe.hxx
@@ -51,7 +51,7 @@ protected:
     virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew) override;
 
 public:
-    SwAccessibleNoTextFrame( SwAccessibleMap* pInitMap,
+    SwAccessibleNoTextFrame( std::shared_ptr<SwAccessibleMap> const& pInitMap,
                              sal_Int16 nInitRole,
                              const SwFlyFrame *pFlyFrame );
 
diff --git a/sw/source/core/access/accpage.cxx b/sw/source/core/access/accpage.cxx
index 6c2d444af7d3..2486fc630e50 100644
--- a/sw/source/core/access/accpage.cxx
+++ b/sw/source/core/access/accpage.cxx
@@ -108,7 +108,7 @@ void SwAccessiblePage::InvalidateFocus_()
     }
 }
 
-SwAccessiblePage::SwAccessiblePage( SwAccessibleMap* pInitMap,
+SwAccessiblePage::SwAccessiblePage(std::shared_ptr<SwAccessibleMap> const& pInitMap,
                                     const SwFrame* pFrame )
     : SwAccessibleContext( pInitMap, AccessibleRole::PANEL, pFrame )
     , bIsSelected( false )
diff --git a/sw/source/core/access/accpage.hxx b/sw/source/core/access/accpage.hxx
index 61cd7a7457c3..355015756df7 100644
--- a/sw/source/core/access/accpage.hxx
+++ b/sw/source/core/access/accpage.hxx
@@ -51,7 +51,8 @@ protected:
 public:
     // convenience constructor to avoid typecast;
     // may only be called with SwPageFrame argument
-    SwAccessiblePage( SwAccessibleMap* pInitMap, const SwFrame* pFrame );
+    SwAccessiblePage(std::shared_ptr<SwAccessibleMap> const& pInitMap,
+                     const SwFrame* pFrame);
 
     // XAccessibleContext methods that need to be overridden
 
diff --git a/sw/source/core/access/accpara.cxx b/sw/source/core/access/accpara.cxx
index 6b93b7e1c574..b5185abd15ba 100644
--- a/sw/source/core/access/accpara.cxx
+++ b/sw/source/core/access/accpara.cxx
@@ -529,7 +529,7 @@ void SwAccessibleParagraph::InvalidateFocus_()
 }
 
 SwAccessibleParagraph::SwAccessibleParagraph(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         const SwTextFrame& rTextFrame )
     : SwClient( const_cast<SwTextNode*>(rTextFrame.GetTextNode()) ) // #i108125#
     , SwAccessibleContext( pInitMap, AccessibleRole::PARAGRAPH, &rTextFrame )
diff --git a/sw/source/core/access/accpara.hxx b/sw/source/core/access/accpara.hxx
index 2413a4de6ce3..685a35d15ca1 100644
--- a/sw/source/core/access/accpara.hxx
+++ b/sw/source/core/access/accpara.hxx
@@ -234,7 +234,7 @@ protected:
 
 public:
 
-    SwAccessibleParagraph( SwAccessibleMap* pInitMap,
+    SwAccessibleParagraph( std::shared_ptr<SwAccessibleMap> const& pInitMap,
                            const SwTextFrame& rTextFrame );
 
     inline operator css::accessibility::XAccessibleText *();
diff --git a/sw/source/core/access/accpreview.cxx b/sw/source/core/access/accpreview.cxx
index 2489dc307a06..3b449d18d9b4 100644
--- a/sw/source/core/access/accpreview.cxx
+++ b/sw/source/core/access/accpreview.cxx
@@ -29,8 +29,8 @@ const sal_Char sImplementationName[] = "com.sun.star.comp.Writer.SwAccessibleDoc
 using ::com::sun::star::uno::RuntimeException;
 using ::com::sun::star::uno::Sequence;
 
-SwAccessiblePreview::SwAccessiblePreview( SwAccessibleMap *pMp ) :
-    SwAccessibleDocumentBase( pMp )
+SwAccessiblePreview::SwAccessiblePreview(std::shared_ptr<SwAccessibleMap> const& pMap)
+    : SwAccessibleDocumentBase(pMap)
 {
     SetName( GetResource( STR_ACCESS_PREVIEW_DOC_NAME ) );
 }
diff --git a/sw/source/core/access/accpreview.hxx b/sw/source/core/access/accpreview.hxx
index cc9cab268795..a60254f6a1f1 100644
--- a/sw/source/core/access/accpreview.hxx
+++ b/sw/source/core/access/accpreview.hxx
@@ -35,7 +35,7 @@ class SwAccessiblePreview : public  SwAccessibleDocumentBase
     virtual ~SwAccessiblePreview() override;
 
 public:
-    SwAccessiblePreview( SwAccessibleMap *pMap );
+    SwAccessiblePreview(std::shared_ptr<SwAccessibleMap> const& pMap);
 
     // XServiceInfo
 
diff --git a/sw/source/core/access/acctable.cxx b/sw/source/core/access/acctable.cxx
index e4a68e420f08..b237242f3d62 100644
--- a/sw/source/core/access/acctable.cxx
+++ b/sw/source/core/access/acctable.cxx
@@ -762,7 +762,7 @@ void SwAccessibleTable::GetStates(
 }
 
 SwAccessibleTable::SwAccessibleTable(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         const SwTabFrame* pTabFrame  ) :
     SwAccessibleContext( pInitMap, AccessibleRole::TABLE, pTabFrame ),
     mpTableData( nullptr )
@@ -1073,7 +1073,8 @@ uno::Reference< XAccessibleTable > SAL_CALL
     // #i87532# - assure that return accessible object is empty,
     // if no column header exists.
     SwAccessibleTableColHeaders* pTableColHeaders =
-        new SwAccessibleTableColHeaders( GetMap(), static_cast< const SwTabFrame *>( GetFrame() ) );
+        new SwAccessibleTableColHeaders(GetMap()->shared_from_this(),
+                    static_cast<const SwTabFrame *>(GetFrame()));
     uno::Reference< XAccessibleTable > xTableColumnHeaders( pTableColHeaders );
     if ( pTableColHeaders->getAccessibleChildCount() <= 0 )
     {
@@ -1808,9 +1809,10 @@ sal_Bool SAL_CALL SwAccessibleTable::unselectColumn( sal_Int32 column )
 }
 
 // #i77106# - implementation of class <SwAccessibleTableColHeaders>
-SwAccessibleTableColHeaders::SwAccessibleTableColHeaders( SwAccessibleMap *pMap2,
-                                                          const SwTabFrame *pTabFrame )
-    : SwAccessibleTable( pMap2, pTabFrame )
+SwAccessibleTableColHeaders::SwAccessibleTableColHeaders(
+        std::shared_ptr<SwAccessibleMap> const& pMap,
+        const SwTabFrame *const pTabFrame)
+    : SwAccessibleTable(pMap, pTabFrame)
 {
     SolarMutexGuard aGuard;
 
diff --git a/sw/source/core/access/acctable.hxx b/sw/source/core/access/acctable.hxx
index f2fc518209ec..6f03d6a4bc38 100644
--- a/sw/source/core/access/acctable.hxx
+++ b/sw/source/core/access/acctable.hxx
@@ -86,7 +86,8 @@ protected:
     virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew) override;
 
 public:
-    SwAccessibleTable( SwAccessibleMap* pInitMap, const SwTabFrame* pTableFrame );
+    SwAccessibleTable(std::shared_ptr<SwAccessibleMap> const& pInitMap,
+                      const SwTabFrame* pTableFrame);
 
     // XInterface
 
@@ -237,7 +238,8 @@ protected:
     virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew) override;
 
 public:
-    SwAccessibleTableColHeaders( SwAccessibleMap *pMap, const SwTabFrame *pTabFrame );
+    SwAccessibleTableColHeaders(std::shared_ptr<SwAccessibleMap> const& pMap,
+                                const SwTabFrame *pTabFrame);
 
     // XInterface
 
diff --git a/sw/source/core/access/acctextframe.cxx b/sw/source/core/access/acctextframe.cxx
index 61ae50523920..dac7b2baeefc 100644
--- a/sw/source/core/access/acctextframe.cxx
+++ b/sw/source/core/access/acctextframe.cxx
@@ -43,7 +43,7 @@ using utl::AccessibleRelationSetHelper;
 using ::com::sun::star::accessibility::XAccessibleContext;
 
 SwAccessibleTextFrame::SwAccessibleTextFrame(
-        SwAccessibleMap* pInitMap,
+        std::shared_ptr<SwAccessibleMap> const& pInitMap,
         const SwFlyFrame& rFlyFrame  ) :
     SwAccessibleFrameBase( pInitMap, AccessibleRole::TEXT_FRAME, &rFlyFrame ),
     msTitle(),
diff --git a/sw/source/core/access/acctextframe.hxx b/sw/source/core/access/acctextframe.hxx
index 3b6d5cec590c..659e0ab3d499 100644
--- a/sw/source/core/access/acctextframe.hxx
+++ b/sw/source/core/access/acctextframe.hxx
@@ -43,7 +43,8 @@ protected:
     virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew) override;
 
 public:
-    SwAccessibleTextFrame( SwAccessibleMap* pInitMap, const SwFlyFrame& rFlyFrame );
+    SwAccessibleTextFrame(std::shared_ptr<SwAccessibleMap> const& pInitMap,
+                          const SwFlyFrame& rFlyFrame);
 
     virtual css::uno::Any SAL_CALL queryInterface(
         css::uno::Type const & rType ) override;
diff --git a/sw/source/core/inc/viewimp.hxx b/sw/source/core/inc/viewimp.hxx
index 011132457c5b..44cb77816fbe 100644
--- a/sw/source/core/inc/viewimp.hxx
+++ b/sw/source/core/inc/viewimp.hxx
@@ -74,7 +74,9 @@ class SwViewShellImp
                                  // Is registered by the SwLayAction ctor and deregistered by the dtor
     SwLayIdle     *m_pIdleAct;     // The same as SwLayAction for SwLayIdle
 
-    SwAccessibleMap *m_pAccessibleMap;    // Accessible wrappers
+    /// note: the map is *uniquely* owned here - the shared_ptr is only
+    /// used so that SwAccessibleContext can check via weak_ptr that it's alive
+    std::shared_ptr<SwAccessibleMap> m_pAccessibleMap;
 
     bool m_bFirstPageInvalid : 1; // Pointer to the first Page invalid?
     bool m_bResetHdlHiddenPaint : 1; // Ditto
diff --git a/sw/source/core/view/viewimp.cxx b/sw/source/core/view/viewimp.cxx
index 335f45a38d1a..6baeba639baa 100644
--- a/sw/source/core/view/viewimp.cxx
+++ b/sw/source/core/view/viewimp.cxx
@@ -106,7 +106,7 @@ SwViewShellImp::SwViewShellImp( SwViewShell *pParent ) :
 
 SwViewShellImp::~SwViewShellImp()
 {
-    delete m_pAccessibleMap;
+    m_pAccessibleMap.reset();
 
     delete m_pPagePreviewLayout;
 
@@ -449,9 +449,9 @@ void SwViewShellImp::InvalidateAccessiblePreviewSelection( sal_uInt16 nSelPage )
 
 SwAccessibleMap *SwViewShellImp::CreateAccessibleMap()
 {
-    OSL_ENSURE( !m_pAccessibleMap, "accessible map exists" );
-    m_pAccessibleMap = new SwAccessibleMap( GetShell() );
-    return m_pAccessibleMap;
+    assert(!m_pAccessibleMap);
+    m_pAccessibleMap.reset(new SwAccessibleMap(GetShell()));
+    return m_pAccessibleMap.get();
 }
 
 void SwViewShellImp::FireAccessibleEvents()


More information about the Libreoffice-commits mailing list