Serialize job execution on all platforms.

On OS X, multiple dialogs pop up asking for permission when starting multiple jobs in parallel. Thus, as there are issues both with kwallet and OS X keychain, just serialize job execution on all platforms. On Windows this doesn't make much of a difference as the job impl's are synchronous anyway.
francoisferrand · Aug 6, 2012 · 4c014e1 · 4c014e1
1 parent be7f9c6
commit 4c014e1
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 120 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -48,10 +48,9 @@ if(UNIX AND NOT APPLE)
     list(APPEND qtkeychain_SOURCES keychain_dbus.cpp)
     qt4_add_dbus_interface(qtkeychain_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/org.kde.KWallet.xml kwallet_interface KWalletInterface)
     list(APPEND qtkeychain_LIBRARIES ${QT_QTDBUS_LIBRARY})
-    list(APPEND qtkeychain_EXTRA_MOC_HEADERS keychain_dbus_p.h)
 endif()
 
-QT4_WRAP_CPP(qtkeychain_MOC_OUTFILES keychain.h keychain_p.h ${qtkeychain_EXTRA_MOC_HEADERS})
+QT4_WRAP_CPP(qtkeychain_MOC_OUTFILES keychain.h keychain_p.h)
 
 if(NOT QTKEYCHAIN_STATIC)
     add_library(qtkeychain SHARED ${qtkeychain_SOURCES} ${qtkeychain_MOC_OUTFILES})

diff --git a/keychain.cpp b/keychain.cpp
@@ -106,7 +106,7 @@ void ReadPasswordJob::setKey( const QString& key ) {
 }
 
 void ReadPasswordJob::doStart() {
-    d->doStart();
+    JobExecutor::instance()->enqueue( this );
 }
 
 WritePasswordJob::WritePasswordJob( const QString& service, QObject* parent )
@@ -137,7 +137,7 @@ void WritePasswordJob::setTextData( const QString& data ) {
 }
 
 void WritePasswordJob::doStart() {
-    d->doStart();
+    JobExecutor::instance()->enqueue( this );
 }
 
 DeletePasswordJob::DeletePasswordJob( const QString& service, QObject* parent )
@@ -171,3 +171,57 @@ void DeletePasswordJobPrivate::jobFinished( Job* job ) {
     q->setErrorString( job->errorString() );
     q->emitFinished();
 }
+
+JobExecutor::JobExecutor()
+    : QObject( 0 )
+    , m_runningJob( 0 )
+{
+}
+
+void JobExecutor::enqueue( Job* job ) {
+    m_queue.append( job );
+    startNextIfNoneRunning();
+}
+
+void JobExecutor::startNextIfNoneRunning() {
+    if ( m_queue.isEmpty() || m_runningJob )
+        return;
+    QPointer<Job> next;
+    while ( !next && !m_queue.isEmpty() ) {
+        next = m_queue.first();
+        m_queue.pop_front();
+    }
+    if ( next ) {
+        connect( next, SIGNAL(finished(QKeychain::Job*)), this, SLOT(jobFinished(QKeychain::Job*)) );
+        connect( next, SIGNAL(destroyed(QObject*)), this, SLOT(jobDestroyed(QObject*)) );
+        m_runningJob = next;
+        if ( ReadPasswordJob* rpj = qobject_cast<ReadPasswordJob*>( m_runningJob ) )
+            rpj->d->scheduledStart();
+        else if ( WritePasswordJob* wpj = qobject_cast<WritePasswordJob*>( m_runningJob) )
+            wpj->d->scheduledStart();
+    }
+}
+
+void JobExecutor::jobDestroyed( QObject* object ) {
+    Q_UNUSED( object ) // for release mode
+    Q_ASSERT( object == m_runningJob );
+    m_runningJob->disconnect( this );
+    m_runningJob = 0;
+    startNextIfNoneRunning();
+}
+
+void JobExecutor::jobFinished( Job* job ) {
+    Q_UNUSED( job ) // for release mode
+    Q_ASSERT( job == m_runningJob );
+    m_runningJob->disconnect( this );
+    m_runningJob = 0;
+    startNextIfNoneRunning();
+}
+
+JobExecutor* JobExecutor::s_instance = 0;
+
+JobExecutor* JobExecutor::instance() {
+    if ( !s_instance )
+        s_instance = new JobExecutor;
+    return s_instance;
+}
diff --git a/keychain_dbus.cpp b/keychain_dbus.cpp
@@ -7,72 +7,13 @@
  * details, check the accompanying file 'COPYING'.                            *
  *****************************************************************************/
 #include "keychain_p.h"
