forms/source/component/DatabaseForm.cxx                 |    3 
 forms/source/misc/InterfaceContainer.cxx                |    1 
 framework/source/layoutmanager/layoutmanager.cxx        |   92 ++++++++--------
 framework/source/layoutmanager/toolbarlayoutmanager.cxx |   22 ++-
 include/osl/mutex.hxx                                   |   16 +-
 5 files changed, 67 insertions(+), 67 deletions(-)

New commits:
commit d38f9934f08939032cca64a32de58fa3901a88d5
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sun Apr 7 15:29:35 2019 +0100
Commit:     Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Tue Apr 9 21:34:07 2019 +0200

    [API CHANGE] Asserts to never clear already cleared guard
    
    ... which could help catch copy-paste errors when wrong guard is cleared
    second time.
    
    Also an assert added that when resetting, there's something to reset
    (i.e., no descendant class had cleared protected pResetT, making reset
    impossible, and thus actually unable to guard anything).
    
    framework/source/layoutmanager/layoutmanager.cxx: made sure to not call
    clear() second time
    
    framework/source/layoutmanager/toolbarlayoutmanager.cxx: restored lock
    lost in commit 777bc22ca6490a4300f30fc1b45287dce789a36f
    
    forms/source/misc/InterfaceContainer.cxx: removed a leftover from commit
    a19cd21e3c03559877428315bebc0ceaf367a461 which reduced guarded scope
    
    forms/source/component/DatabaseForm.cxx: fixed clear-reset sequence
    broken from the initial commit bf4154eb5307ec8c35f000fd1df39ef3abb2eb6d
    
    Change-Id: Ibab6660c79561eee31faf3e6c1128ab141a7e8a3
    Reviewed-on: https://gerrit.libreoffice.org/70381
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>

diff --git a/forms/source/component/DatabaseForm.cxx 
b/forms/source/component/DatabaseForm.cxx
index 30795285a12d..378ee9dc04c9 100644
--- a/forms/source/component/DatabaseForm.cxx
+++ b/forms/source/component/DatabaseForm.cxx
@@ -2853,16 +2853,15 @@ void SAL_CALL ODatabaseForm::unload()
             // close the aggregate
             Reference<XCloseable>  xCloseable;
             query_aggregation( m_xAggregate, xCloseable);
-            aGuard.clear();
             if (xCloseable.is())
                 xCloseable->close();
         }
         catch(const SQLException&)
         {
         }
-        aGuard.reset();
     }
 
+    aGuard.reset();
     m_bLoaded = false;
 
     // if the connection we used while we were loaded is only shared with our 
parent, we
diff --git a/forms/source/misc/InterfaceContainer.cxx 
b/forms/source/misc/InterfaceContainer.cxx
index 36756bdfeae2..e4ddc4265113 100644
--- a/forms/source/misc/InterfaceContainer.cxx
+++ b/forms/source/misc/InterfaceContainer.cxx
@@ -855,7 +855,6 @@ void OInterfaceContainer::implInsert(sal_Int32 _nIndex, 
const Reference< XProper
         aEvt.Accessor <<= _nIndex;
         aEvt.Element  = pElementMetaData->aElementTypeInterface;
 
-        aGuard.clear();
         m_aContainerListeners.notifyEach( 
&XContainerListener::elementInserted, aEvt );
     }
 }
