Revert "NotifyClients no more"

This reverts commit 96bafa464ebdbce3ef04bec9beae5e745bb37794.  It started to
cause an ASan heap-use-after-free during CppunitTest_sw_uiwriter, see the
comments starting at <https://gerrit.libreoffice.org/c/core/+/108604/
5#message-681a6100cff11cb51d20e5a2b2fb38da1f92ab32> "NotifyClients no more".

Change-Id: Id0eef2e75ea047d4b337c51df929eea81fb69f5a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/108717
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

17 files changed, 59 insertions, 53 deletions

diff --git a/sw/source/core/attr/calbck.cxx b/sw/source/core/attr/calbck.cxx
index 11235aeac02a..6acee52c9231 100644
--- a/sw/source/core/attr/calbck.cxx
+++ b/sw/source/core/attr/calbck.cxx
@@ -159,8 +159,9 @@ SwModify::~SwModify()
     if ( IsInSwFntCache() )
         pSwFontCache->Delete( this );
 
+    // notify all clients that they shall remove themselves
     SwPtrMsgPoolItem aDyObject( RES_OBJECTDYING, this );
-    SwModify::SwClientNotify(*this, sw::LegacyModifyHint(&aDyObject, &aDyObject));
+    NotifyClients( &aDyObject, &aDyObject );
 
     // remove all clients that have not done themselves
     // mba: possibly a hotfix for forgotten base class calls?!
@@ -168,6 +169,24 @@ SwModify::~SwModify()
         static_cast<SwClient*>(m_pWriterListeners)->CheckRegistration( &aDyObject );
 }
 
+void SwModify::NotifyClients( const SfxPoolItem* pOldValue, const SfxPoolItem* pNewValue )
+{
+    DBG_TESTSOLARMUTEX();
+    if ( IsInCache() || IsInSwFntCache() )
+    {
+        const sal_uInt16 nWhich = pOldValue ? pOldValue->Which() :
+                                        pNewValue ? pNewValue->Which() : 0;
+        CheckCaching( nWhich );
+    }
+
+    if ( !m_pWriterListeners || IsModifyLocked() )
+        return;
+
+    LockModify();
+    CallSwClientNotify( sw::LegacyModifyHint{ pOldValue, pNewValue } );
+    UnlockModify();
+}
+
 bool SwModify::GetInfo( SfxPoolItem& rInfo ) const
 {
     if(!m_pWriterListeners)
@@ -345,17 +364,7 @@ sw::ClientIteratorBase* sw::ClientIteratorBase::s_pClientIters = nullptr;
 void SwModify::SwClientNotify(const SwModify&, const SfxHint& rHint)
 {
     if(auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
-    {
-        if(IsInCache() || IsInSwFntCache())
-            CheckCaching(pLegacyHint->GetWhich());
-
-        if(IsModifyLocked())
-            return;
-
-        LockModify();
-        CallSwClientNotify(rHint);
-        UnlockModify();
-    }
+        NotifyClients(pLegacyHint->m_pOld, pLegacyHint->m_pNew);
 }
 
 void SwModify::CallSwClientNotify( const SfxHint& rHint ) const
diff --git a/sw/source/core/attr/format.cxx b/sw/source/core/attr/format.cxx
index be7e24e120d7..4b0529d2f253 100644
--- a/sw/source/core/attr/format.cxx
+++ b/sw/source/core/attr/format.cxx
@@ -319,7 +319,7 @@ void SwFormat::SwClientNotify(const SwModify&, const SfxHint& rHint)
             }
     }
     if(bPassToDepends)
-        CallSwClientNotify(sw::LegacyModifyHint(aDependArgs.first, aDependArgs.second));
+        NotifyClients(aDependArgs.first, aDependArgs.second);
 }
 
 bool SwFormat::SetDerivedFrom(SwFormat *pDerFrom)
diff --git a/sw/source/core/doc/CntntIdxStore.cxx b/sw/source/core/doc/CntntIdxStore.cxx
index 989fae5b4abe..c37221e189e8 100644
--- a/sw/source/core/doc/CntntIdxStore.cxx
+++ b/sw/source/core/doc/CntntIdxStore.cxx
@@ -393,9 +393,9 @@ void ContentIdxStoreImpl::RestoreFlys(SwDoc& rDoc, updater_t const & rUpdater, b
         }
         else if( bAuto )
         {
-            SwFrameFormat* pFrameFormat = (*pSpz)[ aEntry.m_nIdx ];
-            const SfxPoolItem* pAnchor = &pFrameFormat->GetAnchor();
-            pFrameFormat->CallSwClientNotify(sw::LegacyModifyHint(pAnchor, pAnchor));
+            SwFrameFormat *pFrameFormat = (*pSpz)[ aEntry.m_nIdx ];
+            SfxPoolItem const *pAnchor = &pFrameFormat->GetAnchor();
+            pFrameFormat->NotifyClients( pAnchor, pAnchor );
         }
     }
 }
