accessibility: fix leak of AccessibleListBoxEntry

The problem is that AccessibleListBoxEntry from the Stylist in the
sidebar are not deleted until the document is closed, which causes
slowdowns because it causes an ever-growing list of window listeners.

There are several issues here:

* AccessibleListBoxEntry::m_aParent appears of dubious merit because
  SvTreeList::Move may move entries to a different parent;
  this member may or may not be initialised depending on where
  the instance is created; there are confusing comments mentioning a
  "Solution" but not the corresponding problem; just remove it and let
  it do a dynamic lookup when needed

* AccessibleListBox::m_mapEntry already exists but is used in only half
  the places where AccessibleListBoxEntry are created; use it everywhere
  (consistently create entry without a parent, see previous point), and
  let AccessibleListBoxEntry know the AccessibleListBox instance so it
  can use the m_mapEntry too

* When VclEventId::ListboxItemRemoved event is received, the m_mapEntry
  is cleared but the AccessibleListBoxEntry survive this happily;
  better dispose them
  (seeing as this event is regularly sent because there's some timer
   clearing the Stylist and recreating it from scratch...)

Change-Id: I6c3336e019e873fa7cc8fa03cb8949a1ff2fe8fa
Reviewed-on: https://gerrit.libreoffice.org/79100
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@cib.de>

4 files changed, 74 insertions, 62 deletions

diff --git a/accessibility/inc/extended/accessiblelistbox.hxx b/accessibility/inc/extended/accessiblelistbox.hxx
index cb075f299bb6..70248208ec0a 100644
--- a/accessibility/inc/extended/accessiblelistbox.hxx
+++ b/accessibility/inc/extended/accessiblelistbox.hxx
@@ -75,6 +75,8 @@ namespace accessibility
         AccessibleListBox( SvTreeListBox const & _rListBox,
                            const css::uno::Reference< css::accessibility::XAccessible >& _xParent );
 
+        rtl::Reference<AccessibleListBoxEntry> implGetAccessible(SvTreeListEntry & rEntry);
+
         // XTypeProvider
         DECLARE_XTYPEPROVIDER()
 
@@ -108,7 +110,7 @@ namespace accessibility
 
     private:
 
-        typedef std::map< SvTreeListEntry*, css::uno::Reference< css::accessibility::XAccessible > > MAP_ENTRY;
+        typedef std::map<SvTreeListEntry*, rtl::Reference<AccessibleListBoxEntry>> MAP_ENTRY;
         MAP_ENTRY m_mapEntry;
 
         css::uno::Reference< css::accessibility::XAccessible > m_xFocusedChild;
