i#105557: fix locking for SwXTextMarkup

... by adding the usual sw::UnoImplPtr pImpl.  This was crashing in
sw_unoapi under ASAN in the dtor's SwClient deregistering.

Change-Id: I038ca21e04fae7599113b949846a23e1ca73181e

sw/source/core/inc/unoflatpara.hxx

@@ -125,7 +125,7 @@ public:
     virtual void SAL_CALL changeAttributes(::sal_Int32 nPos, ::sal_Int32 nLen, const css::uno::Sequence< css::beans::PropertyValue > & aAttributes) throw (css::uno::RuntimeException, css::lang::IllegalArgumentException, std::exception) SAL_OVERRIDE;
     virtual css::uno::Sequence< ::sal_Int32 > SAL_CALL getLanguagePortions() throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE;
 
-    const SwTxtNode* getTxtNode() const { return mpTxtNode;}
+    using SwXTextMarkup::GetTxtNode;
 
     static const css::uno::Sequence< sal_Int8 >& getUnoTunnelId();
 

sw/source/core/inc/unotextmarkup.hxx

@@ -22,10 +22,11 @@
 
 #include <cppuhelper/implbase1.hxx>
 #include <cppuhelper/implbase2.hxx>
+
 #include <com/sun/star/text/XTextMarkup.hpp>
 #include <com/sun/star/text/XMultiTextMarkup.hpp>
-#include <calbck.hxx>
-#include <modeltoviewhelper.hxx>
+
+#include <unobaseclass.hxx>
 
 #include <map>
 
@@ -38,17 +39,16 @@ namespace com { namespace sun { namespace star { namespace text {
 } } } }
 
 class SwTxtNode;
+class ModelToViewHelper;
 class SfxPoolItem;
 
 /** Implementation of the css::text::XTextMarkup interface
  */