diff --git a/sw/source/core/docnode/node.cxx b/sw/source/core/docnode/node.cxx
index 2fb5e255148e..2ed5ff85a3c4 100644
--- a/sw/source/core/docnode/node.cxx
+++ b/sw/source/core/docnode/node.cxx
@@ -1178,7 +1178,7 @@ void SwContentNode::SwClientNotify( const SwModify&, const SfxHint& rHint)
             AttrSetHandleHelper::SetParent(mpAttrSet, *this, pFormatColl, pFormatColl);
         if(bCalcHidden)
             static_cast<SwTextNode*>(this)->SetCalcHiddenCharFlags();
-        CallSwClientNotify(rHint);
+        NotifyClients(pLegacyHint->m_pOld, pLegacyHint->m_pNew);
     }
     else if (auto pModifyChangedHint = dynamic_cast<const sw::ModifyChangedHint*>(&rHint))
     {
@@ -1904,7 +1904,7 @@ void SwContentNode::SetCondFormatColl(SwFormatColl* pColl)
     {
         SwFormatChg aTmp1(pOldColl ? pOldColl : GetFormatColl());
         SwFormatChg aTmp2(pColl ? pColl : GetFormatColl());
-        CallSwClientNotify(sw::LegacyModifyHint(&aTmp1, &aTmp2));
+        NotifyClients(&aTmp1, &aTmp2);
     }
     if(IsInCache())
     {
diff --git a/sw/source/core/edit/editsh.cxx b/sw/source/core/edit/editsh.cxx
index 60a0b7eea389..c956c2544c44 100644
--- a/sw/source/core/edit/editsh.cxx
+++ b/sw/source/core/edit/editsh.cxx
@@ -329,7 +329,7 @@ void SwEditShell::SetGraphicPolygon( const tools::PolyPolygon *pPoly )
     pNd->SetContour( pPoly );
     SwFlyFrame *pFly = static_cast<SwFlyFrame*>(pNd->getLayoutFrame(GetLayout())->GetUpper());
     const SwFormatSurround &rSur = pFly->GetFormat()->GetSurround();
-    pFly->GetFormat()->CallSwClientNotify(sw::LegacyModifyHint(&rSur, &rSur));
+    pFly->GetFormat()->NotifyClients( &rSur, &rSur );
     GetDoc()->getIDocumentState().SetModified();
     EndAllAction();
 }
@@ -344,7 +344,7 @@ void SwEditShell::ClearAutomaticContour()
         pNd->SetContour( nullptr );
         SwFlyFrame *pFly = static_cast<SwFlyFrame*>(pNd->getLayoutFrame(GetLayout())->GetUpper());
         const SwFormatSurround &rSur = pFly->GetFormat()->GetSurround();
-        pFly->GetFormat()->CallSwClientNotify(sw::LegacyModifyHint(&rSur, &rSur));
+        pFly->GetFormat()->NotifyClients( &rSur, &rSur );
         GetDoc()->getIDocumentState().SetModified();
         EndAllAction();
     }
diff --git a/sw/source/core/fields/reffld.cxx b/sw/source/core/fields/reffld.cxx
index 10008bdd73ac..a6c6185c28af 100644
--- a/sw/source/core/fields/reffld.cxx
+++ b/sw/source/core/fields/reffld.cxx
@@ -1090,7 +1090,7 @@ void SwGetRefFieldType::UpdateGetReferences()
         // #i81002#
         pGRef->UpdateField(pFormatField->GetTextField());
     }
-    CallSwClientNotify(sw::LegacyModifyHint(nullptr, nullptr));
+    NotifyClients(nullptr, nullptr);
 }
 
 void SwGetRefFieldType::SwClientNotify(const SwModify&, const SfxHint& rHint)
@@ -1105,7 +1105,7 @@ void SwGetRefFieldType::SwClientNotify(const SwModify&, const SfxHint& rHint)
         UpdateGetReferences();
     else
         // forward to text fields, they "expand" the text
