extragear/multimedia/amarok/src/widgets

Mark Kretschmann kretschmann at kde.org
Fri Feb 6 07:53:07 CET 2009


SVN commit 921999 by markey:

Fix one crash on exit, caused by double-freeing QWidgets stored in a
private d-pointer class of another QWidget.

Dear Amarok devs, please don't _ever_ use d-pointers in QWidgets. Here
is an article that explains the dangers of doing so:

http://www.purinchu.net/wp/2009/02/04/another-programming-tidbit/

CCMAIL: amarok-devel at kde.org
CCBUG: 179400

 M  +176 -192  SidebarWidget.cpp  
 M  +48 -37    SidebarWidget.h  


--- trunk/extragear/multimedia/amarok/src/widgets/SidebarWidget.cpp #921998:921999
@@ -27,205 +27,13 @@
 
 #include "SvgTinter.h"
 
-
 #include <QAbstractItemDelegate>
-#include <QAction>
 #include <QApplication>
-#include <QList>
 #include <QPainter>
 #include <QTimer>
 #include <QWheelEvent>
 
 
-class SideBarWidget::Private
-{
-public:
-    QList<SideBarButton*> buttons;
-    QList<QAction*> actions;
-    QList<QAction*> shortcuts;
-    QAction *closeShortcut;
-    QList<int> visible;
-};
-
-SideBarWidget::SideBarWidget( QWidget *parent )
-    : super( parent )
-    , d( new Private )
-{
-    setContextMenuPolicy( Qt::ActionsContextMenu );
-    setSizePolicy( QSizePolicy::Fixed, QSizePolicy::Expanding );
-    d->closeShortcut = new QAction( this );
-    d->closeShortcut->setVisible( false );
-    d->closeShortcut->setShortcut( QKeySequence( Qt::CTRL | Qt::Key_0 ) );
-    addAction( d->closeShortcut );
-}
-
-SideBarWidget::~SideBarWidget()
-{
-    DEBUG_BLOCK
-
-    // Save index of active browser for session management
-    int i;
-    for( i = 0; i < d->buttons.count() && !d->buttons[i]->isChecked(); i++ ) {}
-    AmarokConfig::setActiveBrowser( i );
-
-    // Save list of visible browsers for session management
-    AmarokConfig::setVisibleBrowsers( d->visible );
-
-    AmarokConfig::self()->writeConfig();
-
-    delete d;
-}
-
-void SideBarWidget::restoreSession()
-{
-    DEBUG_BLOCK
-
-    QList<QAction*> browserActions;
-    foreach( QAction* action, d->actions )
-        if( !action->text().isEmpty() )
-            browserActions << action;
-
-    // Restore visible browsers
-    if( !AmarokConfig::visibleBrowsers().isEmpty() )
-        for( int i = 0; i < d->visible.count(); i++ )
-            if( !AmarokConfig::visibleBrowsers().contains( i ) )
-                browserActions[i]->toggle();
-
-    // Restore active browser
-    const int index = AmarokConfig::activeBrowser();
-    if( index < d->buttons.count() && !d->buttons[index]->isChecked() )
-        d->buttons[index]->click();
-}
-
-int SideBarWidget::addSideBar( const QIcon &icon, const QString &text )
-{
-    SideBarButton *b = new SideBarButton( icon, text, this );
-    connect( b, SIGNAL( clicked( bool ) ), this, SLOT( slotClicked( bool ) ) );
-    d->visible.append( d->buttons.count() );
-    d->buttons.append( b );
-
-    QAction *a = new QAction( icon, text, this );
-    a->setCheckable( true );
-    a->setChecked( true );
-    connect( a, SIGNAL( toggled( bool ) ), this, SLOT( slotSetVisible( bool ) ) );
-    addAction( a );
-    d->actions.append( a );
-
-    QAction *s = new QAction( this );
-    s->setVisible( false );
-    connect( s, SIGNAL( triggered() ), b, SLOT( click() ) );
-    s->setShortcut( QKeySequence( Qt::CTRL | ( Qt::Key_0 + d->visible.count() ) ) );
-    addAction( s );
-    d->shortcuts.append( s );
-
-    if( d->buttons.count() == 1 ) // first one
-        b->click();               // open it
-
-    return count() - 1;
-}
-
-void SideBarWidget::slotClicked( bool checked )
-{
-    if( checked )
-    {
-        int index = -1;
-        for( int i = 0, n = count(); i < n; ++i )
-        {
-            if( d->buttons[i] == sender() )
-                index = i;
-            else
-                d->buttons[i]->setChecked( false );
-        }
-        d->closeShortcut->disconnect();
-        connect( d->closeShortcut, SIGNAL( triggered() ), d->buttons[index], SLOT( click() ) );
-        emit opened( index );
-    }
-    else
-        emit closed();
-}
-
-void SideBarWidget::slotSetVisible( bool visible )
-{
-    DEBUG_BLOCK
-
-    QAction *a = (QAction *)sender();
-    const int index = d->actions.indexOf( a );
-    if( d->visible.contains( index ) == visible )
-        return;
-    d->buttons[index]->setVisible( visible );
-    if( visible )
-        d->visible.append( index );
-    else
-        d->visible.removeAll( index );
-    d->actions[ d->visible.first() ]->setEnabled( d->visible.count() != 1 );
-    qSort( d->visible );
-    if( !visible && d->buttons[index]->isChecked() )
-    {
-        d->buttons[index]->click();
-        for( int i = 0, n = d->visible.count(); i < n; ++i )
-            if( d->visible[i] != index )
-            {
-                d->buttons[ d->visible[i] ]->click();
-                break;
-            }
-    }
-    updateShortcuts();
-}
-
-int SideBarWidget::count() const
-{
-    return d->buttons.count();
-}
-
-QString SideBarWidget::text( int index ) const
-{
-    return d->buttons[index]->text();
-}
-
-QIcon SideBarWidget::icon( int index ) const
-{
-    return d->buttons[index]->icon();
-}
-
-void SideBarWidget::open( int index )
-{
-    d->buttons[ index ]->click();
-}
-
-void SideBarWidget::wheelEvent( QWheelEvent *e )
-{
-    if( !e->delta() ) // *shrug*
-        return;
-
-    int index;
-    const int n = d->visible.count();
-    int current = -1;
-    for( int i = 0; i < n; ++i )
-        if( d->buttons[ d->visible[i] ]->isChecked() )
-        {
-            index = d->visible[ qBound( 0, i - e->delta() / 120, n - 1 ) ];
-            current = i;
-            break;
-        }
-    if( current == -1 )
-        index = d->visible[ qBound( 0, ( e->delta() > 0 ? n : -1 ) - e->delta() / 120, n - 1 ) ];
-    if( current == -1 || index != d->visible[ current ] )
-        d->buttons[ index ]->click();
-}
-
-void SideBarWidget::updateShortcuts()
-{
-    for( int i = 0, n = d->shortcuts.count(); i < n; ++i )
-        d->shortcuts[i]->setShortcut( QKeySequence() );
-    for( int i = 0, n = d->visible.count(); i < n; ++i )
-        d->shortcuts[ d->visible[i] ]->setShortcut( QKeySequence( Qt::CTRL | ( Qt::Key_1 + i ) ) );
-}
-
-
-//////////////////////////////////////////////////////////////////////
-// Class SideBarButton
-//////////////////////////////////////////////////////////////////////
-
 SideBarButton::SideBarButton( const QIcon &icon, const QString &text, QWidget *parent )
     : QAbstractButton( parent )
     , m_animCount( 0 )
