[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 777bc22ca6 forms/source/misc/InterfaceContainer.cxx: removed a leftover from commit a19cd21e3c which reduced guarded scope forms/source/component/DatabaseForm.cxx: fixed clear-reset sequence broken from the initial commit bf4154eb53 Change-Id: Ibab6660c79561eee31faf3e6c1128ab141a7e8a3 Reviewed-on: https://gerrit.libreoffice.org/70381 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2019-04-09 21:34:07 +02:00
parent c1e320836b
commit d38f9934f0
5 changed files with 69 additions and 69 deletions
@@ -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
@@ -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 );
    }
 }
@@ -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;
-
-    bool            bMustBeLayouted( false );
-    bool            bNotify( false );
-    OUString aElementType;
-    OUString aElementName;
-
-    parseResourceURL( aName, aElementType, aElementName );
-
-    if ( aElementType.equalsIgnoreAsciiCase("menubar") &&
-         aElementName.equalsIgnoreAsciiCase("menubar") )
    {
-        if ( !m_bInplaceMenuSet )
+        SolarMutexClearableGuard aWriteLock;
+
+        OUString aElementType;
+        OUString aElementName;
+
+        parseResourceURL(aName, aElementType, aElementName);
+
+        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 )
@@ -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
@@ -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();
        }
    };