[Libreoffice-commits] core.git: Branch 'libreoffice-5-3' - sw/inc sw/source
Michael Stahl
mstahl at redhat.com
Mon Mar 27 22:20:57 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 | 39 +++++++++++++++------------
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, 129 insertions(+), 74 deletions(-)
New commits:
commit aba392d1a07b8b0d40583d7e893a409cf96f1725
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
(cherry picked from commit 0c2229dcab143925c6ad390e0735e2d98c3eecca)
Reviewed-on: https://gerrit.libreoffice.org/35654
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
Tested-by: Caolán McNamara <caolanm at redhat.com>
diff --git a/sw/inc/accmap.hxx b/sw/inc/accmap.hxx
index aaef5a76b08f..c1ff5256e38c 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 694a56069486..b153bfe73215 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 )
, aSelectionHelper( *this )
diff --git a/sw/source/core/access/acccell.hxx b/sw/source/core/access/acccell.hxx
index ab95afdeb1c6..54554f9f0ac0 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 e0069991f214..dce1c0e03f10 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
@@ -1075,6 +1097,7 @@ void SwAccessibleContext::Dispose(bool bRecursive, bool bCanSkipInvisible)
RemoveFrameFromAccessibleMap();
ClearFrame();
m_pMap = nullptr;
+ m_wMap.reset();
m_isDisposing = false;
}
@@ -1432,8 +1455,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 31ed088111e4..4dc504f9a057 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 06b9c5b7f0e2..0df510cbd79c 100644
--- a/sw/source/core/access/accdoc.cxx
+++ b/sw/source/core/access/accdoc.cxx
@@ -70,11 +70,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)
{
}
@@ -348,9 +349,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( GetResource( STR_ACCESS_DOC_NAME ) );
vcl::Window *pWin = pInitMap->GetShell()->GetWin();
diff --git a/sw/source/core/access/accdoc.hxx b/sw/source/core/access/accdoc.hxx
index 47caf4cac4b0..6e927bd3740e 100644
--- a/sw/source/core/access/accdoc.hxx
+++ b/sw/source/core/access/accdoc.hxx
@@ -43,7 +43,7 @@ protected:
virtual ~SwAccessibleDocumentBase() override;
public:
- SwAccessibleDocumentBase( SwAccessibleMap* pInitMap );
+ SwAccessibleDocumentBase(std::shared_ptr<SwAccessibleMap> const& pInitMap);
void SetVisArea();
@@ -120,7 +120,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 ef3b70b992d2..ebff72a5bae9 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 76792b649c5c..5b27308bc69b 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 7c16caba2316..cfcf958e0f0a 100644
--- a/sw/source/core/access/accfootnote.cxx
+++ b/sw/source/core/access/accfootnote.cxx
@@ -41,7 +41,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 12ff3e70e8de..a326dd2c0659 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 13823155ca18..f641ff8f2506 100644
--- a/sw/source/core/access/accframebase.cxx
+++ b/sw/source/core/access/accframebase.cxx
@@ -124,7 +124,7 @@ sal_uInt8 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 6809fb0d4e7a..96a3a4bcd95b 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 6c6a697d9adc..70ae68030f6b 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 754604d25511..da2b59da4659 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 36327d58f70f..1fb6783fc59b 100644
--- a/sw/source/core/access/accheaderfooter.cxx
+++ b/sw/source/core/access/accheaderfooter.cxx
@@ -37,7 +37,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 )
{
@@ -48,7 +48,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 fded39e7d55a..f42aa321c3eb 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 2f23331648e0..114e7f17b4a8 100644
--- a/sw/source/core/access/accmap.cxx
+++ b/sw/source/core/access/accmap.cxx
@@ -1655,6 +1655,7 @@ SwAccessibleMap::SwAccessibleMap( SwViewShell *pSh ) :
SwAccessibleMap::~SwAccessibleMap()
{
+ DBG_TESTSOLARMUTEX();
uno::Reference < XAccessible > xAcc;
{
osl::MutexGuard aGuard( maMutex );
@@ -1665,7 +1666,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());
}
}
@@ -1683,7 +1685,8 @@ SwAccessibleMap::~SwAccessibleMap()
if( xTmp.is() )
{
SwAccessibleContext *pTmp = static_cast< SwAccessibleContext * >( xTmp.get() );
- pTmp->SetMap(nullptr);
+ // TODO is this still needed
+ pTmp->ClearMapPointer();
}
++aIter;
}
@@ -1691,8 +1694,6 @@ SwAccessibleMap::~SwAccessibleMap()
{
osl::MutexGuard aGuard( maMutex );
#if OSL_DEBUG_LEVEL > 0
- SAL_WARN_IF(!(!mpFrameMap || mpFrameMap->empty()), "sw.a11y",
- "Frame map should be empty after disposing the root frame");
if( mpFrameMap )
{
SwAccessibleContextMap_Impl::iterator aIter = mpFrameMap->begin();
@@ -1708,8 +1709,6 @@ SwAccessibleMap::~SwAccessibleMap()
++aIter;
}
}
- SAL_WARN_IF(!(!mpShapeMap || mpShapeMap->empty()), "sw.a11y",
- "Object map should be empty after disposing the root frame");
if( mpShapeMap )
{
SwAccessibleShapeMap_Impl::iterator aIter = mpShapeMap->begin();
@@ -1725,6 +1724,10 @@ 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
delete mpFrameMap;
mpFrameMap = nullptr;
@@ -1786,9 +1789,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() )
{
@@ -1860,15 +1863,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:
@@ -1877,7 +1880,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 );
}
@@ -1889,29 +1892,29 @@ uno::Reference< XAccessible> SwAccessibleMap::GetContext( const SwFrame *pFrame,
switch( SwAccessibleFrameBase::GetNodeType( pFlyFrame ) )
{
case ND_GRFNODE:
- pAcc = new SwAccessibleGraphic( this, pFlyFrame );
+ pAcc = new SwAccessibleGraphic(shared_from_this(), pFlyFrame );
break;
case ND_OLENODE:
- 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 f07962c101b0..aaa7b103f1e9 100644
--- a/sw/source/core/access/accnotextframe.cxx
+++ b/sw/source/core/access/accnotextframe.cxx
@@ -60,7 +60,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 d4b35287c9cd..cb5aee70d454 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 0bf30f7be3ec..7bc46531c3d1 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 ed6e25cbd3f4..1b1522700c4e 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 935495573295..8148dcac195a 100644
--- a/sw/source/core/access/accpara.cxx
+++ b/sw/source/core/access/accpara.cxx
@@ -531,7 +531,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 4843175108a6..8062dbaf80eb 100644
--- a/sw/source/core/access/accpara.hxx
+++ b/sw/source/core/access/accpara.hxx
@@ -236,7 +236,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 269e1ed82c10..38169784c5d0 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 ad807fbc7ac7..5be8b5a9af6e 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 0402dd153d4e..6d8bf2ecede5 100644
--- a/sw/source/core/access/acctable.cxx
+++ b/sw/source/core/access/acctable.cxx
@@ -763,7 +763,7 @@ void SwAccessibleTable::GetStates(
}
SwAccessibleTable::SwAccessibleTable(
- SwAccessibleMap* pInitMap,
+ std::shared_ptr<SwAccessibleMap> const& pInitMap,
const SwTabFrame* pTabFrame ) :
SwAccessibleContext( pInitMap, AccessibleRole::TABLE, pTabFrame ),
mpTableData( nullptr )
@@ -1086,7 +1086,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 )
{
@@ -1854,9 +1855,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 73c3b2a121a7..21165deeea9f 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
@@ -292,7 +293,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 b8e962e1dd68..8a5418043699 100644
--- a/sw/source/core/access/acctextframe.cxx
+++ b/sw/source/core/access/acctextframe.cxx
@@ -44,7 +44,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 bfe74eeb0038..eba7b36a7dcc 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 )
diff --git a/sw/source/core/inc/viewimp.hxx b/sw/source/core/inc/viewimp.hxx
index 465a68fada6b..4a2ab4cb887a 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