@@ -413,5 +221,181 @@
 }
 
 
+//////////////////////////////////////////////////////////////////////
+// Class SideBarWidget
+//////////////////////////////////////////////////////////////////////
+
+SideBarWidget::SideBarWidget( QWidget *parent )
+    : super( parent )
+{
+    setContextMenuPolicy( Qt::ActionsContextMenu );
+    setSizePolicy( QSizePolicy::Fixed, QSizePolicy::Expanding );
+    m_closeShortcut = new QAction( this );
+    m_closeShortcut->setVisible( false );
+    m_closeShortcut->setShortcut( QKeySequence( Qt::CTRL | Qt::Key_0 ) );
+    addAction( m_closeShortcut );
+}
+
+SideBarWidget::~SideBarWidget()
+{
+    DEBUG_BLOCK
+
+    // Save index of active browser for session management
+    int i;
+    for( i = 0; i < m_buttons.count() && !m_buttons[i]->isChecked(); i++ ) {}
+    AmarokConfig::setActiveBrowser( i );
+
+    // Save list of visible browsers for session management
+    AmarokConfig::setVisibleBrowsers( m_visible );
+
+    AmarokConfig::self()->writeConfig();
+}
+
+void SideBarWidget::restoreSession()
+{
+    DEBUG_BLOCK
+
+    QList<QAction*> browserActions;
+    foreach( QAction* action, m_actions )
+        if( !action->text().isEmpty() )
+            browserActions << action;
+
+    // Restore visible browsers
+    if( !AmarokConfig::visibleBrowsers().isEmpty() )
+        for( int i = 0; i < m_visible.count(); i++ )
+            if( !AmarokConfig::visibleBrowsers().contains( i ) )
+                browserActions[i]->toggle();
+
+    // Restore active browser
+    const int index = AmarokConfig::activeBrowser();
+    if( index < m_buttons.count() && !m_buttons[index]->isChecked() )
+        m_buttons[index]->click();
+}
+
+int SideBarWidget::addSideBar( const QIcon &icon, const QString &text )
+{
+    SideBarButton *b = new SideBarButton( icon, text, this );
+    connect( b, SIGNAL( clicked( bool ) ), this, SLOT( slotClicked( bool ) ) );
+    m_visible.append( m_buttons.count() );
+    m_buttons.append( b );
+
+    QAction *a = new QAction( icon, text, this );
+    a->setCheckable( true );
+    a->setChecked( true );
+    connect( a, SIGNAL( toggled( bool ) ), this, SLOT( slotSetVisible( bool ) ) );
+    addAction( a );
+    m_actions.append( a );
+
+    QAction *s = new QAction( this );
+    s->setVisible( false );
+    connect( s, SIGNAL( triggered() ), b, SLOT( click() ) );
+    s->setShortcut( QKeySequence( Qt::CTRL | ( Qt::Key_0 + m_visible.count() ) ) );
+    addAction( s );
+    m_shortcuts.append( s );
+
+    if( m_buttons.count() == 1 ) // first one
+        b->click();               // open it
+
+    return count() - 1;
+}
+
+void SideBarWidget::slotClicked( bool checked )
+{
+    if( checked )
+    {
+        int index = -1;
+        for( int i = 0, n = count(); i < n; ++i )
+        {
+            if( m_buttons[i] == sender() )
+                index = i;
+            else
+                m_buttons[i]->setChecked( false );
+        }
+        m_closeShortcut->disconnect();
+        connect( m_closeShortcut, SIGNAL( triggered() ), m_buttons[index], SLOT( click() ) );
+        emit opened( index );
+    }
+    else
+        emit closed();
+}
+
+void SideBarWidget::slotSetVisible( bool visible )
+{
+    DEBUG_BLOCK
+
+    QAction *a = (QAction *)sender();
+    const int index = m_actions.indexOf( a );
+    if( m_visible.contains( index ) == visible )
+        return;
+    m_buttons[index]->setVisible( visible );
+    if( visible )
+        m_visible.append( index );
+    else
+        m_visible.removeAll( index );
+    m_actions[ m_visible.first() ]->setEnabled( m_visible.count() != 1 );
+    qSort( m_visible );
+    if( !visible && m_buttons[index]->isChecked() )
+    {
+        m_buttons[index]->click();
+        for( int i = 0, n = m_visible.count(); i < n; ++i )
+            if( m_visible[i] != index )
+            {
+                m_buttons[ m_visible[i] ]->click();
+                break;
+            }
+    }
+    updateShortcuts();
+}
+
+int SideBarWidget::count() const
+{
+    return m_buttons.count();
+}
+
+QString SideBarWidget::text( int index ) const
+{
+    return m_buttons[index]->text();
+}
+
+QIcon SideBarWidget::icon( int index ) const
+{
+    return m_buttons[index]->icon();
+}
+
+void SideBarWidget::open( int index )
+{
+    m_buttons[ index ]->click();
+}
+
+void SideBarWidget::wheelEvent( QWheelEvent *e )
+{
+    if( !e->delta() ) // *shrug*
+        return;
+
+    int index;
+    const int n = m_visible.count();
+    int current = -1;
+    for( int i = 0; i < n; ++i )
+        if( m_buttons[ m_visible[i] ]->isChecked() )
+        {
+            index = m_visible[ qBound( 0, i - e->delta() / 120, n - 1 ) ];
+            current = i;
+            break;
+        }
+    if( current == -1 )
+        index = m_visible[ qBound( 0, ( e->delta() > 0 ? n : -1 ) - e->delta() / 120, n - 1 ) ];
+    if( current == -1 || index != m_visible[ current ] )
+        m_buttons[ index ]->click();
+}
+
+void SideBarWidget::updateShortcuts()
+{
+    for( int i = 0, n = m_shortcuts.count(); i < n; ++i )
+        m_shortcuts[i]->setShortcut( QKeySequence() );
+    for( int i = 0, n = m_visible.count(); i < n; ++i )
+        m_shortcuts[ m_visible[i] ]->setShortcut( QKeySequence( Qt::CTRL | ( Qt::Key_1 + i ) ) );
+}
+
+
 #include "SidebarWidget.moc"
 