diff --git a/framework/source/layoutmanager/layoutmanager.cxx 
b/framework/source/layoutmanager/layoutmanager.cxx
index 16b7e099b3c6..274c595ce42c 100644
--- a/framework/source/layoutmanager/layoutmanager.cxx
+++ b/framework/source/layoutmanager/layoutmanager.cxx
@@ -1520,60 +1520,62 @@ void SAL_CALL LayoutManager::destroyElement( const 
OUString& aName )
 {
     SAL_INFO( "fwk", "framework (cd100003) ::LayoutManager::destroyElement" );
 
+    bool bMustBeLayouted(false);
+    bool bNotify(false);
     /* SAFE AREA 
-----------------------------------------------------------------------------------------------
 */
-    SolarMutexClearableGuard aWriteLock;
+    {
+        SolarMutexClearableGuard aWriteLock;
 
-    bool            bMustBeLayouted( false );
-    bool            bNotify( false );
-    OUString aElementType;
-    OUString aElementName;
+        OUString aElementType;
+        OUString aElementName;
 
-    parseResourceURL( aName, aElementType, aElementName );
+        parseResourceURL(aName, aElementType, aElementName);
 
-    if ( aElementType.equalsIgnoreAsciiCase("menubar") &&
-         aElementName.equalsIgnoreAsciiCase("menubar") )
-    {
-        if ( !m_bInplaceMenuSet )
+        if (aElementType.equalsIgnoreAsciiCase("menubar")
+            && aElementName.equalsIgnoreAsciiCase("menubar"))
         {
-            impl_clearUpMenuBar();
-            m_xMenuBar.clear();
+            if (!m_bInplaceMenuSet)
+            {
+                impl_clearUpMenuBar();
+                m_xMenuBar.clear();
+                bNotify = true;
+            }
+        }
+        else if ((aElementType.equalsIgnoreAsciiCase("statusbar")
+                  && aElementName.equalsIgnoreAsciiCase("statusbar"))
+                 || (m_aStatusBarElement.m_aName == aName))
+        {
+            aWriteLock.clear();
+            implts_destroyStatusBar();
+            bMustBeLayouted = true;
             bNotify = true;
         }
-    }
-    else if (( aElementType.equalsIgnoreAsciiCase("statusbar") &&
-               aElementName.equalsIgnoreAsciiCase("statusbar") ) ||
-             ( m_aStatusBarElement.m_aName == aName ))
-    {
-        aWriteLock.clear();
-        implts_destroyStatusBar();
-        bMustBeLayouted = true;
-        bNotify         = true;
-    }
-    else if ( aElementType.equalsIgnoreAsciiCase("progressbar") &&
-              aElementName.equalsIgnoreAsciiCase("progressbar") )
-    {
-        aWriteLock.clear();
-        implts_createProgressBar();
-        bMustBeLayouted = true;
-        bNotify = true;
-    }
-    else if ( aElementType.equalsIgnoreAsciiCase( UIRESOURCETYPE_TOOLBAR ) && 
m_xToolbarManager.is() )
-    {
-        aWriteLock.clear();
-        bNotify         = m_xToolbarManager->destroyToolbar( aName );
-        bMustBeLayouted = m_xToolbarManager->isLayoutDirty();
-    }
-    else if ( aElementType.equalsIgnoreAsciiCase("dockingwindow"))
-    {
-        uno::Reference< frame::XFrame > xFrame( m_xFrame );
-        uno::Reference< XComponentContext > xContext( m_xContext );
-        aWriteLock.clear();
+        else if (aElementType.equalsIgnoreAsciiCase("progressbar")
+                 && aElementName.equalsIgnoreAsciiCase("progressbar"))
+        {
+            aWriteLock.clear();
+            implts_createProgressBar();
+            bMustBeLayouted = true;
+            bNotify = true;
+        }
+        else if (aElementType.equalsIgnoreAsciiCase(UIRESOURCETYPE_TOOLBAR)
+                 && m_xToolbarManager.is())
+        {
+            aWriteLock.clear();
+            bNotify = m_xToolbarManager->destroyToolbar(aName);
+            bMustBeLayouted = m_xToolbarManager->isLayoutDirty();
+        }
+        else if (aElementType.equalsIgnoreAsciiCase("dockingwindow"))
+        {
+            uno::Reference<frame::XFrame> xFrame(m_xFrame);
+            uno::Reference<XComponentContext> xContext(m_xContext);
+            aWriteLock.clear();
 
-        impl_setDockingWindowVisibility( xContext, xFrame, aElementName, false 
);
-        bMustBeLayouted = false;
-        bNotify         = false;
+            impl_setDockingWindowVisibility(xContext, xFrame, aElementName, 
false);
+            bMustBeLayouted = false;
+            bNotify = false;
+        }
     }
-    aWriteLock.clear();
     /* SAFE AREA 
-----------------------------------------------------------------------------------------------
 */
 
     if ( bMustBeLayouted )
diff --git a/framework/source/layoutmanager/toolbarlayoutmanager.cxx 
b/framework/source/layoutmanager/toolbarlayoutmanager.cxx
index aca93c4e2f03..d5708db9e895 100644
--- a/framework/source/layoutmanager/toolbarlayoutmanager.cxx
+++ b/framework/source/layoutmanager/toolbarlayoutmanager.cxx
@@ -440,12 +440,13 @@ bool ToolbarLayoutManager::createToolbar( const OUString& 
rResourceURL )
 {
     bool bNotify( false );
 
-    SolarMutexClearableGuard aReadLock;
-    uno::Reference< frame::XFrame > xFrame( m_xFrame );
-    uno::Reference< awt::XWindow2 > xContainerWindow( m_xContainerWindow );
-    aReadLock.clear();
-
-    bNotify = false;
+    uno::Reference<frame::XFrame> xFrame;
+    uno::Reference<awt::XWindow2> xContainerWindow;
+    {
+        SolarMutexGuard aReadLock;
+        xFrame.set(m_xFrame);
+        xContainerWindow.set(m_xContainerWindow);
+    }
 
     if ( !xFrame.is() || !xContainerWindow.is() )
         return false;
@@ -457,11 +458,14 @@ bool ToolbarLayoutManager::createToolbar( const OUString& 
rResourceURL )
 
         uno::Sequence< beans::PropertyValue > aPropSeq( 2 );
         aPropSeq[0].Name = "Frame";
-        aPropSeq[0].Value <<= m_xFrame;
+        aPropSeq[0].Value <<= xFrame;
         aPropSeq[1].Name = "Persistent";
         aPropSeq[1].Value <<= true;
-        uno::Reference< ui::XUIElementFactory > xUIElementFactory( 
m_xUIElementFactoryManager );
-        aReadLock.clear();
+        uno::Reference<ui::XUIElementFactory> xUIElementFactory;
+        {
+            SolarMutexGuard aReadLock;
+            xUIElementFactory.set(m_xUIElementFactoryManager);
+        }
 
         implts_setToolbarCreation();
         try
diff --git a/include/osl/mutex.hxx b/include/osl/mutex.hxx
index 5bcf0e56cce0..ea7cdf50d19e 100644
--- a/include/osl/mutex.hxx
+++ b/include/osl/mutex.hxx
@@ -178,11 +178,9 @@ namespace osl
         */
         void clear()
         {
-            if(pT)
-            {
-                pT->release();
-                pT = NULL;
-            }
+            assert(pT);
+            pT->release();
+            pT = NULL;
         }
     };
 
@@ -216,11 +214,9 @@ namespace osl
         */
         void reset()
         {
-            if( pResetT )
-            {
-                this->pT = pResetT;
-                this->pT->acquire();
-            }
+            assert(!this->pT);
+            this->pT = pResetT;
+            this->pT->acquire();
         }
     };
 
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to