a11y crash on deleting certain frame in certain document

I've an internal RH document which crashes in a11y when a paragraph with a
graphic and a drawing frame in it is deleted.

The SdrObject is removed and deleted, but when
SwAccessibleContext::DisposeChild is called the object does not pass IsShowing
so its not removed from the accessibility map. Leaving an entry in the map
pointing to a deleted SdrObject

So here take the route-one approach of always removing from the map
accessibility children which depend on a SdrObject which is getting deleted,
whether or not it is inside the visible area at the moment.

The real change here is to SwAccessibleContext::DisposeChild and to
SwFrame::RemoveDrawObj

Change-Id: I764cd54d6216d233756f52b5be66c80737b5e38d
Reviewed-on: https://gerrit.libreoffice.org/26824
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Michael Stahl <mstahl@redhat.com>

18 files changed, 54 insertions, 49 deletions

diff --git a/sw/inc/accmap.hxx b/sw/inc/accmap.hxx
index 7c1f2299b805..25c96967e6dd 100644
--- a/sw/inc/accmap.hxx
+++ b/sw/inc/accmap.hxx
@@ -185,7 +185,8 @@ public:
     void A11yDispose( const SwFrame* pFrame,
                       const SdrObject* pObj,
                       vcl::Window* pWindow,
-                      bool bRecursive = false );
+                      bool bRecursive = false,
+                      bool bCanSkipInvisible = true );
 
     void InvalidatePosOrSize( const SwFrame* pFrame,
                               const SdrObject* pObj,
diff --git a/sw/source/core/access/acccell.cxx b/sw/source/core/access/acccell.cxx
index d448a7f281b7..ca4ce142fe51 100644
--- a/sw/source/core/access/acccell.cxx
+++ b/sw/source/core/access/acccell.cxx
@@ -263,13 +263,13 @@ uno::Sequence< OUString > SAL_CALL SwAccessibleCell::getSupportedServiceNames()
     return aRet;
 }
 
-void SwAccessibleCell::Dispose( bool bRecursive )
+void SwAccessibleCell::Dispose(bool bRecursive, bool bCanSkipInvisible)
 {
     const SwFrame *pParent = GetParent( SwAccessibleChild(GetFrame()), IsInPagePreview() );
     ::rtl::Reference< SwAccessibleContext > xAccImpl(
             GetMap()->GetContextImpl( pParent, false ) );
     if( xAccImpl.is() )
-        xAccImpl->DisposeChild( SwAccessibleChild(GetFrame()), bRecursive );
+        xAccImpl->DisposeChild(SwAccessibleChild(GetFrame()), bRecursive, bCanSkipInvisible);
     SwAccessibleContext::Dispose( bRecursive );
 }
 
diff --git a/sw/source/core/access/acccell.hxx b/sw/source/core/access/acccell.hxx
index 594dc73126d7..9d543c61255c 100644
--- a/sw/source/core/access/acccell.hxx
+++ b/sw/source/core/access/acccell.hxx
@@ -83,7 +83,7 @@ public:
         getSupportedServiceNames()
         throw (css::uno::RuntimeException, std::exception) override;
 
-    virtual void Dispose( bool bRecursive = false ) override;
+    virtual void Dispose(bool bRecursive = false, bool bCanSkipInvisible = true) override;
 
     virtual void InvalidatePosOrSize( const SwRect& rFrame ) override;
 
diff --git a/sw/source/core/access/acccontext.cxx b/sw/source/core/access/acccontext.cxx
index b74ed7006a6b..8fb2e30f7bfa 100644
--- a/sw/source/core/access/acccontext.cxx
+++ b/sw/source/core/access/acccontext.cxx
@@ -393,8 +393,9 @@ void SwAccessibleContext::InvalidateChildrenStates( const SwFrame* _pFrame,
     }
 }
 
