Refactor a ton of stuff. No crashes, but I'm not convinced it all works properly

src/libtomahawk/tomahawksettings.cpp

@@ -392,17 +392,17 @@ TomahawkSettings::setProxyPassword( const QString& password )
 }
 
 
-int
+QNetworkProxy::ProxyType
 TomahawkSettings::proxyType() const
 {
-    return value( "network/proxy/type", QNetworkProxy::NoProxy ).toInt();
+    return static_cast< QNetworkProxy::ProxyType>( value( "network/proxy/type", QNetworkProxy::NoProxy ).toInt() );
 }
 
 
 void
-TomahawkSettings::setProxyType( const int type )
+TomahawkSettings::setProxyType( const QNetworkProxy::ProxyType type )
 {
-    setValue( "network/proxy/type", type );
+    setValue( "network/proxy/type", static_cast< uint >( type ) );
 }
 
 

src/libtomahawk/tomahawksettings.h

@@ -26,6 +26,8 @@
 #include "AtticaManager.h"
 #include "playlist.h"
 
+#include <QtNetwork/QNetworkProxy>
+
 /**
  * Convenience wrapper around QSettings for tomahawk-specific config
  */
@@ -142,8 +144,8 @@ public:
     QString proxyPassword() const;
     void setProxyPassword( const QString &password );
 
-    int proxyType() const;
+    QNetworkProxy::ProxyType proxyType() const;
-    void setProxyType( const int type );
+    void setProxyType( const QNetworkProxy::ProxyType type );
 
     bool proxyDns() const;
     void setProxyDns( bool lookupViaProxy );

src/libtomahawk/utils/tomahawkutils.cpp

@@ -477,7 +477,7 @@ NetworkProxyFactory::proxyForQuery( const QNetworkProxyQuery& query )
 {
     Q_UNUSED( query );
     QList< QNetworkProxy > proxies;
-    proxies << QNetworkProxy( QNetworkProxy::NoProxy );
+    proxies << QNetworkProxy( QNetworkProxy::DefaultProxy ) << QNetworkProxy( QNetworkProxy::NoProxy );
     return proxies;
 }
 
@@ -500,14 +500,14 @@ void
 NetworkProxyFactory::setNoProxyHosts( const QStringList& hosts )
 {
     QStringList newList;
-    qDebug() << Q_FUNC_INFO << "No-proxy hosts:" << hosts;
+    tDebug() << Q_FUNC_INFO << "No-proxy hosts:" << hosts;
     foreach( QString host, hosts )
     {
         QString munge = host.simplified();
         newList << munge;
         //TODO: wildcard support
     }
-    qDebug() << Q_FUNC_INFO << "New no-proxy hosts:" << newList;
+    tDebug() << Q_FUNC_INFO << "New no-proxy hosts:" << newList;
     m_noProxyHosts = newList;
 }
 
@@ -518,13 +518,28 @@ NetworkProxyFactory::setProxy( const QNetworkProxy& proxy )
     m_proxy = proxy;
     if ( !TomahawkSettings::instance()->proxyDns() )
         m_proxy.setCapabilities( QNetworkProxy::TunnelingCapability | QNetworkProxy::ListeningCapability | QNetworkProxy::UdpTunnelingCapability );
-    qDebug() << Q_FUNC_INFO << "Proxy using host" << proxy.hostName() << "and port" << proxy.port();
+    tDebug() << Q_FUNC_INFO << "Proxy using host" << proxy.hostName() << "and port" << proxy.port();
-    qDebug() << Q_FUNC_INFO << "setting proxy to use proxy DNS?" << (TomahawkSettings::instance()->proxyDns() ? "true" : "false");
+    tDebug() << Q_FUNC_INFO << "setting proxy to use proxy DNS?" << (TomahawkSettings::instance()->proxyDns() ? "true" : "false");
 }
 
 
