sw/source/core/doc: Stop abusing observer pattern for code obfuscation.

The old SwClient/SwModify combo is a questionable implementation of the
observer pattern (among other things). The one thing the observer
pattern is good for is dependency inversion: The creator of the message
does not need to know the type of the receiver. Calling the message
handling on the receiver directly introduces tight coupling and entirely
defeats the purpose, leaving us with the worst of both worlds.

In such case, at least be honest about the tight coupling and call a
somewhat more explicit member function of the target.

Thus introduce SwFootNoteInfo::UpdateFormatOrAttr,
SwContentNode::UpdateAttr, remove some useless SwFormatChg clutter in
Set{Foot,End}NodeInfo, and check for some invariants in
SwContentNode::SwClientNotify to limit some of the "a message are two
void pointers" madness.

Change-Id: I32a8d6973231bb5f65c9e144be72d5bcf98f3f44
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105104
Tested-by: Jenkins
Reviewed-by: Bjoern Michaelsen <bjoern.michaelsen@libreoffice.org>

15 files changed, 71 insertions, 50 deletions

diff --git a/sw/inc/ftninfo.hxx b/sw/inc/ftninfo.hxx
index c9084aef3288..17742687c944 100644
--- a/sw/inc/ftninfo.hxx
+++ b/sw/inc/ftninfo.hxx
@@ -72,6 +72,7 @@ public:
 
     void SetPrefix(const OUString& rSet) { m_sPrefix = rSet; }
     void SetSuffix(const OUString& rSet) { m_sSuffix = rSet; }
+    void UpdateFormatOrAttr();
 };
 
 enum SwFootnotePos
diff --git a/sw/inc/node.hxx b/sw/inc/node.hxx
index e1ed5c9a03ac..3708e82f1c5c 100644
--- a/sw/inc/node.hxx
+++ b/sw/inc/node.hxx
@@ -484,6 +484,7 @@ public:
     {
         SwClientNotify(*this, sw::LegacyModifyHint(pOld, pNew));
     }
+    void UpdateAttr(const SwUpdateAttr&);
 
 private:
     SwContentNode( const SwContentNode & rNode ) = delete;