--- trunk/extragear/multimedia/amarok/src/widgets/SidebarWidget.h #921998:921999
@@ -23,53 +23,20 @@
 
 #include "SvgHandler.h"
 
+#include <QAction>
 #include <QAbstractButton>
+#include <QList>
+
 #include <kvbox.h>
 
 class QTimer;
 
-class SideBarWidget: public KVBox
-{
-    typedef KVBox super;
-    Q_OBJECT
-    public:
-        SideBarWidget( QWidget *parent );
-        virtual ~SideBarWidget();
-
-        /* Restores visible/invisible and active/inactive state of browsers from last session */
-        void restoreSession();
-
-        int addSideBar( const QIcon &icon, const QString &text );
-        int count() const;
-        QString text( int index ) const;
-        QIcon icon( int index ) const;
-
-    public slots:
-        void open( int index );
-
-    signals:
-        void opened( int index );
-        void closed();
-
-    protected:
-        virtual void wheelEvent( QWheelEvent *e );
-
-    private slots:
-        void slotClicked( bool checked );
-        void slotSetVisible( bool visible );
-
-    private:
-        void updateShortcuts();
-        class Private;
-        Private* const d;
-};
-
-
 class SideBarButton: public QAbstractButton
 {
     Q_OBJECT
 
     typedef QAbstractButton super;
+
     public:
         SideBarButton( const QIcon &icon, const QString &text, QWidget *parent );
 
@@ -103,4 +70,48 @@
         QTimer* m_autoOpenTimer;
 };
 
+
+class SideBarWidget: public KVBox
+{
+    typedef KVBox super;
+
+    Q_OBJECT
+
+    public:
+        SideBarWidget( QWidget *parent );
+        virtual ~SideBarWidget();
+
+        /* Restores visible/invisible and active/inactive state of browsers from last session */
+        void restoreSession();
+
+        int addSideBar( const QIcon &icon, const QString &text );
+        int count() const;
+        QString text( int index ) const;
+        QIcon icon( int index ) const;
+
+    public slots:
+        void open( int index );
+
+    signals:
+        void opened( int index );
+        void closed();
+
+    protected:
+        virtual void wheelEvent( QWheelEvent *e );
+
+    private slots:
+        void slotClicked( bool checked );
+        void slotSetVisible( bool visible );
+
+    private:
+        void updateShortcuts();
+
+        QList<SideBarButton*> m_buttons;
+        QList<QAction*> m_actions;
+        QList<QAction*> m_shortcuts;
+        QAction* m_closeShortcut;
+        QList<int> m_visible;
+};
+
+
 #endif


More information about the Amarok-devel mailing list