Remove SwModify/SwClient for UNO Bookmarks

- introduce sw::BroadcastingModify for migration
  * still receives .SwModify signals
  * broadcasts those to old SwClients and new SvtListeners
  * add tests
- remove SwClient/SwModify for UNO bookmarks

Change-Id: Icefca478472820b93f4bb59e670e59b60eddfe2b
Reviewed-on: https://gerrit.libreoffice.org/65806
Tested-by: Jenkins
Reviewed-by: Björn Michaelsen <bjoern.michaelsen@libreoffice.org>

sw/inc/IMark.hxx

@@ -36,9 +36,8 @@ namespace sw { namespace mark
     };
 
     class SW_DLLPUBLIC IMark
-        : virtual public SwModify // inherited as interface
+        : virtual public sw::BroadcastingModify // inherited as interface
         , public ::boost::totally_ordered<IMark>
-        , virtual public sw::BroadcasterMixin
     {
         protected:
             IMark() = default;

sw/inc/calbck.hxx

@@ -202,7 +202,7 @@ public:
         { CallSwClientNotify( sw::LegacyModifyHint{ pOldValue, pNewValue } ); };
 
     // a more universal broadcasting mechanism
-    inline void CallSwClientNotify( const SfxHint& rHint ) const;
+    virtual void CallSwClientNotify( const SfxHint& rHint ) const;
 
     virtual ~SwModify() override;
 
@@ -230,6 +230,15 @@ template<typename TElementType, typename TSource, sw::IteratorMode eMode> class
 
 namespace sw
 {
+    // this class is part of the migration: it still forwards the "old"
+    // SwModify events and announces them both to the old SwClients still
+    // registered and also to the new SvtListeners.
+    // Still: in the long run the SwClient/SwModify interface should not be
+    // used anymore, in which case a BroadcasterMixin should be enough instead
+    // then.
+    class SW_DLLPUBLIC BroadcastingModify : public SwModify, public BroadcasterMixin {
+        virtual void CallSwClientNotify(const SfxHint& rHint) const override;
+    };
     // this should be hidden but sadly SwIterator template needs it...
     class ListenerEntry final : public SwClient
     {
@@ -438,12 +447,6 @@ SwClient::SwClient( SwModify* pToRegisterIn )
         pToRegisterIn->Add(this);
 }
 
-void SwModify::CallSwClientNotify( const SfxHint& rHint ) const
-{
-    SwIterator<SwClient,SwModify> aIter(*this);
-    for(SwClient* pClient = aIter.First(); pClient; pClient = aIter.Next())
-        pClient->SwClientNotify( *this, rHint );
-}
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sw/qa/core/uwriter.cxx

@@ -120,6 +120,7 @@ public:
     void testFormulas();
     void testIntrusiveRing();
     void testClientModify();
+    void testBroadcastingModify();
     void testWriterMultiListener();
     void test64kPageDescs();
     void testTdf92308();
@@ -157,6 +158,7 @@ public:
     CPPUNIT_TEST(testFormulas);
     CPPUNIT_TEST(testIntrusiveRing);
     CPPUNIT_TEST(testClientModify);
+    CPPUNIT_TEST(testBroadcastingModify);
     CPPUNIT_TEST(testWriterMultiListener);
     CPPUNIT_TEST(test64kPageDescs);
     CPPUNIT_TEST(testTdf92308);
@@ -1844,6 +1846,15 @@ namespace
         virtual void Modify( const SfxPoolItem*, const SfxPoolItem*) override
             { ++m_nModifyCount; }
     };
+    struct TestListener : SvtListener
+    {
+        int m_nNotifyCount;
+        TestListener() : m_nNotifyCount(0) {};
+        virtual void Notify( const SfxHint& ) override
+        {
+            ++m_nNotifyCount;
+        }
+    };
 }
 void SwDocTest::testClientModify()
 {
@@ -1945,6 +1956,20 @@ void SwDocTest::testClientModify()
     CPPUNIT_ASSERT_EQUAL(1,aClient1.m_nNotifyCount);
     CPPUNIT_ASSERT_EQUAL(1,aClient2.m_nNotifyCount);
 }
+void SwDocTest::testBroadcastingModify()
+{
+    sw::BroadcastingModify aMod;
+    TestClient aClient;
+    TestListener aListener;
+
+    aMod.Add(&aClient);
+    aListener.StartListening(aMod.GetNotifier());
+
+    aMod.ModifyBroadcast(nullptr, nullptr);
+    CPPUNIT_ASSERT_EQUAL(1,aClient.m_nModifyCount);
+    CPPUNIT_ASSERT_EQUAL(1,aClient.m_nModifyCount);
+    CPPUNIT_ASSERT_EQUAL(1,aListener.m_nNotifyCount);
+}
 void SwDocTest::testWriterMultiListener()
 {
     TestModify aMod;

sw/source/core/attr/calbck.cxx

@@ -365,4 +365,17 @@ void sw::WriterMultiListener::EndListeningAll()
 }
 
 sw::ClientIteratorBase* sw::ClientIteratorBase::s_pClientIters = nullptr;
