[vlc-devel] commit: skins2: improve deallocation of ressources for layouts and controls (Erwan Tulou )

git version control git at videolan.org
Thu Jan 14 15:25:26 CET 2010


vlc | branch: master | Erwan Tulou <erwan10 at videolan.org> | Thu Jan 14 14:34:48 2010 +0100| [63304ea5b47fc30e9864884c8632558fba5cd473] | committer: Erwan Tulou 

skins2: improve deallocation of ressources for layouts and controls

Layouts and Controls are interrelated. Whatever the ones first deallocated, it leaves pointers referencing objects already destroyed. and potentially means memory leak.

This patch adds an unsetLayout() function to pair the setLayout() function and aimed at releasing resources.
Policy should now be that things allocated in constructor are released in destructor and things allocated in setLayout are released in unsetLayout.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=63304ea5b47fc30e9864884c8632558fba5cd473
---

 modules/gui/skins2/controls/ctrl_button.cpp  |    6 ++++++
 modules/gui/skins2/controls/ctrl_button.hpp  |    1 +
 modules/gui/skins2/controls/ctrl_generic.cpp |   14 ++++++++++++--
 modules/gui/skins2/controls/ctrl_generic.hpp |    1 +
 modules/gui/skins2/controls/ctrl_move.cpp    |    7 +++++++
 modules/gui/skins2/controls/ctrl_move.hpp    |    1 +
 modules/gui/skins2/controls/ctrl_resize.cpp  |    7 +++++++
 modules/gui/skins2/controls/ctrl_resize.hpp  |    1 +
 modules/gui/skins2/controls/ctrl_video.cpp   |    9 +++++++--
 modules/gui/skins2/controls/ctrl_video.hpp   |    1 +
 modules/gui/skins2/src/generic_layout.cpp    |    9 +++++++++
 11 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/modules/gui/skins2/controls/ctrl_button.cpp b/modules/gui/skins2/controls/ctrl_button.cpp
index 071b0ff..a1eb125 100644
--- a/modules/gui/skins2/controls/ctrl_button.cpp
+++ b/modules/gui/skins2/controls/ctrl_button.cpp
@@ -89,6 +89,12 @@ void CtrlButton::setLayout( GenericLayout *pLayout,
 }
 
 