diff --git a/sw/source/core/crsr/bookmrk.cxx b/sw/source/core/crsr/bookmrk.cxx
index f1271c692c21..bcfbaca715a0 100644
--- a/sw/source/core/crsr/bookmrk.cxx
+++ b/sw/source/core/crsr/bookmrk.cxx
@@ -248,8 +248,8 @@ namespace
 
     auto InvalidatePosition(SwPosition const& rPos) -> void
     {
-        SwUpdateAttr const hint(rPos.nContent.GetIndex(), rPos.nContent.GetIndex(), 0);
-        rPos.nNode.GetNode().GetTextNode()->NotifyClients(nullptr, &hint);
+        SwUpdateAttr const aHint(rPos.nContent.GetIndex(), rPos.nContent.GetIndex(), 0);
+        rPos.nNode.GetNode().GetTextNode()->CallSwClientNotify(sw::LegacyModifyHint(&aHint, &aHint));
     }
 }
 
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 48d258c0c3b6..2e018b7a0567 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -2691,8 +2691,8 @@ bool DocumentContentOperationsManager::Overwrite( const SwPaM &rRg, const OUStri
                                 ? pNode->GetpSwpHints()->Count() : 0;
     if( nOldAttrCnt != nNewAttrCnt )
     {
-        SwUpdateAttr aHint(0,0,0);
-        pNode->ModifyBroadcast(nullptr, &aHint);
+        const SwUpdateAttr aHint(0,0,0);
+        pNode->TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
     }
 
     if (!m_rDoc.GetIDocumentUndoRedo().DoesUndo() &&
diff --git a/sw/source/core/doc/DocumentTimerManager.cxx b/sw/source/core/doc/DocumentTimerManager.cxx
index 842c1262dc0f..dde164bb142f 100644
--- a/sw/source/core/doc/DocumentTimerManager.cxx
+++ b/sw/source/core/doc/DocumentTimerManager.cxx
@@ -194,7 +194,8 @@ IMPL_LINK_NOARG( DocumentTimerManager, DoIdleJobs, Timer*, void )
         const bool bOldLockView = pShell->IsViewLocked();
         pShell->LockView( true );
 
-        m_rDoc.getIDocumentFieldsAccess().GetSysFieldType( SwFieldIds::Chapter )->ModifyNotification( nullptr, nullptr );  // ChapterField
+        auto pChapterFieldType = m_rDoc.getIDocumentFieldsAccess().GetSysFieldType( SwFieldIds::Chapter );
+        pChapterFieldType->SwClientNotify(*pChapterFieldType, sw::LegacyModifyHint( nullptr, nullptr ));  // ChapterField
         m_rDoc.getIDocumentFieldsAccess().UpdateExpFields( nullptr, false );  // Updates ExpressionFields
         m_rDoc.getIDocumentFieldsAccess().UpdateTableFields(nullptr);  // Tables
         m_rDoc.getIDocumentFieldsAccess().UpdateRefFields();  // References
diff --git a/sw/source/core/doc/docdesc.cxx b/sw/source/core/doc/docdesc.cxx
index 0ea944693996..1e0c8d1d6a1d 100644
--- a/sw/source/core/doc/docdesc.cxx
+++ b/sw/source/core/doc/docdesc.cxx
@@ -786,8 +786,7 @@ IMPL_LINK_NOARG( SwDoc, DoUpdateModifiedOLE, Timer *, void )
     ::StartProgress( STR_STATSTR_SWGPRTOLENOTIFY,
                      0, pNodes->size(), GetDocShell());
     getIDocumentLayoutAccess().GetCurrentLayout()->StartAllAction();
-    SwMsgPoolItem aMsgHint( RES_UPDATE_ATTR );
-
+    SwUpdateAttr aHint(0,0,0);
     for( SwOLENodes::size_type i = 0; i < pNodes->size(); ++i )
     {
         ::SetProgressState( i, GetDocShell() );
@@ -799,7 +798,7 @@ IMPL_LINK_NOARG( SwDoc, DoUpdateModifiedOLE, Timer *, void )
         // If it doesn't want to be informed
         if( pOLENd->GetOLEObj().GetOleRef().is() ) // Broken?
         {
-            pOLENd->ModifyNotification( &aMsgHint, &aMsgHint );
+            pOLENd->UpdateAttr(aHint);
         }
     }
     getIDocumentLayoutAccess().GetCurrentLayout()->EndAllAction();
diff --git a/sw/source/core/doc/docfld.cxx b/sw/source/core/doc/docfld.cxx
index dfd44aa959fe..aae20e0da9c4 100644
--- a/sw/source/core/doc/docfld.cxx
+++ b/sw/source/core/doc/docfld.cxx
@@ -920,7 +920,7 @@ void SwDocUpdateField::MakeFieldList_( SwDoc& rDoc, int eGetMode )
 
                         sFormula.clear();
                         // trigger formatting
-                        const_cast<SwFormatField*>(pFormatField)->ModifyNotification( nullptr, nullptr );
+                        const_cast<SwFormatField*>(pFormatField)->UpdateTextNode( nullptr, nullptr );
                     }
                     break;
 
diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx
index 9de4d960a430..3e1817d632fc 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -629,7 +629,7 @@ void SwDoc::SetDefault( const SfxItemSet& rSet )
             {
                 SwFormatChg aChgFormat( mpDfltCharFormat.get() );
                 // notify the frames
-                aCallMod.ModifyNotification( &aChgFormat, &aChgFormat );
+                aCallMod.SwClientNotify(aCallMod, sw::LegacyModifyHint( &aChgFormat, &aChgFormat ));
             }
         }
     }