-void SwAccessibleContext::DisposeChildren( const SwFrame *pFrame,
-                                           bool bRecursive )
+void SwAccessibleContext::DisposeChildren(const SwFrame *pFrame,
+                                          bool bRecursive,
+                                          bool bCanSkipInvisible)
 {
     const SwAccessibleChildSList aVisList( GetVisArea(), *pFrame, *(GetMap()) );
     SwAccessibleChildSList::const_iterator aIter( aVisList.begin() );
@@ -410,7 +411,7 @@ void SwAccessibleContext::DisposeChildren( const SwFrame *pFrame,
             if( xAccImpl.is() )
                 xAccImpl->Dispose( bRecursive );
             else if( bRecursive )
-                DisposeChildren( pLower, bRecursive );
+                DisposeChildren(pLower, bRecursive, bCanSkipInvisible);
         }
         else if ( rLower.GetDrawObject() )
         {
@@ -422,7 +423,7 @@ void SwAccessibleContext::DisposeChildren( const SwFrame *pFrame,
         }
         else if ( rLower.GetWindow() )
         {
-            DisposeChild( rLower, false );
+            DisposeChild(rLower, false, bCanSkipInvisible);
         }
         ++aIter;
     }
@@ -1032,7 +1033,7 @@ void SwAccessibleContext::ScrolledInShape( const SdrObject* ,
     }
 }
 
-void SwAccessibleContext::Dispose( bool bRecursive )
+void SwAccessibleContext::Dispose(bool bRecursive, bool bCanSkipInvisible)
 {
     SolarMutexGuard aGuard;
 
@@ -1044,7 +1045,7 @@ void SwAccessibleContext::Dispose( bool bRecursive )
 
     // dispose children
     if( bRecursive )
-        DisposeChildren( GetFrame(), bRecursive );
+        DisposeChildren(GetFrame(), bRecursive, bCanSkipInvisible);
 
     // get parent
     uno::Reference< XAccessible > xParent( GetWeakParent() );
@@ -1083,12 +1084,13 @@ void SwAccessibleContext::Dispose( bool bRecursive )
 }
 
 void SwAccessibleContext::DisposeChild( const SwAccessibleChild& rChildFrameOrObj,
-                                        bool bRecursive )
+                                        bool bRecursive, bool bCanSkipInvisible )
 {
     SolarMutexGuard aGuard;
 
-    if ( IsShowing( *(GetMap()), rChildFrameOrObj ) ||
+    if ( !bCanSkipInvisible ||
          rChildFrameOrObj.AlwaysIncludeAsChild() ||
+         IsShowing( *(GetMap()), rChildFrameOrObj ) ||
          !SwAccessibleChild( GetFrame() ).IsVisibleChildrenOnly() )
     {
         // If the object could have existed before, than there is nothing to do,
@@ -1119,7 +1121,7 @@ void SwAccessibleContext::DisposeChild( const SwAccessibleChild& rChildFrameOrOb
         }
     }
     else if( bRecursive && rChildFrameOrObj.GetSwFrame() )
-        DisposeChildren( rChildFrameOrObj.GetSwFrame(), bRecursive );
+        DisposeChildren(rChildFrameOrObj.GetSwFrame(), bRecursive, bCanSkipInvisible);
 }
 
 void SwAccessibleContext::InvalidatePosOrSize( const SwRect& )
diff --git a/sw/source/core/access/acccontext.hxx b/sw/source/core/access/acccontext.hxx
index 3adcbbd05900..6c6ae83c591f 100644
--- a/sw/source/core/access/acccontext.hxx
+++ b/sw/source/core/access/acccontext.hxx
@@ -147,8 +147,8 @@ protected:
 
     // Dispose children of the specified SwFrame. The SwFrame might belong to
     // the current object or to any other child or grandchild.
-    void DisposeChildren( const SwFrame *pFrame,
-                          bool bRecursive );
+    void DisposeChildren(const SwFrame *pFrame,
+                         bool bRecursive, bool bCanSkipInvisible);
 
     void DisposeShape( const SdrObject *pObj,
                                 ::accessibility::AccessibleShape *pAccImpl );
@@ -315,10 +315,10 @@ public:
     // thread safe C++ interface
 
     // The object is not visible an longer and should be destroyed
-    virtual void Dispose( bool bRecursive = false );
+    virtual void Dispose(bool bRecursive = false, bool bCanSkipInvisible = true);
 
     // The child object is not visible an longer and should be destroyed
-    virtual void DisposeChild( const sw::access::SwAccessibleChild& rFrameOrObj, bool bRecursive );
+    virtual void DisposeChild(const sw::access::SwAccessibleChild& rFrameOrObj, bool bRecursive, bool bCanSkipInvisible);
 
     // The object has been moved by the layout
     virtual void InvalidatePosOrSize( const SwRect& rFrame );
diff --git a/sw/source/core/access/accdoc.cxx b/sw/source/core/access/accdoc.cxx
index f24ece41ec0b..31529e1bbd58 100644
--- a/sw/source/core/access/accdoc.cxx
+++ b/sw/source/core/access/accdoc.cxx
@@ -375,14 +375,14 @@ SwAccessibleDocument::~SwAccessibleDocument()
         pWin->RemoveChildEventListener( LINK( this, SwAccessibleDocument, WindowChildEventListener ));
 }
 