-class SwXTextMarkup:
-    public ::cppu::WeakImplHelper2
-    <
-        ::com::sun::star::text::XTextMarkup,
-        ::com::sun::star::text::XMultiTextMarkup
-    >,
-    public SwClient
+class SwXTextMarkup
+    : public ::cppu::WeakImplHelper2
+        <   ::com::sun::star::text::XTextMarkup
+        ,   ::com::sun::star::text::XMultiTextMarkup
+        >
 {
 public:
     SwXTextMarkup(SwTxtNode *const rTxtNode,
@@ -71,12 +71,13 @@ private:
     SwXTextMarkup( const SwXTextMarkup & ) SAL_DELETED_FUNCTION;
     SwXTextMarkup & operator =( const SwXTextMarkup & ) SAL_DELETED_FUNCTION;
 
-protected:
-    //SwClient
-    virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew) SAL_OVERRIDE;
+    struct Impl;
+    ::sw::UnoImplPtr<Impl> m_pImpl;
 
-    SwTxtNode* mpTxtNode;
-    const ModelToViewHelper maConversionMap;
+protected:
+    SwTxtNode* GetTxtNode();
+    void ClearTxtNode();
+    const ModelToViewHelper& GetConversionMap();
 };
 
 /** Implementation of the ::com::sun::star::container::XStringKeyMap interface

sw/source/core/unocore/unoflatpara.cxx

@@ -101,16 +101,16 @@ throw (beans::UnknownPropertyException, lang::WrappedTargetException,
 
     if (rPropertyName == "FieldPositions")
     {
-        uno::Sequence<sal_Int32> ret(maConversionMap.getFieldPositions().size());
-        std::copy(maConversionMap.getFieldPositions().begin(),
-                maConversionMap.getFieldPositions().end(), ret.begin());
+        uno::Sequence<sal_Int32> ret(GetConversionMap().getFieldPositions().size());
+        std::copy(GetConversionMap().getFieldPositions().begin(),
+                GetConversionMap().getFieldPositions().end(), ret.begin());
         return uno::makeAny(ret);
     }
     else if (rPropertyName == "FootnotePositions")
     {
-        uno::Sequence<sal_Int32> ret(maConversionMap.getFootnotePositions().size());
-        std::copy(maConversionMap.getFootnotePositions().begin(),
-                maConversionMap.getFootnotePositions().end(), ret.begin());
+        uno::Sequence<sal_Int32> ret(GetConversionMap().getFootnotePositions().size());
+        std::copy(GetConversionMap().getFootnotePositions().begin(),
+                GetConversionMap().getFootnotePositions().end(), ret.begin());
         return uno::makeAny(ret);
     }
     return uno::Any();
@@ -190,17 +190,17 @@ void SAL_CALL SwXFlatParagraph::setChecked( ::sal_Int32 nType, sal_Bool bVal ) t
 {
     SolarMutexGuard aGuard;
 
-    if ( mpTxtNode )
+    if (GetTxtNode())
     {
         if ( text::TextMarkupType::SPELLCHECK == nType )
-            mpTxtNode->SetWrongDirty( !bVal );
+            GetTxtNode()->SetWrongDirty( !bVal );
         else if ( text::TextMarkupType::SMARTTAG == nType )
-            mpTxtNode->SetSmartTagDirty( !bVal );
+            GetTxtNode()->SetSmartTagDirty( !bVal );
         else if( text::TextMarkupType::PROOFREADING == nType )
         {
-            mpTxtNode->SetGrammarCheckDirty( !bVal );
+            GetTxtNode()->SetGrammarCheckDirty( !bVal );
             if( bVal )
-                ::finishGrammarCheck( *mpTxtNode );
+                ::finishGrammarCheck( *GetTxtNode() );
         }
     }
 }
@@ -209,14 +209,14 @@ void SAL_CALL SwXFlatParagraph::setChecked( ::sal_Int32 nType, sal_Bool bVal ) t
 sal_Bool SAL_CALL SwXFlatParagraph::isChecked( ::sal_Int32 nType ) throw (uno::RuntimeException, std::exception)
 {
     SolarMutexGuard aGuard;
-    if ( mpTxtNode )
+    if (GetTxtNode())
     {
         if ( text::TextMarkupType::SPELLCHECK == nType )
-            return mpTxtNode->IsWrongDirty();
+            return GetTxtNode()->IsWrongDirty();
         else if ( text::TextMarkupType::PROOFREADING == nType )
-            return mpTxtNode->IsGrammarCheckDirty();
+            return GetTxtNode()->IsGrammarCheckDirty();
         else if ( text::TextMarkupType::SMARTTAG == nType )
-            return mpTxtNode->IsSmartTagDirty();
+            return GetTxtNode()->IsSmartTagDirty();
     }
 
     return sal_False;
@@ -226,7 +226,7 @@ sal_Bool SAL_CALL SwXFlatParagraph::isChecked( ::sal_Int32 nType ) throw (uno::R
 sal_Bool SAL_CALL SwXFlatParagraph::isModified() throw (uno::RuntimeException, std::exception)
 {
     SolarMutexGuard aGuard;
-    return 0 == mpTxtNode;
+    return 0 == GetTxtNode();
 }
 
 // text::XFlatParagraph:
@@ -234,10 +234,10 @@ lang::Locale SAL_CALL SwXFlatParagraph::getLanguageOfText(::sal_Int32 nPos, ::sa
     throw (uno::RuntimeException, lang::IllegalArgumentException, std::exception)
 {
     SolarMutexGuard aGuard;
-    if (!mpTxtNode)
+    if (!GetTxtNode())
         return LanguageTag::convertToLocale( LANGUAGE_NONE );
 
-    const lang::Locale aLocale( SW_BREAKITER()->GetLocale( mpTxtNode->GetLang(nPos, nLen) ) );
+    const lang::Locale aLocale( SW_BREAKITER()->GetLocale( GetTxtNode()->GetLang(nPos, nLen) ) );
     return aLocale;
 }
 
@@ -247,10 +247,10 @@ lang::Locale SAL_CALL SwXFlatParagraph::getPrimaryLanguageOfText(::sal_Int32 nPo
 {
     SolarMutexGuard aGuard;
 
-    if (!mpTxtNode)
+    if (!GetTxtNode())
         return LanguageTag::convertToLocale( LANGUAGE_NONE );
 
-    const lang::Locale aLocale( SW_BREAKITER()->GetLocale( mpTxtNode->GetLang(nPos, nLen) ) );
+    const lang::Locale aLocale( SW_BREAKITER()->GetLocale( GetTxtNode()->GetLang(nPos, nLen) ) );
     return aLocale;
 }
 
@@ -259,18 +259,18 @@ void SAL_CALL SwXFlatParagraph::changeText(::sal_Int32 nPos, ::sal_Int32 nLen, c
 {
     SolarMutexGuard aGuard;
 
-    if ( !mpTxtNode )
+    if (!GetTxtNode())
         return;
 
-    SwTxtNode* pOldTxtNode = mpTxtNode;
+    SwTxtNode *const pOldTxtNode = GetTxtNode();
 
-    SwPaM aPaM( *mpTxtNode, nPos, *mpTxtNode, nPos+nLen );
+    SwPaM aPaM( *GetTxtNode(), nPos, *GetTxtNode(), nPos+nLen );
 
-    UnoActionContext aAction( mpTxtNode->GetDoc() );
+    UnoActionContext aAction( GetTxtNode()->GetDoc() );
 
     const uno::Reference< text::XTextRange > xRange =
         SwXTextRange::CreateXTextRange(
-            *mpTxtNode->GetDoc(), *aPaM.GetPoint(), aPaM.GetMark() );
+            *GetTxtNode()->GetDoc(), *aPaM.GetPoint(), aPaM.GetMark() );
     uno::Reference< beans::XPropertySet > xPropSet( xRange, uno::UNO_QUERY );
     if ( xPropSet.is() )
     {
@@ -281,7 +281,7 @@ void SAL_CALL SwXFlatParagraph::changeText(::sal_Int32 nPos, ::sal_Int32 nLen, c
     IDocumentContentOperations* pIDCO = pOldTxtNode->getIDocumentContentOperations();
     pIDCO->ReplaceRange( aPaM, aNewText, false );
 
-    mpTxtNode = 0;
+    ClearTxtNode(); // TODO: is this really needed?
 }
 
 // text::XFlatParagraph:
@@ -289,16 +289,16 @@ void SAL_CALL SwXFlatParagraph::changeAttributes(::sal_Int32 nPos, ::sal_Int32 n
 {
     SolarMutexGuard aGuard;
 
-    if ( !mpTxtNode )
+    if (!GetTxtNode())
         return;
 
-    SwPaM aPaM( *mpTxtNode, nPos, *mpTxtNode, nPos+nLen );
+    SwPaM aPaM( *GetTxtNode(), nPos, *GetTxtNode(), nPos+nLen );
 
-    UnoActionContext aAction( mpTxtNode->GetDoc() );
+    UnoActionContext aAction( GetTxtNode()->GetDoc() );
 
     const uno::Reference< text::XTextRange > xRange =
         SwXTextRange::CreateXTextRange(
-            *mpTxtNode->GetDoc(), *aPaM.GetPoint(), aPaM.GetMark() );
+            *GetTxtNode()->GetDoc(), *aPaM.GetPoint(), aPaM.GetMark() );
     uno::Reference< beans::XPropertySet > xPropSet( xRange, uno::UNO_QUERY );
     if ( xPropSet.is() )
     {
@@ -306,7 +306,7 @@ void SAL_CALL SwXFlatParagraph::changeAttributes(::sal_Int32 nPos, ::sal_Int32 n
             xPropSet->setPropertyValue( aAttributes[i].Name, aAttributes[i].Value );
     }
 
-    mpTxtNode = 0;
+    ClearTxtNode(); // TODO: is this really needed?
 }
 
 // text::XFlatParagraph:
@@ -495,7 +495,7 @@ uno::Reference< text::XFlatParagraph > SwXFlatParagraphIterator::getParaAfter(co
     if ( !pFlatParagraph )
         return xRet;
 
-    const SwTxtNode* pCurrentNode = pFlatParagraph->getTxtNode();
+    SwTxtNode const*const pCurrentNode = pFlatParagraph->GetTxtNode();
 
     if ( !pCurrentNode )
         return xRet;
@@ -541,7 +541,7 @@ uno::Reference< text::XFlatParagraph > SwXFlatParagraphIterator::getParaBefore(c
     if ( !pFlatParagraph )
         return xRet;
 
-    const SwTxtNode* pCurrentNode = pFlatParagraph->getTxtNode();
+    SwTxtNode const*const pCurrentNode = pFlatParagraph->GetTxtNode();
 
     if ( !pCurrentNode )
         return xRet;

sw/source/core/unocore/unotextmarkup.cxx

@@ -40,18 +40,48 @@
 
 using namespace ::com::sun::star;
 
+struct SwXTextMarkup::Impl
+    : public SwClient
+{
+    SwTxtNode* m_pTxtNode;
+    ModelToViewHelper const m_ConversionMap;
+
+    Impl(SwTxtNode *const pTxtNode, const ModelToViewHelper& rMap)
+        : SwClient(pTxtNode)
+        , m_pTxtNode(pTxtNode)
+        , m_ConversionMap(rMap)
+    {
+    }
+
+    // SwClient
+    virtual void Modify(const SfxPoolItem *pOld, const SfxPoolItem *pNew) SAL_OVERRIDE;
+};
+
 SwXTextMarkup::SwXTextMarkup(
         SwTxtNode *const pTxtNode, const ModelToViewHelper& rMap)
-    : mpTxtNode(pTxtNode)
-    , maConversionMap(rMap)
+    : m_pImpl(new Impl(pTxtNode, rMap))
 {
-    mpTxtNode->Add(this);
 }
 
 SwXTextMarkup::~SwXTextMarkup()
 {
 }
 
+SwTxtNode* SwXTextMarkup::GetTxtNode()
+{
+    return m_pImpl->m_pTxtNode;
+}
+
+void SwXTextMarkup::ClearTxtNode()
+{
+    m_pImpl->m_pTxtNode = nullptr;
+}
+
+const ModelToViewHelper& SwXTextMarkup::GetConversionMap()
+{
+    return m_pImpl->m_ConversionMap;
+}
+
 uno::Reference< container::XStringKeyMap > SAL_CALL SwXTextMarkup::getMarkupInfoContainer() throw (uno::RuntimeException, std::exception)
 {
     SolarMutexGuard aGuard;
@@ -115,7 +145,7 @@ void SAL_CALL SwXTextMarkup::commitStringMarkup(
     SolarMutexGuard aGuard;
 
     // paragraph already dead or modified?
-    if ( !mpTxtNode || nLength <= 0 )
+    if (!m_pImpl->m_pTxtNode || nLength <= 0)
         return;
 
     if ( nType == text::TextMarkupType::SMARTTAG &&
@@ -127,41 +157,41 @@ void SAL_CALL SwXTextMarkup::commitStringMarkup(
     bool bRepaint = false;
     if ( nType == text::TextMarkupType::SPELLCHECK )
     {
-        pWList = mpTxtNode->GetWrong();
+        pWList = m_pImpl->m_pTxtNode->GetWrong();
         if ( !pWList )
         {
             pWList = new SwWrongList( WRONGLIST_SPELL );
-            mpTxtNode->SetWrong( pWList );
+            m_pImpl->m_pTxtNode->SetWrong( pWList );
         }
     }
     else if ( nType == text::TextMarkupType::PROOFREADING || nType == text::TextMarkupType::SENTENCE )
     {
-        IGrammarContact *pGrammarContact = getGrammarContact( *mpTxtNode );
+        IGrammarContact *pGrammarContact = getGrammarContact(*m_pImpl->m_pTxtNode);
         if( pGrammarContact )
         {
-            pWList = pGrammarContact->getGrammarCheck( *mpTxtNode, true );
+            pWList = pGrammarContact->getGrammarCheck(*m_pImpl->m_pTxtNode, true);
             OSL_ENSURE( pWList, "GrammarContact _has_ to deliver a wrong list" );
         }
         else
         {
-            pWList = mpTxtNode->GetGrammarCheck();
+            pWList = m_pImpl->m_pTxtNode->GetGrammarCheck();
             if ( !pWList )
             {
-                mpTxtNode->SetGrammarCheck( new SwGrammarMarkUp() );
-                pWList = mpTxtNode->GetGrammarCheck();
+                m_pImpl->m_pTxtNode->SetGrammarCheck( new SwGrammarMarkUp() );
+                pWList = m_pImpl->m_pTxtNode->GetGrammarCheck();
             }
         }
-        bRepaint = pWList == mpTxtNode->GetGrammarCheck();
+        bRepaint = pWList == m_pImpl->m_pTxtNode->GetGrammarCheck();
         if( pWList->GetBeginInv() < COMPLETE_STRING )
             static_cast<SwGrammarMarkUp*>(pWList)->ClearGrammarList();
     }
     else if ( nType == text::TextMarkupType::SMARTTAG )
     {
-        pWList = mpTxtNode->GetSmartTags();
+        pWList = m_pImpl->m_pTxtNode->GetSmartTags();
         if ( !pWList )
         {
             pWList = new SwWrongList( WRONGLIST_SMARTTAG );
-            mpTxtNode->SetSmartTags( pWList );
+            m_pImpl->m_pTxtNode->SetSmartTags( pWList );
         }
     }
     else
@@ -171,9 +201,9 @@ void SAL_CALL SwXTextMarkup::commitStringMarkup(
     }
 
     const ModelToViewHelper::ModelPosition aStartPos =
-            maConversionMap.ConvertToModelPosition( nStart );
+        m_pImpl->m_ConversionMap.ConvertToModelPosition( nStart );
     const ModelToViewHelper::ModelPosition aEndPos   =
-            maConversionMap.ConvertToModelPosition( nStart + nLength - 1);
+        m_pImpl->m_ConversionMap.ConvertToModelPosition( nStart + nLength - 1);
 
     const bool bStartInField = aStartPos.mbIsField;
     const bool bEndInField   = aEndPos.mbIsField;
@@ -219,8 +249,10 @@ void SAL_CALL SwXTextMarkup::commitStringMarkup(
                 pSubList = new SwGrammarMarkUp();
                 pWList->InsertSubList( nFieldPosModel, 1, nInsertPos, pSubList );
             }
-            const sal_Int32 nTmpStart = maConversionMap.ConvertToViewPosition( aStartPos.mnPos );
-            const sal_Int32 nTmpLen = maConversionMap.ConvertToViewPosition( aStartPos.mnPos + 1 )
+            const sal_Int32 nTmpStart =
+                m_pImpl->m_ConversionMap.ConvertToViewPosition(aStartPos.mnPos);
+            const sal_Int32 nTmpLen =
+                m_pImpl->m_ConversionMap.ConvertToViewPosition(aStartPos.mnPos + 1)
                                        - nTmpStart - aStartPos.mnSubPos;
             if( nTmpLen > 0 )
             {
@@ -258,7 +290,7 @@ void SAL_CALL SwXTextMarkup::commitStringMarkup(
     }
 
     if( bRepaint )
-        finishGrammarCheck( *mpTxtNode );
+        finishGrammarCheck(*m_pImpl->m_pTxtNode);
 }
 
 static void lcl_commitGrammarMarkUp(
@@ -361,7 +393,7 @@ throw (lang::IllegalArgumentException, uno::RuntimeException, std::exception)
     SolarMutexGuard aGuard;
 
     // paragraph already dead or modified?
-    if ( !mpTxtNode )
+    if (!m_pImpl->m_pTxtNode)
         return;
 
     // check for equal length of all sequnces
@@ -395,29 +427,29 @@ throw (lang::IllegalArgumentException, uno::RuntimeException, std::exception)
     // get appropriate list to use...
     SwGrammarMarkUp* pWList = 0;
     bool bRepaint = false;
-    IGrammarContact *pGrammarContact = getGrammarContact( *mpTxtNode );
+    IGrammarContact *pGrammarContact = getGrammarContact(*m_pImpl->m_pTxtNode);
     if( pGrammarContact )
     {
-        pWList = pGrammarContact->getGrammarCheck( *mpTxtNode, true );
+        pWList = pGrammarContact->getGrammarCheck(*m_pImpl->m_pTxtNode, true);
         OSL_ENSURE( pWList, "GrammarContact _has_ to deliver a wrong list" );
     }
     else
     {
-        pWList = mpTxtNode->GetGrammarCheck();
+        pWList = m_pImpl->m_pTxtNode->GetGrammarCheck();
         if ( !pWList )
         {
-            mpTxtNode->SetGrammarCheck( new SwGrammarMarkUp() );
-            pWList = mpTxtNode->GetGrammarCheck();
+            m_pImpl->m_pTxtNode->SetGrammarCheck( new SwGrammarMarkUp() );
+            pWList = m_pImpl->m_pTxtNode->GetGrammarCheck();
             pWList->SetInvalid( 0, COMPLETE_STRING );
         }
     }
-    bRepaint = pWList == mpTxtNode->GetGrammarCheck();
+    bRepaint = pWList == m_pImpl->m_pTxtNode->GetGrammarCheck();
 
     bool bAcceptGrammarError = false;
     if( pWList->GetBeginInv() < COMPLETE_STRING )
     {
         const ModelToViewHelper::ModelPosition aSentenceEnd =
-            maConversionMap.ConvertToModelPosition(
+            m_pImpl->m_ConversionMap.ConvertToModelPosition(
                 pMarkups[nSentenceMarkUpIndex].nOffset + pMarkups[nSentenceMarkUpIndex].nLength );
         bAcceptGrammarError = aSentenceEnd.mnPos > pWList->GetBeginInv();
         pWList->ClearGrammarList( aSentenceEnd.mnPos );
@@ -428,7 +460,7 @@ throw (lang::IllegalArgumentException, uno::RuntimeException, std::exception)
         for( i = 0;  i < nLen;  ++i )
         {
             const text::TextMarkupDescriptor &rDesc = pMarkups[i];
-            lcl_commitGrammarMarkUp( maConversionMap, pWList, rDesc.nType,
+            lcl_commitGrammarMarkUp(m_pImpl->m_ConversionMap, pWList, rDesc.nType,
                 rDesc.aIdentifier, rDesc.nOffset, rDesc.nLength, rDesc.xMarkupInfoContainer );
         }
     }
@@ -437,23 +469,23 @@ throw (lang::IllegalArgumentException, uno::RuntimeException, std::exception)
         bRepaint = false;
         i = nSentenceMarkUpIndex;
         const text::TextMarkupDescriptor &rDesc = pMarkups[i];
-        lcl_commitGrammarMarkUp( maConversionMap, pWList, rDesc.nType,
+        lcl_commitGrammarMarkUp(m_pImpl->m_ConversionMap, pWList, rDesc.nType,
             rDesc.aIdentifier, rDesc.nOffset, rDesc.nLength, rDesc.xMarkupInfoContainer );
     }
 
     if( bRepaint )
-        finishGrammarCheck( *mpTxtNode );
+        finishGrammarCheck(*m_pImpl->m_pTxtNode);
 
     return;
 }
 
-void SwXTextMarkup::Modify( const SfxPoolItem* /*pOld*/, const SfxPoolItem* /*pNew*/ )
+void SwXTextMarkup::Impl::Modify( const SfxPoolItem* /*pOld*/, const SfxPoolItem* /*pNew*/ )
 {
     if ( GetRegisteredIn() )
         GetRegisteredInNonConst()->Remove( this );
 
     SolarMutexGuard aGuard;
-    mpTxtNode = 0;
+    m_pTxtNode = 0;
 }
 
 SwXStringKeyMap::SwXStringKeyMap()