@@ -638,7 +638,7 @@ void SwDoc::SetDefault( const SfxItemSet& rSet )
     {
         SwAttrSetChg aChgOld( aOld, aOld );
         SwAttrSetChg aChgNew( aNew, aNew );
-        aCallMod.ModifyNotification( &aChgOld, &aChgNew );      // all changed are sent
+        aCallMod.SwClientNotify(aCallMod, sw::LegacyModifyHint( &aChgOld, &aChgNew ));      // all changed are sent
     }
 
     // remove the default formats from the object again
diff --git a/sw/source/core/doc/docftn.cxx b/sw/source/core/doc/docftn.cxx
index eca8d5713f36..8f158ac1c780 100644
--- a/sw/source/core/doc/docftn.cxx
+++ b/sw/source/core/doc/docftn.cxx
@@ -210,30 +210,35 @@ SwCharFormat* SwEndNoteInfo::GetCurrentCharFormat(const bool bAnchor) const
         : m_pCharFormat;
 }
 
+void SwEndNoteInfo::UpdateFormatOrAttr()
+{
+    auto pFormat = GetCurrentCharFormat(m_pCharFormat == nullptr);
+    if (!pFormat || !m_aDepends.IsListeningTo(pFormat) || pFormat->IsFormatInDTOR())
+        return;
+    SwDoc* pDoc = pFormat->GetDoc();
+    SwFootnoteIdxs& rFootnoteIdxs = pDoc->GetFootnoteIdxs();
+    for(auto pTextFootnote : rFootnoteIdxs)
+    {
+        const SwFormatFootnote &rFootnote = pTextFootnote->GetFootnote();
+        if(rFootnote.IsEndNote() == m_bEndNote)
+            pTextFootnote->SetNumber(rFootnote.GetNumber(), rFootnote.GetNumberRLHidden(), rFootnote.GetNumStr());
+    }
+}
+
+
 void SwEndNoteInfo::SwClientNotify( const SwModify& rModify, const SfxHint& rHint)
 {
     if (auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
     {
-        const sal_uInt16 nWhich = pLegacyHint->m_pOld ? pLegacyHint->m_pOld->Which() : pLegacyHint->m_pNew ? pLegacyHint->m_pNew->Which() : 0 ;
-        if (RES_ATTRSET_CHG == nWhich || RES_FMT_CHG == nWhich)
+        switch(pLegacyHint->GetWhich())
         {
-            auto pFormat = GetCurrentCharFormat(m_pCharFormat == nullptr);
-            if (!pFormat || !m_aDepends.IsListeningTo(pFormat) || pFormat->IsFormatInDTOR())
-                return;
-            SwDoc* pDoc = pFormat->GetDoc();
-            SwFootnoteIdxs& rFootnoteIdxs = pDoc->GetFootnoteIdxs();
-            for( size_t nPos = 0; nPos < rFootnoteIdxs.size(); ++nPos )
-            {
-                SwTextFootnote *pTextFootnote = rFootnoteIdxs[ nPos ];
-                const SwFormatFootnote &rFootnote = pTextFootnote->GetFootnote();
-                if ( rFootnote.IsEndNote() == m_bEndNote )
-                {
-                    pTextFootnote->SetNumber(rFootnote.GetNumber(), rFootnote.GetNumberRLHidden(), rFootnote.GetNumStr());
-                }
-            }
+            case RES_ATTRSET_CHG:
+            case RES_FMT_CHG:
+                UpdateFormatOrAttr();
+                break;
+            default:
+                CheckRegistration( pLegacyHint->m_pOld );
         }
-        else
-            CheckRegistration( pLegacyHint->m_pOld );
     }
     else if (auto pModifyChangedHint = dynamic_cast<const sw::ModifyChangedHint*>(&rHint))
     {
@@ -346,9 +351,7 @@ void SwDoc::SetFootnoteInfo(const SwFootnoteInfo& rInfo)
         GetFootnoteIdxs().UpdateAllFootnote();
     else if( bFootnoteChrFormats )
     {
-        SwFormatChg aOld( pOldChrFormat );
-        SwFormatChg aNew( pNewChrFormat );
-        mpFootnoteInfo->ModifyNotification( &aOld, &aNew );
+        mpFootnoteInfo->UpdateFormatOrAttr();
     }
 
     // #i81002# no update during loading
@@ -415,9 +418,7 @@ void SwDoc::SetEndNoteInfo(const SwEndNoteInfo& rInfo)
         GetFootnoteIdxs().UpdateAllFootnote();
     else if( bFootnoteChrFormats )
     {
-        SwFormatChg aOld( pOldChrFormat );
-        SwFormatChg aNew( pNewChrFormat );
-        mpEndNoteInfo->ModifyNotification( &aOld, &aNew );
+        mpEndNoteInfo->UpdateFormatOrAttr();
     }
 
     // #i81002# no update during loading
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index ecb510869bbf..0c0870f52f62 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -1267,7 +1267,7 @@ void SwRangeRedline::InvalidateRange(Invalidation const eWhy)
                 n == nEndNd ? nEndCnt : pNd->GetText().getLength(),
                 RES_FMT_CHG);
 
-            pNd->ModifyNotification(&aHt, &aHt);
+            pNd->TriggerNodeUpdate(sw::LegacyModifyHint(&aHt, &aHt));
 
             // SwUpdateAttr must be handled first, otherwise indexes are off
             if (GetType() == RedlineType::Delete)
diff --git a/sw/source/core/docnode/node.cxx b/sw/source/core/docnode/node.cxx
index 25aeec655363..abfc0d13e827 100644
--- a/sw/source/core/docnode/node.cxx
+++ b/sw/source/core/docnode/node.cxx
@@ -69,6 +69,7 @@
 #include <swcrsr.hxx>
 #include <hints.hxx>
 #include <frameformats.hxx>
+#include <sal/backtrace.hxx>
 
 using namespace ::com::sun::star::i18n;
 
@@ -1090,6 +1091,14 @@ SwContentNode::~SwContentNode()
     if ( mpAttrSet && mbSetModifyAtAttr )
         const_cast<SwAttrSet*>(static_cast<const SwAttrSet*>(mpAttrSet.get()))->SetModifyAtAttr( nullptr );
 }
+void SwContentNode::UpdateAttr(const SwUpdateAttr& rUpdate)
+{
+    if (GetNodes().IsDocNodes()
+            && IsTextNode()
+            && RES_ATTRSET_CHG == rUpdate.getWhichAttr())
+        static_cast<SwTextNode*>(this)->SetCalcHiddenCharFlags();
+    CallSwClientNotify(sw::LegacyModifyHint(&rUpdate, &rUpdate));
+}
 
 void SwContentNode::SwClientNotify( const SwModify&, const SfxHint& rHint)
 {
@@ -1155,12 +1164,22 @@ void SwContentNode::SwClientNotify( const SwModify&, const SfxHint& rHint)
                 break;
 
             case RES_UPDATE_ATTR:
-                if (GetNodes().IsDocNodes()
-                        && IsTextNode()
-                        && pLegacyHint->m_pNew
-                        && RES_ATTRSET_CHG == static_cast<const SwUpdateAttr*>(pLegacyHint->m_pNew)->getWhichAttr())
-                    bCalcHidden = true;
-                break;
+                // RES_UPDATE_ATTR _should_ always contain a SwUpdateAttr hint in old and new.
+                // However, faking one with just a basic SfxPoolItem setting a WhichId has been observed.
+                // This makes the crude "WhichId" type divert from the true type, which is bad.
+                // Thus we are asserting here, but falling back to an proper
+                // hint instead. so that we at least will not spread such poison further.
+                if(pLegacyHint->m_pNew != pLegacyHint->m_pOld)
+                {
+                    auto pBT = sal::backtrace_get(20);
+                    SAL_WARN("sw.core", "UpdateAttr not matching! " << sal::backtrace_to_string(pBT.get()));
+                }
+                assert(pLegacyHint->m_pNew == pLegacyHint->m_pOld);
+                assert(dynamic_cast<const SwUpdateAttr*>(pLegacyHint->m_pNew));
+                const SwUpdateAttr aFallbackHint(0,0,0);
+                const SwUpdateAttr& rUpdateAttr = pLegacyHint->m_pNew ? *static_cast<const SwUpdateAttr*>(pLegacyHint->m_pNew) : aFallbackHint;
+                UpdateAttr(rUpdateAttr);
+                return;
         }
         if(bSetParent && GetpSwAttrSet())
             AttrSetHandleHelper::SetParent(mpAttrSet, *this, pFormatColl, pFormatColl);
diff --git a/sw/source/core/fields/docufld.cxx b/sw/source/core/fields/docufld.cxx
index 051e7e674c09..b32c4937ed1e 100644
--- a/sw/source/core/fields/docufld.cxx
+++ b/sw/source/core/fields/docufld.cxx
@@ -2307,7 +2307,7 @@ void SwRefPageGetFieldType::UpdateField( SwTextField const * pTextField,
         }
     }
     // start formatting
