Revert "Revert "Move SwFntCache link from SwModify down to SwFormat""

- SwFormat::SwClientNotify calls SwModfiy::SwClientNotify explicitly
and used the sideeffect of cache invalidation
- this is now fixed

This reverts commit 2e29dc20b96f2d96f5b64e9ed5efb79e342b3f54.

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

6 files changed, 71 insertions, 57 deletions

diff --git a/sw/inc/calbck.hxx b/sw/inc/calbck.hxx
index 31df9791291b..b5b6ff9a3e30 100644
--- a/sw/inc/calbck.hxx
+++ b/sw/inc/calbck.hxx
@@ -178,7 +178,6 @@ class SW_DLLPUBLIC SwModify: public SwClient
     sw::WriterListener* m_pWriterListeners;                // the start of the linked list of clients
     bool m_bModifyLocked : 1;         // don't broadcast changes now
     bool m_bInCache   : 1;
-    bool m_bInSwFntCache : 1;
 
     SwModify(SwModify const &) = delete;
     SwModify &operator =(const SwModify&) = delete;
@@ -186,7 +185,7 @@ protected:
     virtual void SwClientNotify(const SwModify&, const SfxHint& rHint) override;
 public:
     SwModify()
-        : SwClient(), m_pWriterListeners(nullptr), m_bModifyLocked(false), m_bInCache(false), m_bInSwFntCache(false)
+        : SwClient(), m_pWriterListeners(nullptr), m_bModifyLocked(false), m_bInCache(false)
     {}
 
     // broadcasting mechanism
@@ -204,11 +203,9 @@ public:
     void LockModify()                   { m_bModifyLocked = true;  }
     void UnlockModify()                 { m_bModifyLocked = false; }
     void SetInCache( bool bNew )        { m_bInCache = bNew;       }
-    void SetInSwFntCache( bool bNew )   { m_bInSwFntCache = bNew;  }
     void SetInDocDTOR();
     bool IsModifyLocked() const     { return m_bModifyLocked;  }
     bool IsInCache()      const     { return m_bInCache;       }
-    bool IsInSwFntCache() const     { return m_bInSwFntCache;  }
 
     void CheckCaching( const sal_uInt16 nWhich );
     bool HasOnlyOneListener() const { return m_pWriterListeners && m_pWriterListeners->IsLast(); }
diff --git a/sw/inc/format.hxx b/sw/inc/format.hxx
index e596a26bb882..96e03b342eec 100644
--- a/sw/inc/format.hxx
+++ b/sw/inc/format.hxx
@@ -22,6 +22,7 @@
 #include "swdllapi.h"
 #include "swatrset.hxx"
 #include "calbck.hxx"
+#include "hintids.hxx"
 #include <memory>
 
 class IDocumentSettingAccess;
@@ -59,7 +60,25 @@ class SW_DLLPUBLIC SwFormat : public sw::BroadcastingModify
     bool   m_bAutoUpdateFormat : 1;/**< TRUE: Set attributes of a whole paragraph
                                        at format (UI-side!). */
     bool m_bHidden : 1;
+    bool m_bInSwFntCache : 1;
     std::shared_ptr<SfxGrabBagItem> m_pGrabBagItem; ///< Style InteropGrabBag.