-bool NetworkProxyFactory::operator==( const NetworkProxyFactory& other )
+NetworkProxyFactory&
+NetworkProxyFactory::operator=( const NetworkProxyFactory& rhs )
 {
+    tDebug() << Q_FUNC_INFO;
+    if ( this != &rhs )
+    {
+        m_proxy = QNetworkProxy( rhs.m_proxy );
+        m_noProxyHosts = QStringList( rhs.m_noProxyHosts );
+    }
+
+    return *this;
+}
+
+
+bool NetworkProxyFactory::operator==( const NetworkProxyFactory& other ) const
+{
+    tDebug() << Q_FUNC_INFO;
     if ( m_noProxyHosts != other.m_noProxyHosts || m_proxy != other.m_proxy )
         return false;
 
@@ -539,7 +554,8 @@ NetworkProxyFactory*
 proxyFactory( bool noMutexLocker )
 {
     // Don't lock if being called from nam()
-    QMutexLocker locker( noMutexLocker ? new QMutex() : &s_namAccessMutex );
+    QMutex otherMutex;
+    QMutexLocker locker( noMutexLocker ? &otherMutex : &s_namAccessMutex );
     
     if ( s_threadProxyFactoryHash.contains( QThread::currentThread() ) )
         return s_threadProxyFactoryHash[ QThread::currentThread() ];
@@ -550,14 +566,10 @@ proxyFactory( bool noMutexLocker )
     // create a new proxy factory for this thread
     TomahawkUtils::NetworkProxyFactory *mainProxyFactory = s_threadProxyFactoryHash[ TOMAHAWK_APPLICATION::instance()->thread() ];
     TomahawkUtils::NetworkProxyFactory *newProxyFactory = new TomahawkUtils::NetworkProxyFactory();
-    newProxyFactory->setNoProxyHosts( mainProxyFactory->noProxyHosts() );
+    *newProxyFactory = *mainProxyFactory;
-    newProxyFactory->setProxy( QNetworkProxy ( mainProxyFactory->proxy() ) );
 
     s_threadProxyFactoryHash[ QThread::currentThread() ] = newProxyFactory;
 
-    if ( s_threadNamHash.contains( QThread::currentThread() ) )
-        s_threadNamHash[ QThread::currentThread() ]->setProxyFactory( newProxyFactory );
-    
     return newProxyFactory;
 }
 
@@ -570,40 +582,33 @@ setProxyFactory( NetworkProxyFactory* factory )
 
     if ( !s_threadProxyFactoryHash.contains( TOMAHAWK_APPLICATION::instance()->thread() ) )
         return;
-    
+
+    TomahawkUtils::NetworkProxyFactory *oldProxyFactory = s_threadProxyFactoryHash[ QThread::currentThread() ];
     if ( QThread::currentThread() == TOMAHAWK_APPLICATION::instance()->thread() )
     {
         // If setting new values on the main thread, clear the other entries
         // so that on next access new ones will be created with new proper values
         NetworkProxyFactory::setApplicationProxyFactory( factory );
-        s_threadProxyFactoryHash.clear();
+        foreach( QThread* thread, s_threadProxyFactoryHash.keys() )
+        {
+            if ( thread != QThread::currentThread() )
+            {
+                TomahawkUtils::NetworkProxyFactory *currFactory = s_threadProxyFactoryHash[ thread ];
+                *currFactory = *factory;
+            }
+        }
     }
 
-    // Yes, we really do need to create a new one, or we will crash when we set the factory
+    *s_threadProxyFactoryHash[ QThread::currentThread() ] = *factory;
-    // in the QNAM, because it deletes the old one -- and guess what happens when the old one is
-    // the same as the new one?
-    TomahawkUtils::NetworkProxyFactory *mainProxyFactory = factory;
-    TomahawkUtils::NetworkProxyFactory *newProxyFactory = new TomahawkUtils::NetworkProxyFactory();
-    newProxyFactory->setNoProxyHosts( mainProxyFactory->noProxyHosts() );
-    newProxyFactory->setProxy( QNetworkProxy ( mainProxyFactory->proxy() ) );
-
-    s_threadProxyFactoryHash[ QThread::currentThread() ] = newProxyFactory;
-
-    if ( s_threadNamHash.contains( QThread::currentThread() ) )
-        s_threadNamHash[ QThread::currentThread() ]->setProxyFactory( newProxyFactory );
 }
 
 
 QNetworkAccessManager*
-nam()
+nam()   
 {
     QMutexLocker locker( &s_namAccessMutex );
     if ( s_threadNamHash.contains(  QThread::currentThread() ) )
-    {
-        // Ensure the proxy values are up to date
-        Q_UNUSED( proxyFactory( true ) );
         return s_threadNamHash[ QThread::currentThread() ];
-    }
 
     if ( !s_threadNamHash.contains( TOMAHAWK_APPLICATION::instance()->thread() ) )
         return 0;
@@ -619,10 +624,9 @@ nam()
 
     newNam->setConfiguration( QNetworkConfiguration( mainNam->configuration() ) );
     newNam->setNetworkAccessible( mainNam->networkAccessible() );
+    newNam->setProxyFactory( proxyFactory( true ) );
     
     s_threadNamHash[ QThread::currentThread() ] = newNam;
-    //get the proxy info, must be done *after* setting the new thread in the hash
-    Q_UNUSED( proxyFactory( true ) );
 
     return newNam;
 }
@@ -657,13 +661,6 @@ setNam( QNetworkAccessManager* nam )
         return;
     }
 