+
+void SwModify::CallSwClientNotify( const SfxHint& rHint ) const
+{
+    SwIterator<SwClient,SwModify> aIter(*this);
+    for(SwClient* pClient = aIter.First(); pClient; pClient = aIter.Next())
+        pClient->SwClientNotify( *this, rHint );
+}
+
+void sw::BroadcastingModify::CallSwClientNotify(const SfxHint& rHint) const
+{
+    SwModify::CallSwClientNotify(rHint);
+    const_cast<BroadcastingModify*>(this)->GetNotifier().Broadcast(rHint);
+}
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sw/source/core/crsr/bookmrk.cxx

@@ -27,6 +27,7 @@
 #include <ndtxt.hxx>
 #include <pam.hxx>
 #include <swserv.hxx>
+#include <svl/listener.hxx>
 #include <sfx2/linkmgr.hxx>
 #include <swtypes.hxx>
 #include <UndoBookmark.hxx>
@@ -150,8 +151,7 @@ namespace sw { namespace mark
 {
     MarkBase::MarkBase(const SwPaM& aPaM,
         const OUString& rName)
-        : SwModify(nullptr)
-        , m_pPos1(new SwPosition(*(aPaM.GetPoint())))
+        : m_pPos1(new SwPosition(*(aPaM.GetPoint())))
         , m_aName(rName)
     {
         m_pPos1->nContent.SetMark(this);

sw/source/core/inc/unobookmark.hxx

@@ -29,6 +29,7 @@
 
 #include <cppuhelper/implbase.hxx>
 
+#include <svl/listener.hxx>
 #include <sfx2/Metadatable.hxx>
 
 #include <unobaseclass.hxx>
@@ -144,12 +145,17 @@ public:
 
 class SwXFieldmarkParameters
     : public ::cppu::WeakImplHelper< css::container::XNameContainer>
-    , private SwClient
+    , public SvtListener
 {
+    private:
+        ::sw::mark::IFieldmark* m_pFieldmark;
+        /// @throws css::uno::RuntimeException
+        ::sw::mark::IFieldmark::parameter_map_t* getCoreParameters();
     public:
         SwXFieldmarkParameters(::sw::mark::IFieldmark* const pFieldmark)
+            : m_pFieldmark(pFieldmark)
         {
-            pFieldmark->Add(this);
+            StartListening(pFieldmark->GetNotifier());
         }
 
         // XNameContainer
@@ -164,12 +170,8 @@ class SwXFieldmarkParameters
         // XElementAccess
         virtual css::uno::Type SAL_CALL getElementType(  ) override;
         virtual sal_Bool SAL_CALL hasElements(  ) override;
-    protected:
-        //SwClient
-    virtual void Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew ) override;
-    private:
-        /// @throws css::uno::RuntimeException
-        ::sw::mark::IFieldmark::parameter_map_t* getCoreParameters();
+
+        virtual void Notify( const SfxHint& rHint ) override;
 };
 
 typedef cppu::ImplInheritanceHelper< SwXBookmark,

sw/source/core/unocore/unobkm.cxx

@@ -584,17 +584,17 @@ sal_Bool SwXFieldmarkParameters::hasElements()
     return !getCoreParameters()->empty();
 }
 
-void SwXFieldmarkParameters::Modify(const SfxPoolItem *pOld, const SfxPoolItem *pNew)
+void SwXFieldmarkParameters::Notify(const SfxHint& rHint)
 {
-    ClientModify(this, pOld, pNew);
+    if(rHint.GetId() == SfxHintId::Dying)
+        m_pFieldmark = nullptr;
 }
 
 IFieldmark::parameter_map_t* SwXFieldmarkParameters::getCoreParameters()
 {
-    const IFieldmark* pFieldmark = dynamic_cast< const IFieldmark* >(GetRegisteredIn());
-    if(!pFieldmark)
+    if(!m_pFieldmark)
         throw uno::RuntimeException();
-    return const_cast< IFieldmark* >(pFieldmark)->GetParameters();
+    return m_pFieldmark->GetParameters();
 }
 
 void SwXFieldmark::attachToRange( const uno::Reference < text::XTextRange >& xTextRange )