diff --git a/accessibility/inc/extended/accessiblelistboxentry.hxx b/accessibility/inc/extended/accessiblelistboxentry.hxx
index 2d9e4fac97b1..babaf08a52b5 100644
--- a/accessibility/inc/extended/accessiblelistboxentry.hxx
+++ b/accessibility/inc/extended/accessiblelistboxentry.hxx
@@ -54,7 +54,7 @@ class SvTreeListEntry;
 
 namespace accessibility
 {
-
+    class AccessibleListBox;
 
 // class AccessibleListBoxEntry ------------------------------------------
     typedef ::cppu::WeakAggComponentImplHelper9< css::accessibility::XAccessible
@@ -84,8 +84,8 @@ namespace accessibility
         /// client id in the AccessibleEventNotifier queue
         sal_uInt32                          m_nClientId;
 
-        css::uno::WeakReference< css::accessibility::XAccessible >
-                                            m_aParent;
+        css::uno::WeakReference<css::accessibility::XAccessible> m_wListBox;
+        AccessibleListBox & m_rListBox;
 
         tools::Rectangle               GetBoundingBox_Impl() const;
         tools::Rectangle               GetBoundingBoxOnScreen_Impl() const;
@@ -121,13 +121,14 @@ namespace accessibility
         /** Ctor()
             @param  _rListBox
                 the view control
-            @param  _pEntry
+            @param  rEntry
                 the entry
-            @param  _xParent
-                is our parent accessible object
+            @param rListBox
+                the a11y object for _rListBox
         */
-        AccessibleListBoxEntry( SvTreeListBox& _rListBox, SvTreeListEntry* _pEntry,
-                                const css::uno::Reference< css::accessibility::XAccessible >& _xParent );
+        AccessibleListBoxEntry( SvTreeListBox& _rListBox,
+                                SvTreeListEntry& rEntry,
+                                AccessibleListBox & rListBox);
 
         SvTreeListEntry* GetSvLBoxEntry() const { return m_pSvLBoxEntry; }
 
diff --git a/accessibility/source/extended/accessiblelistbox.cxx b/accessibility/source/extended/accessiblelistbox.cxx
index 28b6dac86621..4887256bf27a 100644
--- a/accessibility/source/extended/accessiblelistbox.cxx
+++ b/accessibility/source/extended/accessiblelistbox.cxx
@@ -137,18 +137,7 @@ namespace accessibility
                             uno::Any aOldValue;
                             aOldValue <<= m_xFocusedChild;
 
-                            MAP_ENTRY::iterator mi = m_mapEntry.find(pEntry);
-                            if(mi != m_mapEntry.end())
-                            {
-                                OSL_ASSERT(mi->second.get() != nullptr);
-                                m_xFocusedChild = mi->second;
-                            }
-                            else
-                            {
-                                AccessibleListBoxEntry *pEntNew = new AccessibleListBoxEntry( *getListBox(), pEntry, nullptr );
-                                m_xFocusedChild = pEntNew;
-                                m_mapEntry.emplace(pEntry,pEntNew);
-                            }
+                            m_xFocusedChild.set(implGetAccessible(*pEntry).get());
 
                             aNewValue <<= m_xFocusedChild;
                             NotifyAccessibleEvent( AccessibleEventId::ACTIVE_DESCENDANT_CHANGED, aOldValue, aNewValue );
@@ -175,9 +164,13 @@ namespace accessibility
                         {
                             uno::Any aNewValue;
                             uno::Any aOldValue;
-                            aOldValue <<= entry.second;
+                            aOldValue <<= uno::Reference<XAccessible>(entry.second.get());
                             NotifyAccessibleEvent( AccessibleEventId::CHILD, aOldValue, aNewValue );
                         }
+                        for (auto const& entry : m_mapEntry)
+                        {   // release references ...
+                            entry.second->dispose();
+                        }
                         m_mapEntry.clear();
                     }
                 }