-    if ( QThread::currentThread() == TOMAHAWK_APPLICATION::instance()->thread() )
-    {
-        // If setting new values on the main thread, clear the other entries
-        // so that on next access new ones will be created with new proper values
-        s_threadNamHash.clear();
-        s_threadProxyFactoryHash.clear();
-    }
     s_threadNamHash[ QThread::currentThread() ] = nam;
 }
 

src/libtomahawk/utils/tomahawkutils.h

@@ -61,12 +61,13 @@ namespace TomahawkUtils
         virtual QList< QNetworkProxy > queryProxy( const QNetworkProxyQuery & query = QNetworkProxyQuery() );
         static QList< QNetworkProxy > proxyForQuery( const QNetworkProxyQuery & query );
 
-        void setNoProxyHosts( const QStringList &hosts );
+        virtual void setNoProxyHosts( const QStringList &hosts );
-        QStringList noProxyHosts() const { return m_noProxyHosts; }
+        virtual QStringList noProxyHosts() const { return m_noProxyHosts; }
-        void setProxy( const QNetworkProxy &proxy );
+        virtual void setProxy( const QNetworkProxy &proxy );
-        QNetworkProxy proxy() { return m_proxy; }
+        virtual QNetworkProxy proxy() { return m_proxy; }
 
-        bool operator==( const NetworkProxyFactory &other );
+        virtual NetworkProxyFactory& operator=( const NetworkProxyFactory &rhs );
+        virtual bool operator==( const NetworkProxyFactory &other ) const;
 
     private:
         QStringList m_noProxyHosts;

src/settingsdialog.cpp

@@ -876,6 +876,8 @@ ProxyDialog::saveSettings()
 {
     qDebug() << Q_FUNC_INFO;
 
+    QNetworkProxy::ProxyType type = static_cast< QNetworkProxy::ProxyType>( m_backwardMap[ ui->typeBox->currentIndex() ] );
+    
     //First set settings
     TomahawkSettings* s = TomahawkSettings::instance();
     s->setProxyHost( ui->hostLineEdit->text() );
@@ -885,21 +887,26 @@ ProxyDialog::saveSettings()
     s->setProxyNoProxyHosts( ui->noHostLineEdit->text() );
     s->setProxyUsername( ui->userLineEdit->text() );
     s->setProxyPassword( ui->passwordLineEdit->text() );
-    s->setProxyType( m_backwardMap[ ui->typeBox->itemData( ui->typeBox->currentIndex() ).toInt() ] );
+    s->setProxyType( type );
     s->setProxyDns( ui->checkBoxUseProxyForDns->checkState() == Qt::Checked );
-
+    s->sync();
-    if( s->proxyHost().isEmpty() )
+    
-        return;
+    TomahawkUtils::NetworkProxyFactory* proxyFactory = TomahawkUtils::proxyFactory();
-
-    TomahawkUtils::NetworkProxyFactory* proxyFactory = new TomahawkUtils::NetworkProxyFactory();
     tDebug() << Q_FUNC_INFO << "Got proxyFactory: " << proxyFactory;
-    QNetworkProxy proxy( static_cast<QNetworkProxy::ProxyType>(s->proxyType()), s->proxyHost(), s->proxyPort(), s->proxyUsername(), s->proxyPassword() );
+    if ( type == QNetworkProxy::NoProxy )
-    proxyFactory->setProxy( proxy );
-    if ( !ui->noHostLineEdit->text().isEmpty() )
     {
-        tDebug() << Q_FUNC_INFO << "hosts line edit is " << ui->noHostLineEdit->text();
+        tDebug() << Q_FUNC_INFO << "Got NoProxy selected";
-        tDebug() << Q_FUNC_INFO << "split hosts line edit is " << ui->noHostLineEdit->text().split( ' ', QString::SkipEmptyParts );
+        proxyFactory->setProxy( QNetworkProxy::NoProxy );
-        proxyFactory->setNoProxyHosts( ui->noHostLineEdit->text().split( ' ', QString::SkipEmptyParts ) );
+    }
+    else
+    {
+        tDebug() << Q_FUNC_INFO << "Got Socks5Proxy selected";
+        proxyFactory->setProxy( QNetworkProxy( type, s->proxyHost(), s->proxyPort(), s->proxyUsername(), s->proxyPassword() ) );
+        if ( !ui->noHostLineEdit->text().isEmpty() )
+        {
+            tDebug() << Q_FUNC_INFO << "hosts line edit is " << ui->noHostLineEdit->text();
+            tDebug() << Q_FUNC_INFO << "split hosts line edit is " << ui->noHostLineEdit->text().split( ' ', QString::SkipEmptyParts );
+            proxyFactory->setNoProxyHosts( ui->noHostLineEdit->text().split( ' ', QString::SkipEmptyParts ) );
+        }
     }
-    TomahawkUtils::setProxyFactory( proxyFactory );
 }