-void SwAccessibleDocument::Dispose( bool bRecursive )
+void SwAccessibleDocument::Dispose(bool bRecursive, bool bCanSkipInvisible)
 {
     OSL_ENSURE( GetFrame() && GetMap(), "already disposed" );
 
     vcl::Window *pWin = GetMap() ? GetMap()->GetShell()->GetWin() : nullptr;
     if( pWin )
         pWin->RemoveChildEventListener( LINK( this, SwAccessibleDocument, WindowChildEventListener ));
-    SwAccessibleContext::Dispose( bRecursive );
+    SwAccessibleContext::Dispose(bRecursive, bCanSkipInvisible);
 }
 
 IMPL_LINK_TYPED( SwAccessibleDocument, WindowChildEventListener, VclWindowEvent&, rEvent, void )
diff --git a/sw/source/core/access/accdoc.hxx b/sw/source/core/access/accdoc.hxx
index 5fdb656b7772..e11f068a0d29 100644
--- a/sw/source/core/access/accdoc.hxx
+++ b/sw/source/core/access/accdoc.hxx
@@ -198,7 +198,7 @@ public:
     // thread safe C++ interface
 
     // The object is not visible an longer and should be destroyed
-    virtual void Dispose( bool bRecursive = false ) override;
+    virtual void Dispose(bool bRecursive = false, bool bCanSkipInvisible = true) override;
 
     // XAccessibleComponent
     sal_Int32 SAL_CALL getBackground()
diff --git a/sw/source/core/access/accframebase.cxx b/sw/source/core/access/accframebase.cxx
index 9cf0253601ca..13823155ca18 100644
--- a/sw/source/core/access/accframebase.cxx
+++ b/sw/source/core/access/accframebase.cxx
@@ -263,14 +263,14 @@ void SwAccessibleFrameBase::Modify( const SfxPoolItem* pOld, const SfxPoolItem *
     }
 }
 
-void SwAccessibleFrameBase::Dispose( bool bRecursive )
+void SwAccessibleFrameBase::Dispose(bool bRecursive, bool bCanSkipInvisible)
 {
     SolarMutexGuard aGuard;
 
     if( GetRegisteredIn() )
         GetRegisteredInNonConst()->Remove( this );
 
-    SwAccessibleContext::Dispose( bRecursive );
+    SwAccessibleContext::Dispose(bRecursive, bCanSkipInvisible);
 }
 
 //Get the selection cursor of the document.
diff --git a/sw/source/core/access/accframebase.hxx b/sw/source/core/access/accframebase.hxx
index 7d1ac0cf9897..6df342ab4372 100644
--- a/sw/source/core/access/accframebase.hxx
+++ b/sw/source/core/access/accframebase.hxx
@@ -57,7 +57,7 @@ public:
     static sal_uInt8 GetNodeType( const SwFlyFrame *pFlyFrame );
 
     // The object is not visible an longer and should be destroyed
-    virtual void Dispose( bool bRecursive = false ) override;
+    virtual void Dispose(bool bRecursive = false, bool bCanSkipInvisible = true) override;
     virtual bool SetSelectedState( bool bSeleted ) override;
 };
 