@@ -190,9 +183,7 @@ namespace accessibility
                     SvTreeListEntry* pEntry = static_cast< SvTreeListEntry* >( rVclWindowEvent.GetData() );
                     if ( pEntry )
                     {
-                        AccessibleListBoxEntry* pAccListBoxEntry =
-                            new AccessibleListBoxEntry( *getListBox(), pEntry, this );
-                        Reference< XAccessible > xChild = pAccListBoxEntry;
+                        Reference<XAccessible> const xChild(implGetAccessible(*pEntry).get());
                         const short nAccEvent =
                                 ( rVclWindowEvent.GetId() == VclEventId::ItemExpanded )
                                 ? AccessibleEventId::LISTBOX_ENTRY_EXPANDED
@@ -222,21 +213,9 @@ namespace accessibility
         AccessibleListBoxEntry* pEntryFocus =static_cast< AccessibleListBoxEntry* >(m_xFocusedChild.get());
         if (pEntryFocus && pEntry && pEntry != pEntryFocus->GetSvLBoxEntry())
         {
-            AccessibleListBoxEntry *pAccCurOptionEntry =nullptr;
-            MAP_ENTRY::iterator mi = m_mapEntry.find(pEntry);
-            if (mi != m_mapEntry.end())
-            {
-                pAccCurOptionEntry= static_cast< AccessibleListBoxEntry* >(mi->second.get());
-            }
-            else
-            {
-                pAccCurOptionEntry =new AccessibleListBoxEntry( *getListBox(), pEntry, nullptr );
-                std::pair<MAP_ENTRY::iterator, bool> pairMi = m_mapEntry.emplace(pAccCurOptionEntry->GetSvLBoxEntry(), pAccCurOptionEntry);
-                mi = pairMi.first;
-            }
-
+            AccessibleListBoxEntry *const pAccCurOptionEntry = implGetAccessible(*pEntry).get();
             uno::Any aNewValue;
-            aNewValue <<= mi->second;//xAcc
+            aNewValue <<= uno::Reference<XAccessible>(pAccCurOptionEntry);
             NotifyAccessibleEvent( AccessibleEventId::CHILD, uno::Any(), aNewValue );//Add
 
             return pAccCurOptionEntry;
@@ -254,7 +233,7 @@ namespace accessibility
         {
             uno::Any aNewValue;
             uno::Any aOldValue;
-            aOldValue <<= mi->second;
+            aOldValue <<= uno::Reference<XAccessible>(mi->second.get());
             NotifyAccessibleEvent( AccessibleEventId::CHILD, aOldValue, aNewValue );
 
             m_mapEntry.erase(mi);
@@ -350,7 +329,8 @@ namespace accessibility
 
         // Solution: Set the parameter of the parent to null to let entry determine the parent by itself
         //return new AccessibleListBoxEntry( *getListBox(), pEntry, this );
-        return new AccessibleListBoxEntry( *getListBox(), pEntry, nullptr );
+        //return new AccessibleListBoxEntry( *getListBox(), pEntry, nullptr );
+        return implGetAccessible(*pEntry).get();
     }
 
     Reference< XAccessible > SAL_CALL AccessibleListBox::getAccessibleParent(  )
@@ -496,7 +476,8 @@ namespace accessibility
             {
                 // Solution: Set the parameter of the parent to null to let entry determine the parent by itself
                 //xChild = new AccessibleListBoxEntry( *getListBox(), pEntry, this );
-                xChild = new AccessibleListBoxEntry( *getListBox(), pEntry, nullptr );
+                //xChild = new AccessibleListBoxEntry( *getListBox(), pEntry, nullptr );
+                xChild = implGetAccessible(*pEntry).get();
                 break;
             }
         }
@@ -527,6 +508,25 @@ namespace accessibility
         }
     }
 
+    rtl::Reference<AccessibleListBoxEntry> AccessibleListBox::implGetAccessible(SvTreeListEntry & rEntry)
+    {
+        rtl::Reference<AccessibleListBoxEntry> pAccessible;
+        auto const it = m_mapEntry.find(&rEntry);
+        if (it != m_mapEntry.end())
+        {
+            pAccessible = it->second;
+        }
+        else
+        {
+            AccessibleListBoxEntry *const pAccListBoxEntry =
+                new AccessibleListBoxEntry(*getListBox(), rEntry, *this);
+            pAccessible = pAccListBoxEntry;
+            m_mapEntry.emplace(&rEntry, pAccessible);
+        }
+        assert(pAccessible.is());
+        return pAccessible;
+    }
+
     VclPtr< SvTreeListBox > AccessibleListBox::getListBox() const
     {
         return GetAs< SvTreeListBox >();
diff --git a/accessibility/source/extended/accessiblelistboxentry.cxx b/accessibility/source/extended/accessiblelistboxentry.cxx
index 1a4bbb02dd5b..460a92a279e1 100644
--- a/accessibility/source/extended/accessiblelistboxentry.cxx
+++ b/accessibility/source/extended/accessiblelistboxentry.cxx
@@ -18,6 +18,7 @@
  */
 
 #include <extended/accessiblelistboxentry.hxx>
+#include <extended/accessiblelistbox.hxx>
 #include <vcl/treelistbox.hxx>
 #include <svtools/stringtransfer.hxx>
 #include <vcl/svlbitm.hxx>
@@ -71,18 +72,17 @@ namespace accessibility
     // Ctor() and Dtor()
 
     AccessibleListBoxEntry::AccessibleListBoxEntry( SvTreeListBox& _rListBox,
-                                                    SvTreeListEntry* _pEntry,
-                                                    const Reference< XAccessible >& _xParent ) :
-
-        AccessibleListBoxEntry_BASE ( m_aMutex ),
-        ListBoxAccessibleBase( _rListBox ),
-
-        m_pSvLBoxEntry  ( _pEntry ),
-        m_nClientId     ( 0 ),
-        m_aParent       ( _xParent )
+                                                    SvTreeListEntry& rEntry,
+                                                    AccessibleListBox & rListBox)
+        : AccessibleListBoxEntry_BASE( m_aMutex )
+        , ListBoxAccessibleBase( _rListBox )
 
