SwFrame: Modify no more

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

7 files changed, 51 insertions, 57 deletions

diff --git a/sw/inc/accmap.hxx b/sw/inc/accmap.hxx
index 58f758706cbf..979fac964823 100644
--- a/sw/inc/accmap.hxx
+++ b/sw/inc/accmap.hxx
@@ -61,7 +61,7 @@ namespace com::sun::star::accessibility { class XAccessible; }
 // There are two reason for this. First of all, a new accessible shape
 // for the XShape might be created soon. It's then cheaper if the XShape
 // still exists. The other reason are situations where an accessible shape
-// is destroyed within an SwFrameFormat::Modify. In this case, destroying
+// is destroyed within an SwFrameFormat::SwClientNotify. In this case, destroying
 // the XShape at the same time (indirectly by destroying the accessible
 // shape) leads to an assert, because a client of the Modify is destroyed
 // within a Modify call.
diff --git a/sw/inc/calbck.hxx b/sw/inc/calbck.hxx
index 8fbb2e87e0b2..e469c3dd4134 100644
--- a/sw/inc/calbck.hxx
+++ b/sw/inc/calbck.hxx
@@ -143,11 +143,6 @@ public:
     SwClient() : m_pRegisteredIn(nullptr) {}
     SwClient(SwClient&&) noexcept;
     virtual ~SwClient() override;
-    // callbacks received from SwModify (friend class - so these methods can be private)
-    // should be called only from SwModify the client is registered in
-    // mba: IMHO this method should be pure virtual
-    // DO NOT USE IN NEW CODE! use SwClientNotify instead.
-    virtual void Modify(const SfxPoolItem*, const SfxPoolItem*);
 
 
     // in case an SwModify object is destroyed that itself is registered in another SwModify,
@@ -183,13 +178,10 @@ class SW_DLLPUBLIC SwModify: public SwClient
     bool m_bInCache   : 1;
     bool m_bInSwFntCache : 1;
 
-    // mba: IMHO this method should be pure virtual
-    // DO NOT USE IN NEW CODE! use CallSwClientNotify instead.
-    virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew) override
-        { NotifyClients( pOld, pNew ); };
-
     SwModify(SwModify const &) = delete;
     SwModify &operator =(const SwModify&) = delete;
+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)
@@ -268,7 +260,6 @@ namespace sw
         /** get Client information */
         virtual bool GetInfo( SfxPoolItem& rInfo) const override;
     private:
-        virtual void Modify(const SfxPoolItem* pOldValue, const SfxPoolItem *pNewValue) override;
         virtual void SwClientNotify(const SwModify& rModify, const SfxHint& rHint) override;
     };
 
diff --git a/sw/qa/core/uwriter.cxx b/sw/qa/core/uwriter.cxx
index 9da61561f3ff..db2422fed04b 100644
--- a/sw/qa/core/uwriter.cxx
+++ b/sw/qa/core/uwriter.cxx
@@ -1827,8 +1827,6 @@ namespace
         int m_nModifyChangedCount;
         const SwModify* m_pLastChangedModify;
         TestClient() : m_nModifyCount(0), m_nNotifyCount(0), m_nModifyChangedCount(0), m_pLastChangedModify(nullptr) {};