diff --git a/sw/source/core/access/accmap.cxx b/sw/source/core/access/accmap.cxx
index 26720d1267ce..3921ec1747d3 100644
--- a/sw/source/core/access/accmap.cxx
+++ b/sw/source/core/access/accmap.cxx
@@ -2244,7 +2244,8 @@ void SwAccessibleMap::RemoveContext( const SdrObject *pObj )
 void SwAccessibleMap::A11yDispose( const SwFrame *pFrame,
                                    const SdrObject *pObj,
                                    vcl::Window* pWindow,
-                                   bool bRecursive )
+                                   bool bRecursive,
+                                   bool bCanSkipInvisible )
 {
     SwAccessibleChild aFrameOrObj( pFrame, pObj, pWindow );
 
@@ -2355,7 +2356,7 @@ void SwAccessibleMap::A11yDispose( const SwFrame *pFrame,
             // be broadcasted at the end of the action to give the table
             // a chance to generate a single table change event.
 
-            xParentAccImpl->DisposeChild( aFrameOrObj, bRecursive );
+            xParentAccImpl->DisposeChild( aFrameOrObj, bRecursive, bCanSkipInvisible );
         }
         else if( xShapeAccImpl.is() )
         {
diff --git a/sw/source/core/access/accnotextframe.cxx b/sw/source/core/access/accnotextframe.cxx
index 154526f6b808..f07962c101b0 100644
--- a/sw/source/core/access/accnotextframe.cxx
+++ b/sw/source/core/access/accnotextframe.cxx
@@ -164,14 +164,14 @@ void SwAccessibleNoTextFrame::Modify( const SfxPoolItem* pOld, const SfxPoolItem
     }
 }
 
-void SwAccessibleNoTextFrame::Dispose( bool bRecursive )
+void SwAccessibleNoTextFrame::Dispose(bool bRecursive, bool bCanSkipInvisible)
 {
     SolarMutexGuard aGuard;
 
     if( aDepend.GetRegisteredIn() )
         aDepend.GetRegisteredIn()->Remove( &aDepend );
 
-    SwAccessibleFrameBase::Dispose( bRecursive );
+    SwAccessibleFrameBase::Dispose(bRecursive, bCanSkipInvisible);
 }
 
 // #i73249#
diff --git a/sw/source/core/access/accnotextframe.hxx b/sw/source/core/access/accnotextframe.hxx
index d11dd25455fb..6be3005af60d 100644
--- a/sw/source/core/access/accnotextframe.hxx
+++ b/sw/source/core/access/accnotextframe.hxx
@@ -98,7 +98,7 @@ public:
         throw ( css::uno::RuntimeException, std::exception ) override;
 
     // The object is not visible an longer and should be destroyed
-    virtual void Dispose( bool bRecursive = false ) override;
+    virtual void Dispose(bool bRecursive = false, bool bCanSkipInvisible = true) override;
 
     virtual sal_Int32 SAL_CALL getCaretPosition(  ) throw (css::uno::RuntimeException, std::exception) override;
     virtual sal_Bool SAL_CALL setCaretPosition( sal_Int32 nIndex ) throw (css::lang::IndexOutOfBoundsException, css::uno::RuntimeException, std::exception) override;
diff --git a/sw/source/core/access/acctable.cxx b/sw/source/core/access/acctable.cxx
index 8f2c4e3ea60e..8172657d0eff 100644
--- a/sw/source/core/access/acctable.cxx
+++ b/sw/source/core/access/acctable.cxx
@@ -1391,18 +1391,18 @@ void SwAccessibleTable::InvalidatePosOrSize( const SwRect& rOldBox )
     SwAccessibleContext::InvalidatePosOrSize( rOldBox );
 }
 