-    const_cast<SwFormatField&>(pTextField->GetFormatField()).ModifyNotification( nullptr, nullptr );
+    const_cast<SwFormatField&>(pTextField->GetFormatField()).UpdateTextNode(nullptr, nullptr);
 }
 
 // queries for relative page numbering
diff --git a/sw/source/core/graphic/ndgrf.cxx b/sw/source/core/graphic/ndgrf.cxx
index f7e9f66e14e3..583ac191fbb2 100644
--- a/sw/source/core/graphic/ndgrf.cxx
+++ b/sw/source/core/graphic/ndgrf.cxx
@@ -272,8 +272,8 @@ bool SwGrfNode::ReRead(
     // create an updates for the frames
     if( bReadGrf && bNewGrf )
     {
-        SwMsgPoolItem aMsgHint( RES_UPDATE_ATTR );
-        lcl_SwClientNotify(*this, aMsgHint);
+        const SwUpdateAttr aHint(0,0,0);
+        lcl_SwClientNotify(*this, aHint);
     }
 
     return bReadGrf;
diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index f3e93f934c10..951fc6d63012 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -1953,7 +1953,7 @@ void SwTextNode::CopyAttr( SwTextNode *pDest, const sal_Int32 nTextStartIdx,
             nOldPos,
             0);
 
-        pDest->ModifyNotification( nullptr, &aHint );
+        pDest->TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
     }
 }
 
diff --git a/sw/source/core/txtnode/thints.cxx b/sw/source/core/txtnode/thints.cxx
index 593674ff8159..e9deb654a54e 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -3221,7 +3221,7 @@ bool SwpHints::TryInsertHint(
                 nHtStart,
                 nWhich);
 
-            rNode.ModifyNotification(nullptr,&aHint);
+            rNode.TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
         }
 
         return true;
@@ -3298,9 +3298,8 @@ bool SwpHints::TryInsertHint(
     // ... and notify listeners
     if ( rNode.HasWriterListeners() )
     {
-        SwUpdateAttr aHint(nHtStart, nHintEnd, nWhich, aWhichSublist);
-
-        rNode.ModifyNotification( nullptr, &aHint );
+        const SwUpdateAttr aHint(nHtStart, nHintEnd, nWhich, aWhichSublist);
+        rNode.TriggerNodeUpdate(sw::LegacyModifyHint(&aHint, &aHint));
     }
 
 #ifdef DBG_UTIL