fix memleak circular dependency of CElementList and CElement

launching impress leaks 70+k

==1458== 78,741 (152 direct, 78,589 indirect) bytes in 1 blocks are definitely lost in loss record 24,296 of 24,315
==1458==    at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1458==    by 0x4C3895D: rtl_allocateMemory_SYSTEM(unsigned long) (alloc_global.cxx:270)
==1458==    by 0x4C38A64: rtl_allocateMemory (alloc_global.cxx:303)
==1458==    by 0x2DCC0B67: cppu::OWeakObject::operator new(unsigned long) (weak.hxx:85)
==1458==    by 0x2DCCB3D3: DOM::CDocument::getElementsByTagName(rtl::OUString const&) (document.cxx:714)
==1458==    by 0x25DC99D6: SdDrawDocument::InitLayoutVector() (drawdoc.cxx:1008)

because the CElementList owns the CElement via m_pElement and
m_pElement owns the CElementList via the addEventListener.

Use a WeakEventListener pattern to let the CElement own
that helper which itself doesn't own the CElementList but is
owned by it instead, and forwards the events to the CElementList

In order to use that pattern the CElementList must be have a m_refCount
of 1 when the addEventListener is called, i.e. post ctor, so rename
the original CElementList as CElementListImpl and call its registerListener
from a wrapper CElementList

Change-Id: Ibd4f19b619543a4ef580366c69efb61b526696ab

2 files changed, 104 insertions, 11 deletions

diff --git a/unoxml/source/dom/elementlist.cxx b/unoxml/source/dom/elementlist.cxx
index 98d150b2436e..9230917a2266 100644
--- a/unoxml/source/dom/elementlist.cxx
+++ b/unoxml/source/dom/elementlist.cxx
@@ -28,6 +28,34 @@ using namespace css::uno;
 using namespace css::xml::dom;
 using namespace css::xml::dom::events;
 