-void SwAccessibleTable::Dispose( bool bRecursive )
+void SwAccessibleTable::Dispose(bool bRecursive, bool bCanSkipInvisible)
 {
     SolarMutexGuard aGuard;
 
     if( GetRegisteredIn() )
         GetRegisteredInNonConst()->Remove( this );
 
-    SwAccessibleContext::Dispose( bRecursive );
+    SwAccessibleContext::Dispose(bRecursive, bCanSkipInvisible);
 }
 
 void SwAccessibleTable::DisposeChild( const SwAccessibleChild& rChildFrameOrObj,
-                                      bool bRecursive )
+                                      bool bRecursive, bool bCanSkipInvisible )
 {
     SolarMutexGuard aGuard;
 
@@ -1421,7 +1421,7 @@ void SwAccessibleTable::DisposeChild( const SwAccessibleChild& rChildFrameOrObj,
     // about its change. We then must not call the superclass
     uno::Reference< XAccessible > xAcc( GetMap()->GetContext( pFrame, false ) );
     if( !xAcc.is() )
-        SwAccessibleContext::DisposeChild( rChildFrameOrObj, bRecursive );
+        SwAccessibleContext::DisposeChild( rChildFrameOrObj, bRecursive, bCanSkipInvisible );
 }
 
 void SwAccessibleTable::InvalidateChildPosOrSize( const SwAccessibleChild& rChildFrameOrObj,
diff --git a/sw/source/core/access/acctable.hxx b/sw/source/core/access/acctable.hxx
index 857636eaa19d..939e823a07dc 100644
--- a/sw/source/core/access/acctable.hxx
+++ b/sw/source/core/access/acctable.hxx
@@ -224,10 +224,10 @@ public:
     virtual void InvalidatePosOrSize( const SwRect& rOldBox ) override;
 
     // The object is not visible an longer and should be destroyed
-    virtual void Dispose( bool bRecursive = false ) override;
+    virtual void Dispose(bool bRecursive = false, bool bCanSkipInvisible = true) override;
 
     virtual void DisposeChild( const sw::access::SwAccessibleChild& rFrameOrObj,
-                               bool bRecursive ) override;
+                               bool bRecursive, bool bCanSkipInvisible ) override;
     virtual void InvalidateChildPosOrSize( const sw::access::SwAccessibleChild& rFrameOrObj,
                                            const SwRect& rFrame ) override;
 
diff --git a/sw/source/core/draw/dview.cxx b/sw/source/core/draw/dview.cxx
index 495485c82bdd..5105092e4e8a 100644
--- a/sw/source/core/draw/dview.cxx
+++ b/sw/source/core/draw/dview.cxx
@@ -375,7 +375,7 @@ void SwDrawView::MoveRepeatedObjs( const SwAnchoredObject& _rMovedAnchoredObj,
                 }
                 else
                 {
-                    rImp.DisposeAccessibleObj( pAnchoredObj->GetDrawObj() );
+                    rImp.DisposeAccessibleObj(pAnchoredObj->GetDrawObj(), true);
                     rImp.AddAccessibleObj( pAnchoredObj->GetDrawObj() );
                 }
             }
@@ -411,7 +411,7 @@ void SwDrawView::MoveRepeatedObjs( const SwAnchoredObject& _rMovedAnchoredObj,
                     }
                     else
                     {
-                        rImp.DisposeAccessibleObj( pAnchoredObj->GetDrawObj() );
+                        rImp.DisposeAccessibleObj(pAnchoredObj->GetDrawObj(), true);
                         rImp.AddAccessibleObj( pAnchoredObj->GetDrawObj() );
                     }
                 }
@@ -621,7 +621,7 @@ void SwDrawView::ObjOrderChanged( SdrObject* pObj, sal_uLong nOldPos,
                 }
                 else
                 {
-                    rImp.DisposeAccessibleObj( pTmpObj );
+                    rImp.DisposeAccessibleObj(pTmpObj, true);
                     rImp.AddAccessibleObj( pTmpObj );
                 }
             }