+    void InvalidateInSwFntCache(sal_uInt16 nWhich)
+    {
+        if(isCHRATR(nWhich))
+        {
+            m_bInSwFntCache = false;
+        }
+        else
+        {
+            switch(nWhich)
+            {
+                case RES_OBJECTDYING:
+                case RES_FMT_CHG:
+                case RES_ATTRSET_CHG:
+                    m_bInSwFntCache = false;
+            }
+        }
+    };
 
 protected:
     SwFormat( SwAttrPool& rPool, const char* pFormatNm,
@@ -75,7 +94,9 @@ public:
     SwFormat &operator=(const SwFormat&);
 
     /// for Querying of Writer-functions.
-    sal_uInt16 Which() const { return m_nWhichId; }
+    sal_uInt16 Which() const { return m_nWhichId; };
+    bool IsInSwFntCache() const { return m_bInSwFntCache; };
+    void SetInSwFntCache() { m_bInSwFntCache = true; };
 
     /// Copy attributes even among documents.
     void CopyAttrs( const SwFormat& );
diff --git a/sw/source/core/attr/calbck.cxx b/sw/source/core/attr/calbck.cxx
index 1c86c75fc992..a4fff2809f72 100644
--- a/sw/source/core/attr/calbck.cxx
+++ b/sw/source/core/attr/calbck.cxx
@@ -17,15 +17,17 @@
  *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
  */
 
-#include <frame.hxx>
+
+#include <algorithm>
 #include <format.hxx>
+#include <frame.hxx>
 #include <hintids.hxx>
 #include <hints.hxx>
+#include <osl/diagnose.hxx>
+#include <sal/log.hxx>
 #include <swcache.hxx>
-#include <swfntcch.hxx>
 #include <tools/debug.hxx>
-#include <sal/log.hxx>
-#include <algorithm>
+
 #ifdef DBG_UTIL
 #include <sal/backtrace.hxx>
 #endif
@@ -156,9 +158,6 @@ SwModify::~SwModify()
     if ( IsInCache() )
         SwFrame::GetCache().Delete( this );
 
-    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));
@@ -274,21 +273,13 @@ SwClient* SwModify::Remove( SwClient* pDepend )
     return pDepend;
 }
 
-void SwModify::CheckCaching( const sal_uInt16 nWhich )
+void SwModify::CheckCaching(const sal_uInt16 nWhich)
 {
-    if( isCHRATR( nWhich ) )
+    switch(nWhich)
     {
-        SetInSwFntCache( false );
-    }
-    else
-    {
-        switch( nWhich )
-        {
         case RES_OBJECTDYING:
         case RES_FMT_CHG:
         case RES_ATTRSET_CHG:
-            SetInSwFntCache( false );
-            [[fallthrough]];
         case RES_UL_SPACE:
         case RES_LR_SPACE:
         case RES_BOX:
@@ -296,13 +287,11 @@ void SwModify::CheckCaching( const sal_uInt16 nWhich )
         case RES_FRM_SIZE:
         case RES_KEEP:
         case RES_BREAK:
-            if( IsInCache() )
+            if(IsInCache())
             {
-                SwFrame::GetCache().Delete( this );
-                SetInCache( false );
+                SwFrame::GetCache().Delete(this);
+                SetInCache(false);
             }
-            break;
-        }
     }
 }
 