+namespace
+{
+    class WeakEventListener : public ::cppu::WeakImplHelper1<css::xml::dom::events::XEventListener>
+    {
+    private:
+        css::uno::WeakReference<css::xml::dom::events::XEventListener> mxOwner;
+
+    public:
+        WeakEventListener(const css::uno::Reference<css::xml::dom::events::XEventListener>& rOwner)
+            : mxOwner(rOwner)
+        {
+        }
+
+        virtual ~WeakEventListener()
+        {
+        }
+
+        virtual void SAL_CALL handleEvent(const css::uno::Reference<css::xml::dom::events::XEvent>& rEvent)
+            throw(css::uno::RuntimeException, std::exception) SAL_OVERRIDE
+        {
+            css::uno::Reference<css::xml::dom::events::XEventListener> xOwner(mxOwner.get(),
+                css::uno::UNO_QUERY);
+            if (xOwner.is())
+                xOwner->handleEvent(rEvent);
+        }
+    };
+}
+
 namespace DOM
 {
 
@@ -43,25 +71,46 @@ namespace DOM
     CElementList::CElementList(::rtl::Reference<CElement> const& pElement,
             ::osl::Mutex & rMutex,
             OUString const& rName, OUString const*const pURI)
+        : m_xImpl(new CElementListImpl(pElement, rMutex, rName, pURI))
+    {
+        if (pElement.is()) {
+            m_xImpl->registerListener(*pElement);
+        }
+    }
+
+    CElementListImpl::CElementListImpl(::rtl::Reference<CElement> const& pElement,
+            ::osl::Mutex & rMutex,
+            OUString const& rName, OUString const*const pURI)
         : m_pElement(pElement)
         , m_rMutex(rMutex)
         , m_pName(lcl_initXmlString(rName))
         , m_pURI((pURI) ? lcl_initXmlString(*pURI) : 0)
         , m_bRebuild(true)
     {
-        if (m_pElement.is()) {
-            registerListener(*m_pElement);
+    }
+
+    CElementListImpl::~CElementListImpl()
+    {
+        if (m_xEventListener.is() && m_pElement.is())
+        {
+            Reference< XEventTarget > xTarget(static_cast<XElement*>(m_pElement.get()), UNO_QUERY);
+            assert(xTarget.is());
+            if (!xTarget.is())
+                return;
+            bool capture = false;
+            xTarget->removeEventListener("DOMSubtreeModified", m_xEventListener, capture);
         }
     }
 
-    void CElementList::registerListener(CElement & rElement)
+    void CElementListImpl::registerListener(CElement & rElement)
     {
         try {
             Reference< XEventTarget > const xTarget(
                     static_cast<XElement*>(& rElement), UNO_QUERY_THROW);
             bool capture = false;
+            m_xEventListener = new WeakEventListener(this);
             xTarget->addEventListener("DOMSubtreeModified",
-                    Reference< XEventListener >(this), capture);
+                    m_xEventListener, capture);
         } catch (const Exception &e){
             OString aMsg("Exception caught while registering NodeList as listener:\n");
             aMsg += OUStringToOString(e.Message, RTL_TEXTENCODING_ASCII_US);
@@ -69,7 +118,7 @@ namespace DOM
         }
     }
 
-    void CElementList::buildlist(xmlNodePtr pNode, bool start)
+    void CElementListImpl::buildlist(xmlNodePtr pNode, bool start)
     {
         // bail out if no rebuild is needed
         if (start) {
@@ -107,7 +156,7 @@ namespace DOM
     /**
     The number of nodes in the list.
     */
-    sal_Int32 SAL_CALL CElementList::getLength() throw (RuntimeException, std::exception)
+    sal_Int32 SAL_CALL CElementListImpl::getLength() throw (RuntimeException, std::exception)
     {
         ::osl::MutexGuard const g(m_rMutex);
 
@@ -120,7 +169,7 @@ namespace DOM
     /**
     Returns the indexth item in the collection.
     */
-    Reference< XNode > SAL_CALL CElementList::item(sal_Int32 index)
+    Reference< XNode > SAL_CALL CElementListImpl::item(sal_Int32 index)
         throw (RuntimeException, std::exception)
     {
         if (index < 0) throw RuntimeException();
@@ -139,7 +188,7 @@ namespace DOM
     }
 
     // tree mutations can change the list
-    void SAL_CALL CElementList::handleEvent(Reference< XEvent > const&)
+    void SAL_CALL CElementListImpl::handleEvent(Reference< XEvent > const&)
         throw (RuntimeException, std::exception)
     {
         ::osl::MutexGuard const g(m_rMutex);
diff --git a/unoxml/source/dom/elementlist.hxx b/unoxml/source/dom/elementlist.hxx
index 3cde5bce6fef..18289e6d111d 100644
--- a/unoxml/source/dom/elementlist.hxx
+++ b/unoxml/source/dom/elementlist.hxx
@@ -36,6 +36,7 @@
 #include <com/sun/star/xml/dom/events/XEvent.hpp>
 #include <com/sun/star/xml/dom/events/XEventListener.hpp>
 
+#include <cppuhelper/implbase1.hxx>
 #include <cppuhelper/implbase2.hxx>
 
 namespace DOM
@@ -44,11 +45,16 @@ namespace DOM
 
     typedef std::vector< xmlNodePtr > nodevector_t;
 
-    class CElementList
+    class CElementListImpl
         : public cppu::WeakImplHelper2< css::xml::dom::XNodeList,
                 css::xml::dom::events::XEventListener >
     {
     private:
+        /** @short  proxy weak binding to forward Events to ourself without
+                    an ownership cycle
+          */
+        css::uno::Reference< css::xml::dom::events::XEventListener > m_xEventListener;
+
         ::rtl::Reference<CElement> const m_pElement;
         ::osl::Mutex & m_rMutex;
         ::boost::scoped_array<xmlChar> const m_pName;
@@ -57,13 +63,16 @@ namespace DOM
         nodevector_t m_nodevector;
 
         void buildlist(xmlNodePtr pNode, bool start=true);
-        void registerListener(CElement & rElement);
 
     public:
-        CElementList(::rtl::Reference<CElement> const& pElement,
+        CElementListImpl(::rtl::Reference<CElement> const& pElement,
                 ::osl::Mutex & rMutex,
                 OUString const& rName, OUString const*const pURI = 0);
 
+        void registerListener(CElement & rElement);
+
+        ~CElementListImpl();
+
         /**
         The number of nodes in the list.
         */
@@ -78,6 +87,41 @@ namespace DOM
         virtual void SAL_CALL handleEvent(const css::uno::Reference< css::xml::dom::events::XEvent >& evt)
             throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE;
     };
+
+    class CElementList
+        : public cppu::WeakImplHelper2< css::xml::dom::XNodeList,
+                css::xml::dom::events::XEventListener >
+    {
+    private:
+        rtl::Reference<CElementListImpl> m_xImpl;
+    public:
+        CElementList(::rtl::Reference<CElement> const& pElement,
+                ::osl::Mutex & rMutex,
+                OUString const& rName, OUString const*const pURI = 0);
+
+        /**
+        The number of nodes in the list.
+        */
+        virtual sal_Int32 SAL_CALL getLength() throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE
+        {
+            return m_xImpl->getLength();
+        }
+        /**
+        Returns the indexth item in the collection.
+        */
+        virtual css::uno::Reference< css::xml::dom::XNode > SAL_CALL item(sal_Int32 index)
+            throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE
+        {
+            return m_xImpl->item(index);
+        }
+
+        // XEventListener
+        virtual void SAL_CALL handleEvent(const css::uno::Reference< css::xml::dom::events::XEvent >& evt)
+            throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE
+        {
+            m_xImpl->handleEvent(evt);
+        }
+    };
 }
 
 #endif