-        virtual void Modify( const SfxPoolItem*, const SfxPoolItem*) override
-        { assert(false); }
         virtual void SwClientNotify(const SwModify&, const SfxHint& rHint) override
         {
             if(typeid(TestHint) == typeid(rHint))
@@ -1846,8 +1844,11 @@ namespace
     {
         int m_nModifyCount;
         OtherTestClient() : m_nModifyCount(0) {};
-        virtual void Modify( const SfxPoolItem*, const SfxPoolItem*) override
-            { ++m_nModifyCount; }
+        virtual void SwClientNotify(const SwModify&, const SfxHint& rHint) override
+        {
+            if(dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
+                ++m_nModifyCount;
+        }
     };
     struct TestListener : SvtListener
     {
diff --git a/sw/source/core/access/accmap.cxx b/sw/source/core/access/accmap.cxx
index 8f3543dcd4df..4771f4c8e903 100644
--- a/sw/source/core/access/accmap.cxx
+++ b/sw/source/core/access/accmap.cxx
@@ -2293,7 +2293,7 @@ void SwAccessibleMap::A11yDispose( const SwFrame *pFrame,
             (xParentAccImpl.is() || xShapeAccImpl.is()) )
         {
             // Keep a reference to the XShape to avoid that it
-            // is deleted with a SwFrameFormat::Modify.
+            // is deleted with a SwFrameFormat::SwClientNotify.
             uno::Reference < drawing::XShape > xShape(
                 const_cast< SdrObject * >( pObj )->getUnoShape(),
                 uno::UNO_QUERY );
diff --git a/sw/source/core/attr/calbck.cxx b/sw/source/core/attr/calbck.cxx
index 675c4e54993b..c14baebc7f22 100644
--- a/sw/source/core/attr/calbck.cxx
+++ b/sw/source/core/attr/calbck.cxx
@@ -33,11 +33,6 @@ namespace sw
 {
     bool ListenerEntry::GetInfo(SfxPoolItem& rInfo) const
         { return m_pToTell == nullptr || m_pToTell->GetInfo( rInfo ); }
-    void ListenerEntry::Modify(const SfxPoolItem *const pOldValue,
-                               const SfxPoolItem *const pNewValue)
-    {
-        SwClientNotify(*GetRegisteredIn(), sw::LegacyModifyHint(pOldValue, pNewValue));
-    }
     void ListenerEntry::SwClientNotify(const SwModify& rModify, const SfxHint& rHint)
     {
         if (auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
@@ -110,10 +105,8 @@ std::unique_ptr<sw::ModifyChangedHint> SwClient::CheckRegistration( const SfxPoo
 
 void SwClient::SwClientNotify(const SwModify&, const SfxHint& rHint)
 {
-    if (auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
-    {
-        Modify(pLegacyHint->m_pOld, pLegacyHint->m_pNew);
-    }
+    if(auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
+        CheckRegistration(pLegacyHint->m_pOld);
 };
 
 void SwClient::StartListeningToSameModifyAs(const SwClient& other)
@@ -130,11 +123,6 @@ void SwClient::EndListeningAll()
         m_pRegisteredIn->Remove(this);
 }
 
-void SwClient::Modify(SfxPoolItem const*const pOldValue, SfxPoolItem const*const /*pNewValue*/)
-{
-    CheckRegistration( pOldValue );
-}
-
 void SwModify::SetInDocDTOR()
 {
     // If the document gets destroyed anyway, just tell clients to
@@ -359,6 +347,12 @@ void sw::WriterMultiListener::EndListeningAll()
 
 sw::ClientIteratorBase* sw::ClientIteratorBase::s_pClientIters = nullptr;
 
+void SwModify::SwClientNotify(const SwModify&, const SfxHint& rHint)
+{
+    if(auto pLegacyHint = dynamic_cast<const sw::LegacyModifyHint*>(&rHint))
+        NotifyClients(pLegacyHint->m_pOld, pLegacyHint->m_pNew);
+}
+
 void SwModify::CallSwClientNotify( const SfxHint& rHint ) const
 {
     DBG_TESTSOLARMUTEX();
diff --git a/sw/source/core/inc/frame.hxx b/sw/source/core/inc/frame.hxx
index ff1a3aba26de..5a797f20feac 100644
--- a/sw/source/core/inc/frame.hxx
+++ b/sw/source/core/inc/frame.hxx
@@ -489,7 +489,7 @@ protected:
 
     // draw shadow and borders
     void PaintShadow( const SwRect&, SwRect&, const SwBorderAttrs& ) const;
-    virtual void Modify( const SfxPoolItem*, const SfxPoolItem* ) override;
+    virtual void SwClientNotify(const SwModify&, const SfxHint&) override;
 
     virtual const IDocumentDrawModelAccess& getIDocumentDrawModelAccess( );
 
diff --git a/sw/source/core/layout/wsfrm.cxx b/sw/source/core/layout/wsfrm.cxx
index 57b4d433a242..2937e5abaa78 100644
--- a/sw/source/core/layout/wsfrm.cxx
+++ b/sw/source/core/layout/wsfrm.cxx
@@ -473,14 +473,17 @@ void SwTextFrame::CheckDirection( bool bVert )
              bVert, true, bBrowseMode);
 }
 
-void SwFrame::Modify( const SfxPoolItem* pOld, const SfxPoolItem * pNew )
+void SwFrame::SwClientNotify(const SwModify&, const SfxHint& rHint)
 {
+    auto pLegacy = dynamic_cast<const sw::LegacyModifyHint*>(&rHint);
+    if(!pLegacy)
+        return;
     sal_uInt8 nInvFlags = 0;
 
-    if( pOld && pNew && RES_ATTRSET_CHG == pNew->Which() )
+    if(pLegacy->m_pOld && pLegacy->m_pNew && RES_ATTRSET_CHG == pLegacy->m_pNew->Which())
     {
-        SfxItemIter aNIter( *static_cast<const SwAttrSetChg*>(pNew)->GetChgSet() );
-        SfxItemIter aOIter( *static_cast<const SwAttrSetChg*>(pOld)->GetChgSet() );
+        SfxItemIter aNIter(*static_cast<const SwAttrSetChg*>(pLegacy->m_pNew)->GetChgSet());
+        SfxItemIter aOIter(*static_cast<const SwAttrSetChg*>(pLegacy->m_pOld)->GetChgSet());
         const SfxPoolItem* pNItem = aNIter.GetCurItem();
         const SfxPoolItem* pOItem = aOIter.GetCurItem();
         do
@@ -491,32 +494,32 @@ void SwFrame::Modify( const SfxPoolItem* pOld, const SfxPoolItem * pNew )
         } while (pNItem);
     }
     else
-        UpdateAttrFrame( pOld, pNew, nInvFlags );
+        UpdateAttrFrame(pLegacy->m_pOld, pLegacy->m_pNew, nInvFlags);
 
-    if ( nInvFlags == 0 )
+    if(nInvFlags == 0)
         return;
 
-    SwPageFrame *pPage = FindPageFrame();
-    InvalidatePage( pPage );
-    if ( nInvFlags & 0x01 )
+    SwPageFrame* pPage = FindPageFrame();
+    InvalidatePage(pPage);
+    if(nInvFlags & 0x01)
     {
         InvalidatePrt_();
-        if( !GetPrev() && IsTabFrame() && IsInSct() )
+        if(!GetPrev() && IsTabFrame() && IsInSct())
             FindSctFrame()->InvalidatePrt_();
     }
-    if ( nInvFlags & 0x02 )
+    if(nInvFlags & 0x02)
         InvalidateSize_();
-    if ( nInvFlags & 0x04 )
+    if(nInvFlags & 0x04)
         InvalidatePos_();
-    if ( nInvFlags & 0x08 )
+    if(nInvFlags & 0x08)
         SetCompletePaint();
     SwFrame *pNxt;
-    if ( nInvFlags & 0x30 && nullptr != (pNxt = GetNext()) )
+    if(nInvFlags & 0x30 && nullptr != (pNxt = GetNext()))
     {
-        pNxt->InvalidatePage( pPage );
-        if ( nInvFlags & 0x10 )
+        pNxt->InvalidatePage(pPage);
+        if(nInvFlags & 0x10)
             pNxt->InvalidatePos_();
-        if ( nInvFlags & 0x20 )
+        if(nInvFlags & 0x20)
             pNxt->SetCompletePaint();
     }
 }
@@ -2322,7 +2325,7 @@ SwTwips SwContentFrame::ShrinkFrame( SwTwips nDist, bool bTst, bool bInfo )
     return nReal;
 }
 
-void SwContentFrame::SwClientNotify(const SwModify&, const SfxHint& rHint)
+void SwContentFrame::SwClientNotify(const SwModify& rMod, const SfxHint& rHint)
 {
     auto pLegacy = dynamic_cast<const sw::LegacyModifyHint*>(&rHint);
     if(!pLegacy)
@@ -2345,7 +2348,7 @@ void SwContentFrame::SwClientNotify(const SwModify&, const SfxHint& rHint)
             pOItem = aOIter.NextItem();
         } while(pNItem);
         if(aOldSet.Count() || aNewSet.Count())
-            SwFrame::Modify(&aOldSet, &aNewSet);
+            SwFrame::SwClientNotify(rMod, sw::LegacyModifyHint(&aOldSet, &aNewSet));
     }
     else
         UpdateAttr_(pLegacy->m_pOld, pLegacy->m_pNew, nInvFlags);
@@ -2462,11 +2465,13 @@ void SwContentFrame::UpdateAttr_( const SfxPoolItem* pOld, const SfxPoolItem* pN
         case RES_LR_SPACE:
         case RES_BOX:
         case RES_SHADOW:
-            Prepare( PrepareHint::FixSizeChanged );
-            SwFrame::Modify( pOld, pNew );
-            rInvFlags |= 0x30;
-            break;
-
+            {
+                Prepare( PrepareHint::FixSizeChanged );
+                SwModify aMod;
+                SwFrame::SwClientNotify(aMod, sw::LegacyModifyHint(pOld, pNew));
+                rInvFlags |= 0x30;
+                break;
+            }
         case RES_BREAK:
             {
                 rInvFlags |= 0x42;
@@ -2542,7 +2547,10 @@ void SwContentFrame::UpdateAttr_( const SfxPoolItem* pOld, const SfxPoolItem* pN
                 pNewSet->ClearItem( nWhich );
         }
         else
-            SwFrame::Modify( pOld, pNew );
+        {
+            SwModify aMod;
+            SwFrame::SwClientNotify(aMod, sw::LegacyModifyHint(pOld, pNew));
+        }
     }
 }