@@ -352,7 +341,7 @@ void SwModify::SwClientNotify(const SwModify&, const SfxHint& rHint)
     if(auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
     {
         DBG_TESTSOLARMUTEX();
-        if(IsInCache() || IsInSwFntCache())
+        if(IsInCache())
             CheckCaching(pLegacyHint->GetWhich());
         if(IsModifyLocked())
             return;
diff --git a/sw/source/core/attr/format.cxx b/sw/source/core/attr/format.cxx
index 6e0f55ef8da0..cb36ad574e67 100644
--- a/sw/source/core/attr/format.cxx
+++ b/sw/source/core/attr/format.cxx
@@ -17,26 +17,26 @@
  *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
  */
 
-#include <doc.hxx>
 #include <DocumentSettingManager.hxx> //For SwFmt::getIDocumentSettingAccess()
 #include <IDocumentTimerAccess.hxx>
+#include <doc.hxx>
 #include <fmtcolfunc.hxx>
-#include <frame.hxx>
 #include <format.hxx>
+#include <frame.hxx>
+#include <frmatr.hxx>
 #include <hintids.hxx>
 #include <hints.hxx>
-#include <swcache.hxx>
-#include <frmatr.hxx>
 #include <osl/diagnose.h>
+#include <sal/log.hxx>
 #include <svl/grabbagitem.hxx>
 #include <svx/sdr/attribute/sdrallfillattributeshelper.hxx>
 #include <svx/unobrushitemhelper.hxx>
 #include <svx/xdef.hxx>
-#include <sal/log.hxx>
+#include <swcache.hxx>
+#include <swfntcch.hxx>
 
 using namespace com::sun::star;
 
-
 SwFormat::SwFormat( SwAttrPool& rPool, const char* pFormatNm,
               const sal_uInt16* pWhichRanges, SwFormat *pDrvdFrame,
               sal_uInt16 nFormatWhich ) :
@@ -49,7 +49,7 @@ SwFormat::SwFormat( SwAttrPool& rPool, const char* pFormatNm,
 {
     m_bAutoUpdateFormat = false; // LAYER_IMPL
     m_bAutoFormat = true;
-    m_bFormatInDTOR = m_bHidden = false;
+    m_bFormatInDTOR = m_bHidden = m_bInSwFntCache = false;
 
     if( pDrvdFrame )
     {
@@ -70,7 +70,7 @@ SwFormat::SwFormat( SwAttrPool& rPool, const OUString& rFormatNm,
 {
     m_bAutoUpdateFormat = false; // LAYER_IMPL
     m_bAutoFormat = true;
-    m_bFormatInDTOR = m_bHidden = false;
+    m_bFormatInDTOR = m_bHidden = m_bInSwFntCache = false;
 
     if( pDrvdFrame )
     {
@@ -90,6 +90,7 @@ SwFormat::SwFormat( const SwFormat& rFormat ) :
     m_bFormatInDTOR = false; // LAYER_IMPL
     m_bAutoFormat = rFormat.m_bAutoFormat;
     m_bHidden = rFormat.m_bHidden;
+    m_bInSwFntCache = false;
     m_bAutoUpdateFormat = rFormat.m_bAutoUpdateFormat;
 
     if( auto pDerived = rFormat.DerivedFrom() )
@@ -116,7 +117,7 @@ SwFormat &SwFormat::operator=(const SwFormat& rFormat)
         SwFrame::GetCache().Delete( this );
         SetInCache( false );
     }
-    SetInSwFntCache( false );
+    m_bInSwFntCache = false;
 
     // copy only array with attributes delta
     SwAttrSet aOld( *m_aSet.GetPool(), m_aSet.GetRanges() ),
@@ -183,7 +184,7 @@ void SwFormat::CopyAttrs( const SwFormat& rFormat )
         SwFrame::GetCache().Delete( this );
         SetInCache( false );
     }
-    SetInSwFntCache( false );
+    m_bInSwFntCache = false;
 
     // special treatments for some attributes
     SwAttrSet* pChgSet = const_cast<SwAttrSet*>(&rFormat.m_aSet);
@@ -230,6 +231,8 @@ SwFormat::~SwFormat()
     for(SwClient* pClient = aIter.First(); pClient; pClient = aIter.Next())
         pClient->CheckRegistrationFormat(*this);
     assert(!HasWriterListeners());
+    if(m_bInSwFntCache)
+        pSwFontCache->Delete( this );
 }
 
 void SwFormat::SwClientNotify(const SwModify&, const SfxHint& rHint)
@@ -239,8 +242,7 @@ void SwFormat::SwClientNotify(const SwModify&, const SfxHint& rHint)
         return;
 
     std::unique_ptr<SwAttrSetChg> pOldClientChg, pNewClientChg;
-    auto aDependArgs = std::pair<const SfxPoolItem*, const SfxPoolItem*>(pLegacy->m_pOld, pLegacy->m_pNew);
-    bool bPassToDepends = true;
+    auto pDependsHint = std::make_unique<sw::LegacyModifyHint>(pLegacy->m_pOld, pLegacy->m_pNew);
     const sal_uInt16 nWhich = pLegacy->GetWhich();
     switch(nWhich)
     {
@@ -287,10 +289,10 @@ void SwFormat::SwClientNotify(const SwModify&, const SfxHint& rHint)
                 {
                     pOldClientChg.reset(new SwAttrSetChg(*pOldAttrSetChg));
                     pOldClientChg->GetChgSet()->Differentiate(m_aSet);
-                    aDependArgs = std::pair<const SfxPoolItem*, const SfxPoolItem*>(pOldClientChg.get(), pNewClientChg.get());
+                    pDependsHint.reset(new sw::LegacyModifyHint(pOldClientChg.get(), pNewClientChg.get()));
                 }
                 else
-                    bPassToDepends = false;
+                    pDependsHint.reset(nullptr);
             }
             break;
         }
@@ -316,11 +318,14 @@ void SwFormat::SwClientNotify(const SwModify&, const SfxHint& rHint)
             {
                 // DropCaps might come into this block
                 SAL_WARN_IF(RES_PARATR_DROP != nWhich, "sw.core", "Hint was sent without sender");
-                bPassToDepends = false;
+                pDependsHint.reset(nullptr);
             }
     }
-    if(bPassToDepends)
-        SwModify::SwClientNotify(*this, sw::LegacyModifyHint(aDependArgs.first, aDependArgs.second));
+    if(pDependsHint)
+    {
+        InvalidateInSwFntCache(pDependsHint->GetWhich());
+        SwModify::SwClientNotify(*this, *pDependsHint);
+    }
 }
 
 bool SwFormat::SetDerivedFrom(SwFormat *pDerFrom)
@@ -357,7 +362,7 @@ bool SwFormat::SetDerivedFrom(SwFormat *pDerFrom)
         SwFrame::GetCache().Delete( this );
         SetInCache( false );
     }
-    SetInSwFntCache( false );
+    m_bInSwFntCache = false;
 
     pDerFrom->Add( this );
     m_aSet.SetParent( &pDerFrom->m_aSet );
@@ -458,11 +463,9 @@ SfxItemState SwFormat::GetBackgroundState(std::unique_ptr<SvxBrushItem>& rItem)
 
 bool SwFormat::SetFormatAttr( const SfxPoolItem& rAttr )
 {
-    if ( IsInCache() || IsInSwFntCache() )
-    {
-        const sal_uInt16 nWhich = rAttr.Which();
-        CheckCaching( nWhich );
-    }
+    const sal_uInt16 nWhich = rAttr.Which();
+    InvalidateInSwFntCache( nWhich );
+    CheckCaching( nWhich );
 
     bool bRet = false;
 
@@ -547,7 +550,7 @@ bool SwFormat::SetFormatAttr( const SfxItemSet& rSet )
         SwFrame::GetCache().Delete( this );
         SetInCache( false );
     }
-    SetInSwFntCache( false );
+    m_bInSwFntCache = false;
 
     bool bRet = false;
 
@@ -644,7 +647,12 @@ bool SwFormat::ResetFormatAttr( sal_uInt16 nWhich1, sal_uInt16 nWhich2 )
     if( !nWhich2 || nWhich2 < nWhich1 )
         nWhich2 = nWhich1; // then set to 1st ID, only this item
 
-    if ( IsInCache() || IsInSwFntCache() )
+    if(m_bInSwFntCache)
+    {
+        for( sal_uInt16 n = nWhich1; n < nWhich2; ++n )
+            InvalidateInSwFntCache( n );
+    }
+    if ( IsInCache() )
     {
         for( sal_uInt16 n = nWhich1; n < nWhich2; ++n )
             CheckCaching( n );
@@ -675,7 +683,7 @@ sal_uInt16 SwFormat::ResetAllFormatAttr()
         SwFrame::GetCache().Delete( this );
         SetInCache( false );
     }
-    SetInSwFntCache( false );
+    m_bInSwFntCache = false;
 
     // if Modify is locked then no modifications will be sent
     if( IsModifyLocked() )
@@ -699,7 +707,7 @@ void SwFormat::DelDiffs( const SfxItemSet& rSet )
         SwFrame::GetCache().Delete( this );
         SetInCache( false );
     }
