Revert "replace SwClient by SvtListener in SwGrammarContact"

This reverts commit dcae40491eea2534c30ba96f10bc45035d073a46, it causes
heap-use-after-free during JunitTest_sw_unoapi_1 (see also
<https://ci.libreoffice.org/job/lo_ubsan/1913/> and following),

> ==1281072==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150043ea120 at pc 0x7fd7ddcc4afe bp 0x7fd6603350b0 sp 0x7fd6603350a8
> READ of size 8 at 0x6150043ea120 thread T116 (cppu_threadpool)
>  #0 in SwTextNode::SetGrammarCheck(SwGrammarMarkUp*, bool) at sw/source/core/txtnode/txtedt.cxx:2194:10
>  #1 in (anonymous namespace)::SwGrammarContact::updateCursorPosition(SwPosition const&) at sw/source/core/txtnode/SwGrammarContact.cxx:94:26
>  #2 in SwCursorShell::UpdateCursorPos() at sw/source/core/crsr/crsrsh.cxx:1508:26
>  #3 in SwCursorShell::UpdateCursor(unsigned short, bool) at sw/source/core/crsr/crsrsh.cxx:1815:5
>  #4 in SwCursorShell::EndAction(bool, bool) at sw/source/core/crsr/crsrsh.cxx:283:5
>  #5 in SwRootFrame::EndAllAction(bool) at sw/source/core/layout/pagechg.cxx:1913:27
>  #6 in UnoActionContext::~UnoActionContext() at sw/source/core/unocore/unoobj2.cxx:202:25
>  #7 in SwXTextCursor::DeleteAndInsert(rtl::OUString const&, bool) at sw/source/core/unocore/unoobj.cxx:759:1
>  #8 in SwXTextCursor::setString(rtl::OUString const&) at sw/source/core/unocore/unoobj.cxx:1721:5
>  #9 in SwXText::setString(rtl::OUString const&) at sw/source/core/unocore/unotext.cxx:968:11
>  #10 in gcc3::callVirtualMethod(void*, unsigned int, void*, _typelib_TypeDescriptionReference*, bool, unsigned long*, unsigned int, unsigned long*, double*) at bridges/source/cpp_uno/gcc3_linux_x86-64/callvirtualmethod.cxx:77:5
>  #11 in cpp_call(bridges::cpp_uno::shared::UnoInterfaceProxy*, bridges::cpp_uno::shared::VtableSlot, _typelib_TypeDescriptionReference*, int, _typelib_MethodParameter*, void*, void**, _uno_Any**) at bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:233:13
>  #12 in unoInterfaceProxyDispatch at bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:413:13
>  #13 in binaryurp::IncomingRequest::execute_throw(binaryurp::BinaryAny*, std::__debug::vector<binaryurp::BinaryAny, std::allocator<binaryurp::BinaryAny> >*) const at binaryurp/source/incomingrequest.cxx:235:13
>  #14 in binaryurp::IncomingRequest::execute() const at binaryurp/source/incomingrequest.cxx:78:26
>  #15 in request at binaryurp/source/reader.cxx:85:9
>  #16 in cppu_threadpool::JobQueue::enter(void const*, bool) at cppu/source/threadpool/jobqueue.cxx:100:17
>  #17 in cppu_threadpool::ORequestThread::run() at cppu/source/threadpool/thread.cxx:165:31
>  #18 in threadFunc at include/osl/thread.hxx:189:15
>  #19 in osl_thread_start_Impl(void*) at sal/osl/unx/thread.cxx:264:9
>
> 0x6150043ea120 is located 416 bytes inside of 464-byte region [0x6150043e9f80,0x6150043ea150)
> freed by thread T116 (cppu_threadpool) here:
>  #0 in operator delete(void*, unsigned long) at ~/github.com/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:172:3
>  #1 in SwTextNode::~SwTextNode() at sw/source/core/txtnode/ndtxt.cxx:231:1
>  #2 in SwNodes::RemoveNode(unsigned long, unsigned long, bool) at sw/source/core/docnode/nodes.cxx:2274:13
>  #3 in SwNodes::Delete(SwNodeIndex const&, unsigned long) at sw/source/core/docnode/nodes.cxx:1221:9
>  #4 in SwTextNode::JoinNext() at sw/source/core/txtnode/ndtxt.cxx:1027:14
>  #5 in sw_JoinText(SwPaM&, bool) at sw/source/core/doc/docedt.cxx:489:22
>  #6 in sw::DocumentContentOperationsManager::DeleteAndJoinImpl(SwPaM&, bool) at sw/source/core/doc/DocumentContentOperationsManager.cxx:4043:9
>  #7 in (anonymous namespace)::lcl_DoWithBreaks(sw::DocumentContentOperationsManager&, SwPaM&, bool (sw::DocumentContentOperationsManager::*)(SwPaM&, bool), bool) at sw/source/core/doc/DocumentContentOperationsManager.cxx:630:20
>  #8 in sw::DocumentContentOperationsManager::DeleteAndJoin(SwPaM&, bool) at sw/source/core/doc/DocumentContentOperationsManager.cxx:2184:22
>  #9 in SwXTextCursor::DeleteAndInsert(rtl::OUString const&, bool) at sw/source/core/unocore/unoobj.cxx:744:50
>  #10 in SwXTextCursor::setString(rtl::OUString const&) at sw/source/core/unocore/unoobj.cxx:1721:5
>  #11 in SwXText::setString(rtl::OUString const&) at sw/source/core/unocore/unotext.cxx:968:11
>  #12 in gcc3::callVirtualMethod(void*, unsigned int, void*, _typelib_TypeDescriptionReference*, bool, unsigned long*, unsigned int, unsigned long*, double*) at bridges/source/cpp_uno/gcc3_linux_x86-64/callvirtualmethod.cxx:77:5
>  #13 in cpp_call(bridges::cpp_uno::shared::UnoInterfaceProxy*, bridges::cpp_uno::shared::VtableSlot, _typelib_TypeDescriptionReference*, int, _typelib_MethodParameter*, void*, void**, _uno_Any**) at bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:233:13
>  #14 in unoInterfaceProxyDispatch at bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:413:13
>  #15 in binaryurp::IncomingRequest::execute_throw(binaryurp::BinaryAny*, std::__debug::vector<binaryurp::BinaryAny, std::allocator<binaryurp::BinaryAny> >*) const at binaryurp/source/incomingrequest.cxx:235:13
>  #16 in binaryurp::IncomingRequest::execute() const at binaryurp/source/incomingrequest.cxx:78:26
>  #17 in request at binaryurp/source/reader.cxx:85:9
>  #18 in cppu_threadpool::JobQueue::enter(void const*, bool) at cppu/source/threadpool/jobqueue.cxx:100:17
>  #19 in cppu_threadpool::ORequestThread::run() at cppu/source/threadpool/thread.cxx:165:31
>
> previously allocated by thread T116 (cppu_threadpool) here:
>  #0 in operator new(unsigned long) at ~/github.com/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:99:3
>  #1 in SwDoc::SwDoc() at sw/source/core/doc/docnew.cxx:329:5
>  #2 in SwDocFac::GetDoc() at sw/source/filter/basflt/docfact.cxx:36:21
>  #3 in SwDocShell::AddLink() at sw/source/uibase/app/docshini.cxx:396:28
>  #4 in SwDocShell::InitNew(com::sun::star::uno::Reference<com::sun::star::embed::XStorage> const&) at sw/source/uibase/app/docshini.cxx:96:9
>  #5 in SfxObjectShell::DoInitNew(SfxMedium*) at sfx2/source/doc/objstor.cxx:466:10
>  #6 in SfxBaseModel::initNew() at sfx2/source/doc/sfxbasemodel.cxx:1802:42
>  #7 in (anonymous namespace)::SfxFrameLoader_Impl::load(com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&) at sfx2/source/view/frmload.cxx:674:28
>  #8 in framework::LoadEnv::impl_loadContent() at framework/source/loadenv/loadenv.cxx:1163:37
>  #9 in framework::LoadEnv::start() at framework/source/loadenv/loadenv.cxx:394:20
>  #10 in framework::LoadEnv::startLoading(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, rtl::OUString const&, int, LoadEnvFeatures) at framework/source/loadenv/loadenv.cxx:299:5
>  #11 in framework::LoadEnv::loadComponentFromURL(com::sun::star::uno::Reference<com::sun::star::frame::XComponentLoader> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) at framework/source/loadenv/loadenv.cxx:168:14
>  #12 in framework::Desktop::loadComponentFromURL(rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) at framework/source/services/desktop.cxx:593:12
>  #13 in non-virtual thunk to framework::Desktop::loadComponentFromURL(rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) at framework/source/services/desktop.cxx
>  #14 in gcc3::callVirtualMethod(void*, unsigned int, void*, _typelib_TypeDescriptionReference*, bool, unsigned long*, unsigned int, unsigned long*, double*) at bridges/source/cpp_uno/gcc3_linux_x86-64/callvirtualmethod.cxx:77:5
>  #15 in cpp_call(bridges::cpp_uno::shared::UnoInterfaceProxy*, bridges::cpp_uno::shared::VtableSlot, _typelib_TypeDescriptionReference*, int, _typelib_MethodParameter*, void*, void**, _uno_Any**) at bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:233:13
>  #16 in unoInterfaceProxyDispatch at bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:413:13
>  #17 in binaryurp::IncomingRequest::execute_throw(binaryurp::BinaryAny*, std::__debug::vector<binaryurp::BinaryAny, std::allocator<binaryurp::BinaryAny> >*) const at binaryurp/source/incomingrequest.cxx:235:13
>  #18 in binaryurp::IncomingRequest::execute() const at binaryurp/source/incomingrequest.cxx:78:26
>  #19 in request at binaryurp/source/reader.cxx:85:9

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

1 files changed, 43 insertions, 46 deletions

diff --git a/sw/source/core/txtnode/SwGrammarContact.cxx b/sw/source/core/txtnode/SwGrammarContact.cxx
index e296cc7375e6..7b061bdaf9ba 100644
--- a/sw/source/core/txtnode/SwGrammarContact.cxx
+++ b/sw/source/core/txtnode/SwGrammarContact.cxx
@@ -23,7 +23,6 @@
 #include <ndtxt.hxx>
 #include <SwGrammarMarkUp.hxx>
 #include <txtfrm.hxx>
-#include <svl/listener.hxx>
 
 namespace {
 
@@ -38,32 +37,34 @@ namespace {
  * will replace the old list. If the grammar checker has completed the paragraph ('setChecked')
  * then a timer is setup which replaces the old list as well.
  */
-class SwGrammarContact : public IGrammarContact, public SvtListener
+class SwGrammarContact : public IGrammarContact, public SwClient
 {
-    Timer m_aTimer;
-    std::unique_ptr<SwGrammarMarkUp> m_pProxyList;
-    bool m_isFinished;
-    SwTextNode* m_pTextNode;
-    DECL_LINK( TimerRepaint, Timer *, void );
+    Timer aTimer;
+    std::unique_ptr<SwGrammarMarkUp> mpProxyList;
+    bool mbFinished;
+    SwTextNode* getMyTextNode() { return static_cast<SwTextNode*>(GetRegisteredIn()); }
+      DECL_LINK( TimerRepaint, Timer *, void );
 
 public:
     SwGrammarContact();
-    virtual ~SwGrammarContact() override { m_aTimer.Stop(); }
+    virtual ~SwGrammarContact() override { aTimer.Stop(); }
 
     // (pure) virtual functions of IGrammarContact
     virtual void updateCursorPosition( const SwPosition& rNewPos ) override;
     virtual SwGrammarMarkUp* getGrammarCheck( SwTextNode& rTextNode, bool bCreate ) override;
     virtual void finishGrammarCheck( SwTextNode& rTextNode ) override;
-    virtual void Notify( const SfxHint& rHint) override;
+protected:
+    // virtual function of SwClient
+    virtual void SwClientNotify( const SwModify&, const SfxHint& rHint) override;
 };
 
 }
 
-SwGrammarContact::SwGrammarContact() : m_isFinished( false ), m_pTextNode(nullptr)
+SwGrammarContact::SwGrammarContact() : mbFinished( false )
 {
-    m_aTimer.SetTimeout( 2000 );  // Repaint of grammar check after 'setChecked'
-    m_aTimer.SetInvokeHandler( LINK(this, SwGrammarContact, TimerRepaint) );
-    m_aTimer.SetDebugName( "sw::SwGrammarContact TimerRepaint" );
+    aTimer.SetTimeout( 2000 );  // Repaint of grammar check after 'setChecked'
+    aTimer.SetInvokeHandler( LINK(this, SwGrammarContact, TimerRepaint) );
+    aTimer.SetDebugName( "sw::SwGrammarContact TimerRepaint" );
 }
 
 IMPL_LINK( SwGrammarContact, TimerRepaint, Timer *, pTimer, void )
@@ -71,10 +72,10 @@ IMPL_LINK( SwGrammarContact, TimerRepaint, Timer *, pTimer, void )
     if( pTimer )
     {
         pTimer->Stop();
-        if( m_pTextNode )
+        if( GetRegisteredIn() )
         {   //Replace the old wrong list by the proxy list and repaint all frames
-            m_pTextNode->SetGrammarCheck( m_pProxyList.release() );
-            SwTextFrame::repaintTextFrames( *m_pTextNode );
+            getMyTextNode()->SetGrammarCheck( mpProxyList.release() );
+            SwTextFrame::repaintTextFrames( *getMyTextNode() );
         }
     }
 }
@@ -83,51 +84,48 @@ IMPL_LINK( SwGrammarContact, TimerRepaint, Timer *, pTimer, void )
 void SwGrammarContact::updateCursorPosition( const SwPosition& rNewPos )
 {
     SwTextNode* pTextNode = rNewPos.nNode.GetNode().GetTextNode();
-    if( pTextNode == m_pTextNode ) // paragraph has been changed
+    if( pTextNode == GetRegisteredIn() ) // paragraph has been changed
         return;
 
-    m_aTimer.Stop();
-    if( m_pTextNode ) // My last paragraph has been left
+    aTimer.Stop();
+    if( GetRegisteredIn() ) // My last paragraph has been left
     {
-        if( m_pProxyList )
+        if( mpProxyList )
         {   // replace old list by the proxy list and repaint
-            m_pTextNode->SetGrammarCheck( m_pProxyList.release() );
-            SwTextFrame::repaintTextFrames( *m_pTextNode );
+            getMyTextNode()->SetGrammarCheck( mpProxyList.release() );
+            SwTextFrame::repaintTextFrames( *getMyTextNode() );
         }
         EndListeningAll();
     }
     if( pTextNode )
-    {
-        m_pTextNode = pTextNode;
-        StartListening(pTextNode->GetNotifier()); // welcome new paragraph
-    }
+        pTextNode->Add( this ); // welcome new paragraph
 }
 
 /* deliver a grammar check list for the given text node */
 SwGrammarMarkUp* SwGrammarContact::getGrammarCheck( SwTextNode& rTextNode, bool bCreate )
 {
     SwGrammarMarkUp *pRet = nullptr;
-    if( m_pTextNode == &rTextNode ) // hey, that's my current paragraph!
+    if( GetRegisteredIn() == &rTextNode ) // hey, that's my current paragraph!
     {   // so you will get a proxy list...
         if( bCreate )
         {
-            if( m_isFinished )
+            if( mbFinished )
             {
-                m_pProxyList.reset();
+                mpProxyList.reset();
             }
-            if( !m_pProxyList )
+            if( !mpProxyList )
             {
                 if( rTextNode.GetGrammarCheck() )
-                    m_pProxyList.reset( static_cast<SwGrammarMarkUp*>(rTextNode.GetGrammarCheck()->Clone()) );
+                    mpProxyList.reset( static_cast<SwGrammarMarkUp*>(rTextNode.GetGrammarCheck()->Clone()) );
                 else
                 {
-                    m_pProxyList.reset( new SwGrammarMarkUp() );
-                    m_pProxyList->SetInvalid( 0, COMPLETE_STRING );
+                    mpProxyList.reset( new SwGrammarMarkUp() );
+                    mpProxyList->SetInvalid( 0, COMPLETE_STRING );
                 }
             }
-            m_isFinished = false;
+            mbFinished = false;
         }
-        pRet = m_pProxyList.get();
+        pRet = mpProxyList.get();
     }
     else
     {
@@ -143,32 +141,31 @@ SwGrammarMarkUp* SwGrammarContact::getGrammarCheck( SwTextNode& rTextNode, bool
     return pRet;
 }
 
-void SwGrammarContact::Notify(const SfxHint& rHint)
+void SwGrammarContact::SwClientNotify(const SwModify&, const SfxHint& rHint)
 {
     auto pLegacy = dynamic_cast<const sw::LegacyModifyHint*>(&rHint);
     if(!pLegacy || pLegacy->GetWhich() != RES_OBJECTDYING)
         return;
-    m_aTimer.Stop();
+    aTimer.Stop();
     EndListeningAll();
-    m_pTextNode = nullptr;
-    m_pProxyList.reset();
+    mpProxyList.reset();
 }
 
 void SwGrammarContact::finishGrammarCheck( SwTextNode& rTextNode )
 {
-    if( &rTextNode != m_pTextNode ) // not my paragraph
+    if( &rTextNode != GetRegisteredIn() ) // not my paragraph
         SwTextFrame::repaintTextFrames( rTextNode ); // can be repainted directly
     else
     {
-        if( m_pProxyList )
+        if( mpProxyList )
         {
-            m_isFinished = true;
-            m_aTimer.Start(); // will replace old list and repaint with delay
+            mbFinished = true;
+            aTimer.Start(); // will replace old list and repaint with delay
         }
-        else if( m_pTextNode->GetGrammarCheck() )
+        else if( getMyTextNode()->GetGrammarCheck() )
         {   // all grammar problems seems to be gone, no delay needed
-            m_pTextNode->SetGrammarCheck( nullptr );
-            SwTextFrame::repaintTextFrames( *m_pTextNode );
+            getMyTextNode()->SetGrammarCheck( nullptr );
+            SwTextFrame::repaintTextFrames( *getMyTextNode() );
         }
     }
 }