+void CtrlButton::unsetLayout()
+{
+    m_pLayout->getActiveVar().delObserver( this );
+    CtrlGeneric::unsetLayout();
+}
+
 void CtrlButton::handleEvent( EvtGeneric &rEvent )
 {
     m_fsm.handleTransition( rEvent.getAsString() );
diff --git a/modules/gui/skins2/controls/ctrl_button.hpp b/modules/gui/skins2/controls/ctrl_button.hpp
index f1ef77e..6e4581a 100644
--- a/modules/gui/skins2/controls/ctrl_button.hpp
+++ b/modules/gui/skins2/controls/ctrl_button.hpp
@@ -48,6 +48,7 @@ public:
     /// Set the position and the associated layout of the control
     virtual void setLayout( GenericLayout *pLayout,
                             const Position &rPosition );
+    virtual void unsetLayout();
 
     /// Handle an event
     virtual void handleEvent( EvtGeneric &rEvent );
diff --git a/modules/gui/skins2/controls/ctrl_generic.cpp b/modules/gui/skins2/controls/ctrl_generic.cpp
index 77e0e8e..81465fe 100644
--- a/modules/gui/skins2/controls/ctrl_generic.cpp
+++ b/modules/gui/skins2/controls/ctrl_generic.cpp
@@ -29,6 +29,8 @@
 #include "../utils/position.hpp"
 #include "../utils/var_bool.hpp"
 
+#include <assert.h>
+
 
 CtrlGeneric::CtrlGeneric( intf_thread_t *pIntf, const UString &rHelp,
                           VarBool *pVisible):
@@ -45,7 +47,6 @@ CtrlGeneric::CtrlGeneric( intf_thread_t *pIntf, const UString &rHelp,
 
 CtrlGeneric::~CtrlGeneric()
 {
-    delete m_pPosition;
     if( m_pVisible )
     {
         m_pVisible->delObserver( this );
@@ -56,12 +57,21 @@ CtrlGeneric::~CtrlGeneric()
 void CtrlGeneric::setLayout( GenericLayout *pLayout,
                              const Position &rPosition )
 {
+    assert( !m_pLayout && pLayout);
+
     m_pLayout = pLayout;
-    delete m_pPosition;
     m_pPosition = new Position( rPosition );
     onPositionChange();
 }
 
+void CtrlGeneric::unsetLayout()
+{
+    assert( m_pLayout );
+
+    delete m_pPosition;
+    m_pPosition = NULL;
+    m_pLayout = NULL;
+}
 
 void CtrlGeneric::notifyLayout( int width, int height,
                                 int xOffSet, int yOffSet ) const
diff --git a/modules/gui/skins2/controls/ctrl_generic.hpp b/modules/gui/skins2/controls/ctrl_generic.hpp
index 886c27b..d1decf7 100644
--- a/modules/gui/skins2/controls/ctrl_generic.hpp
+++ b/modules/gui/skins2/controls/ctrl_generic.hpp
@@ -58,6 +58,7 @@ public:
     /// Set the position and the associated layout of the control
     virtual void setLayout( GenericLayout *pLayout,
                             const Position &rPosition );
+    virtual void unsetLayout();
 
     /// Get the position of the control in the layout, if any
     virtual const Position *getPosition() const { return m_pPosition; }
diff --git a/modules/gui/skins2/controls/ctrl_move.cpp b/modules/gui/skins2/controls/ctrl_move.cpp
index e890d16..f4cf888 100644
--- a/modules/gui/skins2/controls/ctrl_move.cpp
+++ b/modules/gui/skins2/controls/ctrl_move.cpp
@@ -80,6 +80,13 @@ void CtrlMove::setLayout( GenericLayout *pLayout, const Position &rPosition )
 }
 
 
+void CtrlMove::unsetLayout( )
+{
+    m_rCtrl.unsetLayout();
+    CtrlGeneric::unsetLayout();
+}
+
+
 const Position *CtrlMove::getPosition() const
 {
     return m_rCtrl.getPosition();
diff --git a/modules/gui/skins2/controls/ctrl_move.hpp b/modules/gui/skins2/controls/ctrl_move.hpp
index 5a4f958..67e813f 100644
--- a/modules/gui/skins2/controls/ctrl_move.hpp
+++ b/modules/gui/skins2/controls/ctrl_move.hpp
@@ -54,6 +54,7 @@ public:
     /// Set the position and the associated layout of the decorated control
     virtual void setLayout( GenericLayout *pLayout,
                             const Position &rPosition );
+    virtual void unsetLayout( );
 
     /// Get the position of the decorated control in the layout, if any
     virtual const Position *getPosition() const;
diff --git a/modules/gui/skins2/controls/ctrl_resize.cpp b/modules/gui/skins2/controls/ctrl_resize.cpp
index 5cd7070..1fb37b7 100644
--- a/modules/gui/skins2/controls/ctrl_resize.cpp
+++ b/modules/gui/skins2/controls/ctrl_resize.cpp
@@ -89,6 +89,13 @@ void CtrlResize::setLayout( GenericLayout *pLayout, const Position &rPosition )
 }
 
 
+void CtrlResize::unsetLayout()
+{
+    m_rCtrl.unsetLayout();
+    CtrlGeneric::unsetLayout();
+}
+
+
 const Position *CtrlResize::getPosition() const
 {
     return m_rCtrl.getPosition();
diff --git a/modules/gui/skins2/controls/ctrl_resize.hpp b/modules/gui/skins2/controls/ctrl_resize.hpp
index f540773..7d2d718 100644
--- a/modules/gui/skins2/controls/ctrl_resize.hpp
+++ b/modules/gui/skins2/controls/ctrl_resize.hpp
@@ -55,6 +55,7 @@ public:
     /// Set the position and the associated layout of the decorated control
     virtual void setLayout( GenericLayout *pLayout,
                             const Position &rPosition );
+    virtual void unsetLayout();
 
     /// Get the position of the decorated control in the layout, if any
     virtual const Position *getPosition() const;
diff --git a/modules/gui/skins2/controls/ctrl_video.cpp b/modules/gui/skins2/controls/ctrl_video.cpp
index 68c8e4c..484feb8 100644
--- a/modules/gui/skins2/controls/ctrl_video.cpp
+++ b/modules/gui/skins2/controls/ctrl_video.cpp
@@ -48,8 +48,6 @@ CtrlVideo::~CtrlVideo()
 {
     VarBool &rFullscreen = VlcProc::instance( getIntf() )->getFullscreenVar();
     rFullscreen.delObserver( this );
-
-    //m_pLayout->getActiveVar().delObserver( this );
 }
 
 
@@ -111,6 +109,13 @@ void CtrlVideo::setLayout( GenericLayout *pLayout,
 }
 
 
+void CtrlVideo::unsetLayout()
+{
+    m_pLayout->getActiveVar().delObserver( this );
+    CtrlGeneric::unsetLayout();
+}
+
+
 void CtrlVideo::resizeControl( int width, int height )
 {
     WindowManager &rWindowManager =
diff --git a/modules/gui/skins2/controls/ctrl_video.hpp b/modules/gui/skins2/controls/ctrl_video.hpp
index 9afd703..570bd1c 100644
--- a/modules/gui/skins2/controls/ctrl_video.hpp
+++ b/modules/gui/skins2/controls/ctrl_video.hpp
@@ -78,6 +78,7 @@ public:
     /// Set the position and the associated layout of the control
     virtual void setLayout( GenericLayout *pLayout,
                             const Position &rPosition );
+    virtual void unsetLayout();
 
     // resize the video Control
     virtual void resizeControl( int width, int height );
diff --git a/modules/gui/skins2/src/generic_layout.cpp b/modules/gui/skins2/src/generic_layout.cpp
index 8b35371..217d3b0 100644
--- a/modules/gui/skins2/src/generic_layout.cpp
+++ b/modules/gui/skins2/src/generic_layout.cpp
@@ -56,11 +56,20 @@ GenericLayout::GenericLayout( intf_thread_t *pIntf, int width, int height,
 GenericLayout::~GenericLayout()
 {
     delete m_pImage;
+
     list<Anchor*>::const_iterator it;
     for( it = m_anchorList.begin(); it != m_anchorList.end(); it++ )
     {
         delete *it;
     }
+
+    list<LayeredControl>::const_iterator iter;
+    for( iter = m_controlList.begin(); iter != m_controlList.end(); iter++ )
+    {
+        CtrlGeneric *pCtrl = (*iter).m_pControl;
+        pCtrl->unsetLayout();
+    }
+
 }
 
 




More information about the vlc-devel mailing list