+        , m_pSvLBoxEntry(&rEntry)
+        , m_nClientId( 0 )
+        , m_wListBox(&rListBox)
+        , m_rListBox(rListBox)
     {
-        _rListBox.FillEntryPath( _pEntry, m_aEntryPath );
+        _rListBox.FillEntryPath( m_pSvLBoxEntry, m_aEntryPath );
     }
 
     AccessibleListBoxEntry::~AccessibleListBoxEntry()
@@ -246,7 +246,7 @@ namespace accessibility
 
             ListBoxAccessibleBase::disposing();
         }
-        m_aParent.clear();
+        m_wListBox.clear();
     }
 
     // XServiceInfo
@@ -302,13 +302,15 @@ namespace accessibility
         if ( !pEntry )
             throw IndexOutOfBoundsException();
 
-        return new AccessibleListBoxEntry( *getListBox(), pEntry, this );
-    }
+        uno::Reference<XAccessible> xListBox(m_wListBox);
+        assert(xListBox.is());
 
+        return m_rListBox.implGetAccessible(*pEntry).get();
+    }
 
     Reference< XAccessible > AccessibleListBoxEntry::implGetParentAccessible( ) const
     {
-        Reference< XAccessible > xParent(m_aParent);
+        Reference< XAccessible > xParent;
         if ( !xParent.is() )
         {
             OSL_ENSURE( m_aEntryPath.size(), "AccessibleListBoxEntry::getAccessibleParent: invalid path!" );
@@ -332,10 +334,13 @@ namespace accessibility
                 if ( pParentEntry )
                     pParentEntry = getListBox()->GetParent(pParentEntry);
                 if ( pParentEntry )
-                    xParent = new AccessibleListBoxEntry( *getListBox(), pParentEntry, nullptr );
-                    // note that we pass NULL here as parent-accessible:
-                    // this is allowed, as the AccessibleListBoxEntry class will create its parent
+                {
+                    uno::Reference<XAccessible> xListBox(m_wListBox);
+                    assert(xListBox.is());
+                    return m_rListBox.implGetAccessible(*pParentEntry).get();
+                    // the AccessibleListBoxEntry class will create its parent
                     // when needed
+                }
             }
         }
 
@@ -555,10 +560,12 @@ namespace accessibility
             throw RuntimeException();
 
         Reference< XAccessible > xAcc;
-        AccessibleListBoxEntry* pAccEntry = new AccessibleListBoxEntry( *getListBox(), pEntry, this );
+        uno::Reference<XAccessible> xListBox(m_wListBox);
+        assert(xListBox.is());
+        auto pAccEntry = m_rListBox.implGetAccessible(*pEntry);
         tools::Rectangle aRect = pAccEntry->GetBoundingBox_Impl();
         if ( aRect.IsInside( VCLPoint( _aPoint ) ) )
-            xAcc = pAccEntry;
+            xAcc = pAccEntry.get();
         return xAcc;
     }
 
@@ -948,7 +955,9 @@ namespace accessibility
 
             if ( nSelCount == ( nSelectedChildIndex + 1 ) )
             {
-                xChild = new AccessibleListBoxEntry( *getListBox(), pEntry, this );
+                uno::Reference<XAccessible> xListBox(m_wListBox);
+                assert(xListBox.is());
+                xChild = m_rListBox.implGetAccessible(*pEntry).get();
                 break;
             }
         }