-#include "keychain_dbus_p.h"
 
 #include <QSettings>
 
 #include <auto_ptr.h>
 
 using namespace QKeychain;
 
-JobExecutor::JobExecutor()
-    : QObject( 0 )
-    , m_runningJob( 0 )
-{
-}
-
-void JobExecutor::enqueue( Job* job ) {
-    m_queue.append( job );
-    startNextIfNoneRunning();
-}
-
-void JobExecutor::startNextIfNoneRunning() {
-    if ( m_queue.isEmpty() || m_runningJob )
-        return;
-    QPointer<Job> next;
-    while ( !next && !m_queue.isEmpty() ) {
-        next = m_queue.first();
-        m_queue.pop_front();
-    }
-    if ( next ) {
-        connect( next, SIGNAL(finished(QKeychain::Job*)), this, SLOT(jobFinished(QKeychain::Job*)) );
-        connect( next, SIGNAL(destroyed(QObject*)), this, SLOT(jobDestroyed(QObject*)) );
-        m_runningJob = next;
-        if ( ReadPasswordJob* rpj = qobject_cast<ReadPasswordJob*>( m_runningJob ) )
-            rpj->d->scheduledStart();
-        else if ( WritePasswordJob* wpj = qobject_cast<WritePasswordJob*>( m_runningJob) )
-            wpj->d->scheduledStart();
-    }
-}
-
-void JobExecutor::jobDestroyed( QObject* object ) {
-    Q_UNUSED( object ) // for release mode
-    Q_ASSERT( object == m_runningJob );
-    m_runningJob->disconnect( this );
-    m_runningJob = 0;
-    startNextIfNoneRunning();
-}
-
-void JobExecutor::jobFinished( Job* job ) {
-    Q_UNUSED( job ) // for release mode
-    Q_ASSERT( job == m_runningJob );
-    m_runningJob->disconnect( this );
-    m_runningJob = 0;
-    startNextIfNoneRunning();
-}
-
-JobExecutor* JobExecutor::s_instance = 0;
-
-JobExecutor* JobExecutor::instance() {
-    if ( !s_instance )
-        s_instance = new JobExecutor;
-    return s_instance;
-}
-
-void ReadPasswordJobPrivate::doStart() {
-    JobExecutor::instance()->enqueue( q );
-}
-
 void ReadPasswordJobPrivate::scheduledStart() {
     iface = new org::kde::KWallet( QLatin1String("org.kde.kwalletd"), QLatin1String("/modules/kwalletd"), QDBusConnection::sessionBus(), this );
     const QDBusPendingReply<int> reply = iface->open( QLatin1String("kdewallet"), 0, q->service() );
@@ -187,10 +128,6 @@ void ReadPasswordJobPrivate::kwalletReadFinished( QDBusPendingCallWatcher* watch
     q->emitFinished();
 }
 
-void WritePasswordJobPrivate::doStart() {
-    JobExecutor::instance()->enqueue( q );
-}
-
 void WritePasswordJobPrivate::scheduledStart() {
     iface = new org::kde::KWallet( QLatin1String("org.kde.kwalletd"), QLatin1String("/modules/kwalletd"), QDBusConnection::sessionBus(), this );
     const QDBusPendingReply<int> reply = iface->open( QLatin1String("kdewallet"), 0, q->service() );

diff --git a/keychain_dbus_p.h b/keychain_dbus_p.h
diff --git a/keychain_mac.cpp b/keychain_mac.cpp
@@ -61,7 +61,7 @@ static OSStatus readPw( QByteArray* pw,
     return ret;
 }
 
-void ReadPasswordJobPrivate::doStart()
+void ReadPasswordJobPrivate::scheduledStart()
 {
     QString errorString;
     Error error = NoError;
@@ -141,7 +141,7 @@ static QKeychain::Error writeEntryImpl( const QString& service,
     return NoError;
 }
 
-void WritePasswordJobPrivate::doStart()
+void WritePasswordJobPrivate::scheduledStart()
 {
     QString errorString;
     Error error = NoError;

diff --git a/keychain_p.h b/keychain_p.h
@@ -13,6 +13,7 @@
 #include <QObject>
 #include <QPointer>
 #include <QSettings>
+#include <QVector>
 
 #if defined(Q_OS_UNIX) && !defined(Q_WS_MAC)
 
@@ -52,7 +53,8 @@ class ReadPasswordJobPrivate : public QObject {
     Q_OBJECT
 public:
     explicit ReadPasswordJobPrivate( ReadPasswordJob* qq ) : q( qq ), walletHandle( 0 ), dataType( Text ) {}
-    void doStart();
+    void scheduledStart();
+
     ReadPasswordJob* const q;
     QByteArray data;
     QString key;
@@ -66,7 +68,6 @@ class ReadPasswordJobPrivate : public QObject {
 #if defined(Q_OS_UNIX) && !defined(Q_WS_MAC)
     org::kde::KWallet* iface;
     friend class QKeychain::JobExecutor;
-    void scheduledStart();
 
 private Q_SLOTS:
     void kwalletOpenFinished( QDBusPendingCallWatcher* watcher );
@@ -85,7 +86,8 @@ class WritePasswordJobPrivate : public QObject {
     Q_OBJECT
 public:
     explicit WritePasswordJobPrivate( WritePasswordJob* qq ) : q( qq ), mode( Delete ) {}
-    void doStart();
+    void scheduledStart();
+
     enum Mode {
         Delete,
         Text,
@@ -100,7 +102,7 @@ class WritePasswordJobPrivate : public QObject {
 #if defined(Q_OS_UNIX) && !defined(Q_WS_MAC)
     org::kde::KWallet* iface;
     friend class QKeychain::JobExecutor;
-    void scheduledStart();
+
 
 private Q_SLOTS:
     void kwalletOpenFinished( QDBusPendingCallWatcher* watcher );
@@ -123,6 +125,28 @@ private Q_SLOTS:
     void jobFinished( QKeychain::Job* );
 };
 
+class JobExecutor : public QObject {
+    Q_OBJECT
+public:
+
+    static JobExecutor* instance();
+
+    void enqueue( Job* job );
+
+private:
+    explicit JobExecutor();
+    void startNextIfNoneRunning();
+
+private Q_SLOTS:
+    void jobFinished( QKeychain::Job* );
+    void jobDestroyed( QObject* object );
+
+private:
+    static JobExecutor* s_instance;
+    Job* m_runningJob;
+    QVector<QPointer<Job> > m_queue;
+};
+
 }
 
 #endif // KEYCHAIN_P_H
diff --git a/keychain_win.cpp b/keychain_win.cpp
@@ -17,7 +17,7 @@
 
 using namespace QKeychain;
 
-void ReadPasswordJobPrivate::doStart() {
+void ReadPasswordJobPrivate::scheduledStart() {
     //Use settings member if there, create local settings object if not
     std::auto_ptr<QSettings> local( !q->settings() ? new QSettings( q->service() ) : 0 );
     QSettings* actual = q->settings() ? q->settings() : local.get();
@@ -52,7 +52,7 @@ void ReadPasswordJobPrivate::doStart() {
     q->emitFinished();
 }
 
-void WritePasswordJobPrivate::doStart() {
+void WritePasswordJobPrivate::scheduledStart() {
     if ( mode == Delete ) {
         //Use settings member if there, create local settings object if not
         std::auto_ptr<QSettings> local( !q->settings() ? new QSettings( q->service() ) : 0 );