[Libreoffice-commits] core.git: Branch 'libreoffice-5-3' - desktop/source

[Stephan Bergmann]

 desktop/source/deployment/gui/dp_gui_extlistbox.cxx |  132 +++++++++++---------
 1 file changed, 74 insertions(+), 58 deletions(-)

New commits:
commit d5364fe73702b1097e546ea35dd493723e7333ed
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Tue Nov 29 14:03:52 2016 +0100

    Window::Invalidate must be called with SolarMutex locked
    
    ...it internally calls Scheduler::Start, which asserts now since
    c00d8271ba443c4f0acf657c226eea4824597f95 "vcl: assert solar mutex is held for
    various timer / scheduler ops."  Caused the assert to fire when removing an
    extension via "Tools - Extension Manager... - Remove".
    
    Change-Id: Ied0591b799af333b3a4c11af07f5f8f1657304d6
    (cherry picked from commit e006b9cf2cfba5995b97a9a9551d362b7eb45572)
    Reviewed-on: https://gerrit.libreoffice.org/31366
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/desktop/source/deployment/gui/dp_gui_extlistbox.cxx b/desktop/source/deployment/gui/dp_gui_extlistbox.cxx
index df198dc..9d62aed 100644
--- a/desktop/source/deployment/gui/dp_gui_extlistbox.cxx
+++ b/desktop/source/deployment/gui/dp_gui_extlistbox.cxx
@@ -376,45 +376,52 @@ void ExtensionBox_Impl::DeleteRemoved()
 //This function may be called with nPos < 0
 void ExtensionBox_Impl::selectEntry( const long nPos )
 {
-    //ToDo whe should not use the guard at such a big scope here.
-    //Currently it is used to guard m_vEntries and m_nActive. m_nActive will be
-    //modified in this function.
-    //It would be probably best to always use a copy of m_vEntries
-    //and some other state variables from ExtensionBox_Impl for
-    //the whole painting operation. See issue i86993
-    ::osl::ClearableMutexGuard guard(m_entriesMutex);
-
-    if ( m_bInCheckMode )
-        return;
-
-    if ( m_bHasActive )
+    bool invalidate = false;
     {
-        if ( nPos == m_nActive )
+        //ToDo whe should not use the guard at such a big scope here.
+        //Currently it is used to guard m_vEntries and m_nActive. m_nActive will be
+        //modified in this function.
+        //It would be probably best to always use a copy of m_vEntries
+        //and some other state variables from ExtensionBox_Impl for
+        //the whole painting operation. See issue i86993
+        ::osl::MutexGuard guard(m_entriesMutex);
+
+        if ( m_bInCheckMode )
             return;
 
-        m_bHasActive = false;
-        m_vEntries[ m_nActive ]->m_bActive = false;
-    }
+        if ( m_bHasActive )
+        {
+            if ( nPos == m_nActive )
+                return;
 
-    if ( ( nPos >= 0 ) && ( nPos < (long) m_vEntries.size() ) )
-    {
-        m_bHasActive = true;
-        m_nActive = nPos;
-        m_vEntries[ nPos ]->m_bActive = true;
+            m_bHasActive = false;
+            m_vEntries[ m_nActive ]->m_bActive = false;
+        }
+
+        if ( ( nPos >= 0 ) && ( nPos < (long) m_vEntries.size() ) )
+        {
+            m_bHasActive = true;
+            m_nActive = nPos;
+            m_vEntries[ nPos ]->m_bActive = true;
+
+            if ( IsReallyVisible() )
+            {
+                m_bAdjustActive = true;
+            }
+        }
 
         if ( IsReallyVisible() )
         {
-            m_bAdjustActive = true;
+            m_bNeedsRecalc = true;
+            invalidate = true;
         }
     }
 
-    if ( IsReallyVisible() )
+    if (invalidate)
     {
-        m_bNeedsRecalc = true;
+        SolarMutexGuard g;
         Invalidate();
     }
-
-    guard.clear();
 }
 
 
@@ -1005,45 +1012,54 @@ void ExtensionBox_Impl::removeEntry( const uno::Reference< deployment::XPackage
 {
    if ( ! m_bInDelete )
     {
-        ::osl::ClearableMutexGuard aGuard( m_entriesMutex );
-
-        typedef std::vector< TEntry_Impl >::iterator ITER;
-
-        for ( ITER iIndex = m_vEntries.begin(); iIndex < m_vEntries.end(); ++iIndex )
+        bool invalidate = false;
         {
-            if ( (*iIndex)->m_xPackage == xPackage )
-            {
-                long nPos = iIndex - m_vEntries.begin();
-
-                // Entries mustn't be removed here, because they contain a hyperlink control
-                // which can only be deleted when the thread has the solar mutex. Therefore
-                // the entry will be moved into the m_vRemovedEntries list which will be
-                // cleared on the next paint event
-                m_vRemovedEntries.push_back( *iIndex );
-                (*iIndex)->m_xPackage->removeEventListener(m_xRemoveListener.get());
-                m_vEntries.erase( iIndex );
-
-                m_bNeedsRecalc = true;
+            ::osl::ClearableMutexGuard aGuard( m_entriesMutex );
 
-                if ( IsReallyVisible() )
-                    Invalidate();
+            typedef std::vector< TEntry_Impl >::iterator ITER;
 
-                if ( m_bHasActive )
+            for ( ITER iIndex = m_vEntries.begin(); iIndex < m_vEntries.end(); ++iIndex )
+            {
+                if ( (*iIndex)->m_xPackage == xPackage )
                 {
-                    if ( nPos < m_nActive )
-                        m_nActive -= 1;
-                    else if ( ( nPos == m_nActive ) &&
-                              ( nPos == (long) m_vEntries.size() ) )
-                        m_nActive -= 1;
-
-                    m_bHasActive = false;
-                    //clear before calling out of this method
-                    aGuard.clear();
-                    selectEntry( m_nActive );
+                    long nPos = iIndex - m_vEntries.begin();
+
+                    // Entries mustn't be removed here, because they contain a hyperlink control
+                    // which can only be deleted when the thread has the solar mutex. Therefore
+                    // the entry will be moved into the m_vRemovedEntries list which will be
+                    // cleared on the next paint event
+                    m_vRemovedEntries.push_back( *iIndex );
+                    (*iIndex)->m_xPackage->removeEventListener(m_xRemoveListener.get());
+                    m_vEntries.erase( iIndex );
+
+                    m_bNeedsRecalc = true;
+
+                    if ( IsReallyVisible() )
+                        invalidate = true;
+
+                    if ( m_bHasActive )
+                    {
+                        if ( nPos < m_nActive )
+                            m_nActive -= 1;
+                        else if ( ( nPos == m_nActive ) &&
+                                  ( nPos == (long) m_vEntries.size() ) )
+                            m_nActive -= 1;
+
+                        m_bHasActive = false;
+                        //clear before calling out of this method
+                        aGuard.clear();
+                        selectEntry( m_nActive );
+                    }
+                    break;
                 }
-                break;
             }
         }
+
+        if (invalidate)
+        {
+            SolarMutexGuard g;
+            Invalidate();
+        }
     }
 }