-        CallSwClientNotify(rHint);
+        NotifyClients(pLegacy->m_pOld, pLegacy->m_pNew);
 }
 
 namespace sw {
diff --git a/sw/source/core/layout/anchoredobject.cxx b/sw/source/core/layout/anchoredobject.cxx
index 5c7bec0b44b2..eb2ef8016a22 100644
--- a/sw/source/core/layout/anchoredobject.cxx
+++ b/sw/source/core/layout/anchoredobject.cxx
@@ -86,7 +86,6 @@ SwAnchoredObject::SwAnchoredObject() :
     mbClearedEnvironment( false ),
     // --> #i3317#
     mbTmpConsiderWrapInfluence( false ),
-    mbInvalidatingObjects( false ),
     // --> #i68520#
     maObjRectWithSpaces(),
     mbObjRectWithSpacesValid( false ),
@@ -616,12 +615,10 @@ void SwAnchoredObject::SetObjLeft( const SwTwips _nLeft)
     at the anchor frame and all following anchored objects on the page
     frame are invalidated.
 */
-
 void SwAnchoredObject::UpdateObjInSortedList()
 {
-    if ( !GetAnchorFrame() || mbInvalidatingObjects )
+    if ( !GetAnchorFrame() )
         return;
-    mbInvalidatingObjects = true;
 
     if ( GetFrameFormat().getIDocumentSettingAccess().get(DocumentSettingId::CONSIDER_WRAP_ON_OBJECT_POSITION) )
     {
@@ -629,10 +626,11 @@ void SwAnchoredObject::UpdateObjInSortedList()
         if ( GetAnchorFrame()->GetDrawObjs() )
         {
             const SwSortedObjs* pObjs = GetAnchorFrame()->GetDrawObjs();
-            for(auto it = pObjs->begin(); it != pObjs->end(); ++it)
+            // determine start index
+            for (auto it = pObjs->begin(); it != pObjs->end(); ++it)
             {
                 SwAnchoredObject* pAnchoredObj = *it;
-                if(pAnchoredObj->ConsiderObjWrapInfluenceOnObjPos())
+                if ( pAnchoredObj->ConsiderObjWrapInfluenceOnObjPos() )
                     pAnchoredObj->InvalidateObjPosForConsiderWrapInfluence();
                 else
                     pAnchoredObj->InvalidateObjPos();
@@ -661,7 +659,6 @@ void SwAnchoredObject::UpdateObjInSortedList()
     {
         GetPageFrame()->GetSortedObjs()->Update( *this );
     }
-    mbInvalidatingObjects = false;
 }
 
 /** method to determine, if invalidation of position is allowed
diff --git a/sw/source/core/layout/pagechg.cxx b/sw/source/core/layout/pagechg.cxx
index 1bc8d05a8167..95321684de87 100644
--- a/sw/source/core/layout/pagechg.cxx
+++ b/sw/source/core/layout/pagechg.cxx
@@ -1700,12 +1700,12 @@ void SwRootFrame::AssertPageFlys( SwPageFrame *pPage )
                         // It can move by itself. Just send a modify to its anchor attribute.
 #if OSL_DEBUG_LEVEL > 1
                         const size_t nCnt = pPage->GetSortedObjs()->size();
-                        rFormat.CallSwClientNotify(sw::LegacyModifyHint(nullptr, &rAnch));
+                        rFormat.NotifyClients( nullptr, &rAnch );
                         OSL_ENSURE( !pPage->GetSortedObjs() ||
                                 nCnt != pPage->GetSortedObjs()->size(),
                                 "Object couldn't be reattached!" );
 #else
-                        rFormat.CallSwClientNotify(sw::LegacyModifyHint(nullptr, &rAnch));
+                        rFormat.NotifyClients( nullptr, &rAnch );
 #endif
                         // Do not increment index, in this case
                         continue;
diff --git a/sw/source/core/layout/pagedesc.cxx b/sw/source/core/layout/pagedesc.cxx
index 3fd05a3d70ae..2f7c9e0047bd 100644
--- a/sw/source/core/layout/pagedesc.cxx
+++ b/sw/source/core/layout/pagedesc.cxx
@@ -269,7 +269,7 @@ void SwPageDesc::SwClientNotify(const SwModify& rModify, const SfxHint& rHint)
                 : pLegacyHint->m_pNew
                 ? pLegacyHint->m_pNew->Which()
                 : 0;
-        CallSwClientNotify(rHint);
+        NotifyClients(pLegacyHint->m_pOld, pLegacyHint->m_pNew);
         if((RES_ATTRSET_CHG == nWhich)
                 || (RES_FMT_CHG == nWhich)
                 || isCHRATR(nWhich)
diff --git a/sw/source/core/tox/tox.cxx b/sw/source/core/tox/tox.cxx
index 466b7d6acdd9..f201578a84e0 100644
--- a/sw/source/core/tox/tox.cxx
+++ b/sw/source/core/tox/tox.cxx
@@ -156,7 +156,7 @@ void SwTOXMark::Notify(const SfxHint& rHint)
 {
     if (auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
     {
-        CallSwClientNotify(rHint);
+        NotifyClients(pLegacyHint->m_pOld, pLegacyHint->m_pNew);
         if (pLegacyHint->m_pOld && (RES_REMOVE_UNO_OBJECT == pLegacyHint->m_pOld->Which()))
             SetXTOXMark(css::uno::Reference<css::text::XDocumentIndexMark>(nullptr));
     } else if (auto pCollectHint = dynamic_cast<const sw::CollectTextMarksHint*>(&rHint))
diff --git a/sw/source/core/txtnode/atrfld.cxx b/sw/source/core/txtnode/atrfld.cxx
index f8f326fc8f8f..aa4dab94b8e5 100644
--- a/sw/source/core/txtnode/atrfld.cxx
+++ b/sw/source/core/txtnode/atrfld.cxx
@@ -294,7 +294,7 @@ void SwFormatField::UpdateTextNode(const SfxPoolItem* pOld, const SfxPoolItem* p
     {   // invalidate cached UNO object
         m_wXTextField = nullptr;
         // ??? why does this Modify method not already do this?
-        CallSwClientNotify(sw::LegacyModifyHint(pOld, pNew));
+        NotifyClients(pOld, pNew);
         return;
     }
 
diff --git a/sw/source/core/txtnode/atrftn.cxx b/sw/source/core/txtnode/atrftn.cxx
index 02188cf011de..5ec1f6f93a01 100644
--- a/sw/source/core/txtnode/atrftn.cxx
+++ b/sw/source/core/txtnode/atrftn.cxx
@@ -165,7 +165,7 @@ void SwFormatFootnote::InvalidateFootnote()
 {
     SwPtrMsgPoolItem const item(RES_REMOVE_UNO_OBJECT,
             &static_cast<sw::BroadcastingModify&>(*this)); // cast to base class (void*)
-    CallSwClientNotify(sw::LegacyModifyHint(&item, &item));
+    NotifyClients(&item, &item);
 }
 
 void SwFormatFootnote::SetEndNote( bool b )
diff --git a/sw/source/core/txtnode/atrref.cxx b/sw/source/core/txtnode/atrref.cxx
index 8b11c4e525c7..fdea6ce5dad7 100644
--- a/sw/source/core/txtnode/atrref.cxx
+++ b/sw/source/core/txtnode/atrref.cxx
@@ -68,7 +68,7 @@ void SwFormatRefMark::InvalidateRefMark()
 {
     SwPtrMsgPoolItem const item(RES_REMOVE_UNO_OBJECT,
             &static_cast<sw::BroadcastingModify&>(*this)); // cast to base class (void*)
-    CallSwClientNotify(sw::LegacyModifyHint(&item, &item));
+    NotifyClients(&item, &item);
 }
 
 // attribute for content references in the text
diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index 79b0f389d358..e71ea4b76119 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -396,7 +396,7 @@ void MoveMergedFlysAndFootnotes(std::vector<SwTextFrame*> const& rFrames,
                 if (rFirstNode.GetIndex() < rAnchor.GetContentAnchor()->nNode.GetIndex())
                 {
                     // move it to the new frame of "this"
-                    rFormat.CallSwClientNotify(sw::LegacyModifyHint(&rAnchor, &rAnchor));
+                    rFormat.NotifyClients(&rAnchor, &rAnchor);
                     // note pObjs will be deleted if it becomes empty
                     assert(!pFrame->GetDrawObjs() || !pObjs->Contains(*pObj));
                 }
@@ -2278,7 +2278,7 @@ OUString SwTextNode::InsertText( const OUString & rStr, const SwIndex & rIdx,
     if ( HasWriterListeners() )
     {   // send this before messing with hints, which will send RES_UPDATE_ATTR
         SwInsText aHint( aPos, nLen );
-        CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+        NotifyClients( nullptr, &aHint );
     }
 
     if ( HasHints() )
@@ -2708,12 +2708,12 @@ void SwTextNode::EraseText(const SwIndex &rIdx, const sal_Int32 nCount,
     if( 1 == nCnt )
     {
         SwDelChr aHint( nStartIdx );
-        CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+        NotifyClients( nullptr, &aHint );
     }
     else
     {
         SwDelText aHint( nStartIdx, nCnt );
-        CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+        NotifyClients( nullptr, &aHint );
     }
 
     OSL_ENSURE(rIdx.GetIndex() == nStartIdx, "huh? start index has changed?");
@@ -2765,9 +2765,9 @@ void SwTextNode::GCAttr()
             nMax,
             0);
 
-        CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+        NotifyClients( nullptr, &aHint );
         SwFormatChg aNew( GetTextColl() );
-        CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aNew));
+        NotifyClients( nullptr, &aNew );
     }
 }
 
@@ -2839,7 +2839,7 @@ void SwTextNode::NumRuleChgd()
     // Important note:
     {
         SvxLRSpaceItem& rLR = const_cast<SvxLRSpaceItem&>(GetSwAttrSet().GetLRSpace());
-        CallSwClientNotify(sw::LegacyModifyHint(&rLR, &rLR));
+        NotifyClients( &rLR, &rLR );
     }
 
     SetWordCountDirty( true );
@@ -3708,12 +3708,12 @@ void SwTextNode::ReplaceText( const SwIndex& rStart, const sal_Int32 nDelLen,
 
     SetIgnoreDontExpand( bOldExpFlg );
     SwDelText aDelHint( nStartPos, nDelLen );
-    CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aDelHint));
+    NotifyClients( nullptr, &aDelHint );
 
     if (sInserted.getLength())
     {
         SwInsText aHint( nStartPos, sInserted.getLength() );
-        CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+        NotifyClients( nullptr, &aHint );
     }
 }
 
diff --git a/sw/source/core/txtnode/thints.cxx b/sw/source/core/txtnode/thints.cxx
index 6052754c8b46..a5626e71f053 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -1709,7 +1709,7 @@ void SwTextNode::DeleteAttribute( SwTextAttr * const pAttr )
 
         m_pSwpHints->Delete( pAttr );
         SwTextAttr::Destroy( pAttr, GetDoc().GetAttrPool() );
-        CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+        NotifyClients( nullptr, &aHint );
 
         TryDeleteSwpHints();
     }
@@ -1785,7 +1785,7 @@ void SwTextNode::DeleteAttributes(
 
                 m_pSwpHints->DeleteAtPos( nPos );
                 SwTextAttr::Destroy( pTextHt, GetDoc().GetAttrPool() );
-                CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+                NotifyClients( nullptr, &aHint );
             }
         }
     }
@@ -2586,7 +2586,7 @@ void SwTextNode::FormatToTextAttr( SwTextNode* pNd )
             if( aNdSet.Count() )
             {
                 SwFormatChg aTmp1( pNd->GetFormatColl() );
-                pNd->CallSwClientNotify(sw::LegacyModifyHint(&aTmp1, &aTmp1));
+                pNd->NotifyClients( &aTmp1, &aTmp1 );
             }
         }
     }
diff --git a/sw/source/core/txtnode/txtedt.cxx b/sw/source/core/txtnode/txtedt.cxx
index ba25c5fccba1..edc0ac296d00 100644
--- a/sw/source/core/txtnode/txtedt.cxx
+++ b/sw/source/core/txtnode/txtedt.cxx
@@ -634,9 +634,9 @@ void SwTextNode::RstTextAttr(
         nMax,
         0);
 
-    CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+    NotifyClients( nullptr, &aHint );
     SwFormatChg aNew( GetFormatColl() );
-    CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aNew));
+    NotifyClients( nullptr, &aNew );
 }
 
 static sal_Int32 clipIndexBounds(const OUString &rStr, sal_Int32 nPos)
@@ -1983,10 +1983,10 @@ void SwTextNode::ReplaceTextOnly( sal_Int32 nPos, sal_Int32 nLen,
 
     // notify the layout!
     SwDelText aDelHint( nPos, nTLen );
-    CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aDelHint));
+    NotifyClients( nullptr, &aDelHint );
 
     SwInsText aHint( nPos, nTLen );
-    CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aHint));
+    NotifyClients( nullptr, &aHint );
 }
 
 // the return values allows us to see if we did the heavy-
diff --git a/sw/source/core/unocore/unoobj.cxx b/sw/source/core/unocore/unoobj.cxx
index 964471ded48c..bf0f89ac0bbe 100644
--- a/sw/source/core/unocore/unoobj.cxx
+++ b/sw/source/core/unocore/unoobj.cxx
@@ -2483,7 +2483,7 @@ void SAL_CALL SwXTextCursor::invalidateMarkings(::sal_Int32 nType)
     if (fmtColl == nullptr) return;
 
     SwFormatChg aNew( fmtColl );
-    txtNode->CallSwClientNotify(sw::LegacyModifyHint(nullptr, &aNew));
+    txtNode->NotifyClients( nullptr, &aNew );
 }
 
 void SAL_CALL