-    SetInSwFntCache( false );
+    m_bInSwFntCache = false;
 
     // if Modify is locked then no modifications will be sent
     if( IsModifyLocked() )
diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index 99331779c4c5..64a7f2a5da85 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -2832,7 +2832,6 @@ void SwTextNode::NumRuleChgd()
         SwFrame::GetCache().Delete( this );
         SetInCache( false );
     }
-    SetInSwFntCache( false );
 
     // Sending "noop" modify in order to cause invalidations of registered
     // <SwTextFrame> instances to get the list style change respectively the change
diff --git a/sw/source/core/txtnode/swfntcch.cxx b/sw/source/core/txtnode/swfntcch.cxx
index 49783fd25942..100a0e7a7965 100644
--- a/sw/source/core/txtnode/swfntcch.cxx
+++ b/sw/source/core/txtnode/swfntcch.cxx
@@ -60,7 +60,7 @@ SwFontObj *SwFontAccess::Get( )
 
 SwCacheObj *SwFontAccess::NewObj( )
 {
-    const_cast<SwTextFormatColl*>(static_cast<const SwTextFormatColl*>(m_pOwner))->SetInSwFntCache( true );
+    const_cast<SwTextFormatColl*>(static_cast<const SwTextFormatColl*>(m_pOwner))->SetInSwFntCache();
     return new SwFontObj( m_pOwner, m_pShell );
 }