@@ -640,7 +640,7 @@ void SwDrawView::ObjOrderChanged( SdrObject* pObj, sal_uLong nOldPos,
     else
     {
         // adjustments for accessibility API
-        rImp.DisposeAccessibleObj( pObj );
+        rImp.DisposeAccessibleObj(pObj, true);
         rImp.AddAccessibleObj( pObj );
     }
 
diff --git a/sw/source/core/inc/viewimp.hxx b/sw/source/core/inc/viewimp.hxx
index 4ebe4897c7fb..be88ca9d2036 100644
--- a/sw/source/core/inc/viewimp.hxx
+++ b/sw/source/core/inc/viewimp.hxx
@@ -225,10 +225,10 @@ public:
 
     /// Remove a frame from the accessible view
     void DisposeAccessible( const SwFrame *pFrame, const SdrObject *pObj,
-                            bool bRecursive );
+                            bool bRecursive, bool bCanSkipInvisible );
     inline void DisposeAccessibleFrame( const SwFrame *pFrame,
                                bool bRecursive = false );
-    inline void DisposeAccessibleObj( const SdrObject *pObj );
+    inline void DisposeAccessibleObj( const SdrObject *pObj, bool bCanSkipInvisible );
 
     /// Move a frame's position in the accessible view
     void MoveAccessible( const SwFrame *pFrame, const SdrObject *pObj,
@@ -278,12 +278,12 @@ inline SwAccessibleMap& SwViewShellImp::GetAccessibleMap()
 inline void SwViewShellImp::DisposeAccessibleFrame( const SwFrame *pFrame,
                                bool bRecursive )
 {
-    DisposeAccessible( pFrame, nullptr, bRecursive );
+    DisposeAccessible( pFrame, nullptr, bRecursive, true );
 }
 
-inline void SwViewShellImp::DisposeAccessibleObj( const SdrObject *pObj )
+inline void SwViewShellImp::DisposeAccessibleObj( const SdrObject *pObj, bool bCanSkipInvisible )
 {
-    DisposeAccessible( nullptr, pObj, false );
+    DisposeAccessible( nullptr, pObj, false, bCanSkipInvisible );
 }
 
 inline void SwViewShellImp::MoveAccessibleFrame( const SwFrame *pFrame,
diff --git a/sw/source/core/layout/fly.cxx b/sw/source/core/layout/fly.cxx
index ab0a2eacbc17..0eff50c862eb 100644
--- a/sw/source/core/layout/fly.cxx
+++ b/sw/source/core/layout/fly.cxx
@@ -2154,8 +2154,8 @@ void SwFrame::RemoveDrawObj( SwAnchoredObject& _rToRemoveObj )
     if( pSh )
     {
         SwRootFrame* pLayout = getRootFrame();
-        if( pLayout && pLayout->IsAnyShellAccessible() )
-        pSh->Imp()->DisposeAccessibleObj( _rToRemoveObj.GetDrawObj() );
+        if (pLayout && pLayout->IsAnyShellAccessible())
+            pSh->Imp()->DisposeAccessibleObj(_rToRemoveObj.GetDrawObj(), false);
     }
 
     // deregister from page frame
diff --git a/sw/source/core/view/viewimp.cxx b/sw/source/core/view/viewimp.cxx
index 09b21ba64f0d..e403be91202e 100644
--- a/sw/source/core/view/viewimp.cxx
+++ b/sw/source/core/view/viewimp.cxx
@@ -300,15 +300,16 @@ void SwViewShellImp::UpdateAccessible()
         GetAccessibleMap().GetDocumentView();
 }
 
-void SwViewShellImp::DisposeAccessible( const SwFrame *pFrame,
-                                   const SdrObject *pObj,
-                                   bool bRecursive )
+void SwViewShellImp::DisposeAccessible(const SwFrame *pFrame,
+                                       const SdrObject *pObj,
+                                       bool bRecursive,
+                                       bool bCanSkipInvisible)
 {
     OSL_ENSURE( !pFrame || pFrame->IsAccessibleFrame(), "frame is not accessible" );
     for(SwViewShell& rTmp : GetShell()->GetRingContainer())
     {
         if( rTmp.Imp()->IsAccessible() )
-            rTmp.Imp()->GetAccessibleMap().A11yDispose( pFrame, pObj, nullptr, bRecursive );
+            rTmp.Imp()->GetAccessibleMap().A11yDispose( pFrame, pObj, nullptr, bRecursive, bCanSkipInvisible );
     }
 }