From ddb2484ad76a1ef433aa84b23f0dffc6a703795f Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 5 Jan 2015 15:25:41 +0100 Subject: [PATCH] Pass around scriptobject_ptr instead of ScriptObject* --- src/libtomahawk/Typedefs.h | 6 +++++ src/libtomahawk/resolvers/JSAccount.cpp | 4 ++-- src/libtomahawk/resolvers/JSAccount.h | 4 ++-- src/libtomahawk/resolvers/JSResolver.cpp | 2 +- src/libtomahawk/resolvers/ScriptAccount.cpp | 23 +++++++++++++------ src/libtomahawk/resolvers/ScriptAccount.h | 12 ++++++---- .../resolvers/ScriptInfoPlugin.cpp | 4 ++-- src/libtomahawk/resolvers/ScriptInfoPlugin.h | 2 +- src/libtomahawk/resolvers/ScriptJob.cpp | 6 ++--- src/libtomahawk/resolvers/ScriptJob.h | 8 ++++--- .../resolvers/ScriptLinkGeneratorPlugin.cpp | 4 ++-- .../resolvers/ScriptLinkGeneratorPlugin.h | 2 +- .../resolvers/ScriptLinkGeneratorPlugin_p.h | 4 ++-- src/libtomahawk/resolvers/ScriptObject.cpp | 22 ++++++++++++++++-- src/libtomahawk/resolvers/ScriptObject.h | 5 ++++ src/libtomahawk/resolvers/ScriptObject_p.h | 1 + src/libtomahawk/resolvers/ScriptPlugin.cpp | 4 ++-- src/libtomahawk/resolvers/ScriptPlugin.h | 6 ++--- src/libtomahawk/resolvers/SyncScriptJob.cpp | 4 +++- 19 files changed, 84 insertions(+), 39 deletions(-) diff --git a/src/libtomahawk/Typedefs.h b/src/libtomahawk/Typedefs.h index c81109fbb..f4cbc3d31 100644 --- a/src/libtomahawk/Typedefs.h +++ b/src/libtomahawk/Typedefs.h @@ -55,6 +55,7 @@ namespace Tomahawk class GeneratorInterface; class PeerInfo; class DatabaseCommand; + class ScriptObject; typedef QSharedPointer collection_ptr; typedef QSharedPointer playlist_ptr; @@ -82,6 +83,11 @@ namespace Tomahawk typedef QSharedPointer dyncontrol_ptr; typedef QSharedPointer geninterface_ptr; + + // Scripting + typedef QSharedPointer< ScriptObject > scriptobject_ptr; + typedef QWeakPointer< ScriptObject > scriptobject_wptr; + // let's keep these typesafe, they are different kinds of GUID: typedef QString QID; //query id typedef QString RID; //result id diff --git a/src/libtomahawk/resolvers/JSAccount.cpp b/src/libtomahawk/resolvers/JSAccount.cpp index fae5af4f1..44c2208d7 100644 --- a/src/libtomahawk/resolvers/JSAccount.cpp +++ b/src/libtomahawk/resolvers/JSAccount.cpp @@ -53,7 +53,7 @@ JSAccount::setResolver( JSResolver* resolver ) void -JSAccount::scriptPluginFactory( const QString& type, ScriptObject* object ) +JSAccount::scriptPluginFactory( const QString& type, const scriptobject_ptr& object ) { if ( type == "resolver" ) { @@ -151,7 +151,7 @@ JSAccount::startJob( ScriptJob* scriptJob ) const QVariant -JSAccount::syncInvoke( ScriptObject* scriptObject, const QString& methodName, const QVariantMap& arguments ) +JSAccount::syncInvoke( const scriptobject_ptr& scriptObject, const QString& methodName, const QVariantMap& arguments ) { QString eval = QString( "Tomahawk.PluginManager.invokeSync(" diff --git a/src/libtomahawk/resolvers/JSAccount.h b/src/libtomahawk/resolvers/JSAccount.h index 42cd778b6..047eb11e1 100644 --- a/src/libtomahawk/resolvers/JSAccount.h +++ b/src/libtomahawk/resolvers/JSAccount.h @@ -45,7 +45,7 @@ public: JSAccount( const QString& name ); void startJob( ScriptJob* scriptJob ) override; - const QVariant syncInvoke( ScriptObject* scriptObject, const QString& methodName, const QVariantMap& arguments ) override; + const QVariant syncInvoke( const scriptobject_ptr& scriptObject, const QString& methodName, const QVariantMap& arguments ) override; const QString name() const; @@ -70,7 +70,7 @@ public: void addToJavaScriptWindowObject( const QString& name, QObject* object ); void setResolver( JSResolver* resolver ); - void scriptPluginFactory( const QString& type, ScriptObject* object ) override; + void scriptPluginFactory( const QString& type, const scriptobject_ptr& object ) override; static QString serializeQVariantMap(const QVariantMap& map); diff --git a/src/libtomahawk/resolvers/JSResolver.cpp b/src/libtomahawk/resolvers/JSResolver.cpp index 7ea293eb0..59e819442 100644 --- a/src/libtomahawk/resolvers/JSResolver.cpp +++ b/src/libtomahawk/resolvers/JSResolver.cpp @@ -59,7 +59,7 @@ using namespace Tomahawk; JSResolver::JSResolver( const QString& accountId, const QString& scriptPath, const QStringList& additionalScriptPaths ) : Tomahawk::ExternalResolverGui( scriptPath ) - , ScriptPlugin( nullptr ) + , ScriptPlugin( scriptobject_ptr() ) , d_ptr( new JSResolverPrivate( this, accountId, scriptPath, additionalScriptPaths ) ) { Q_D( JSResolver ); diff --git a/src/libtomahawk/resolvers/ScriptAccount.cpp b/src/libtomahawk/resolvers/ScriptAccount.cpp index cdf19ebef..778b12e9b 100644 --- a/src/libtomahawk/resolvers/ScriptAccount.cpp +++ b/src/libtomahawk/resolvers/ScriptAccount.cpp @@ -20,6 +20,7 @@ #include "ScriptObject.h" #include "../utils/Logger.h" +#include "../Typedefs.h" // TODO: register factory methods instead of hardcoding all plugin types in here #include "../utils/LinkGenerator.h" @@ -46,7 +47,7 @@ requestIdGenerator() ScriptJob* -ScriptAccount::invoke( ScriptObject* scriptObject, const QString& methodName, const QVariantMap& arguments ) +ScriptAccount::invoke( const scriptobject_ptr& scriptObject, const QString& methodName, const QVariantMap& arguments ) { QString requestId = requestIdGenerator(); @@ -88,11 +89,12 @@ ScriptAccount::reportScriptJobResult( const QVariantMap& result ) void ScriptAccount::registerScriptPlugin( const QString& type, const QString& objectId ) { - ScriptObject* object = m_objects.value( objectId ); + scriptobject_ptr object = m_objects.value( objectId ); if( !object ) { - object = new ScriptObject( objectId, this ); - connect( object, SIGNAL( destroyed( QObject* ) ), SLOT( onScriptObjectDeleted( QObject* ) ) ); + object = scriptobject_ptr( new ScriptObject( objectId, this ), &ScriptObject::deleteLater ); + object->setWeakRef( object.toWeakRef() ); + connect( object.data(), SIGNAL( destroyed( QObject* ) ), SLOT( onScriptObjectDeleted() ) ); m_objects.insert( objectId, object ); } @@ -101,14 +103,21 @@ ScriptAccount::registerScriptPlugin( const QString& type, const QString& objectI void -ScriptAccount::onScriptObjectDeleted( QObject* scriptObject ) +ScriptAccount::onScriptObjectDeleted() { - m_objects.remove( m_objects.key( static_cast< ScriptObject* >( scriptObject ) ) ); + foreach( const scriptobject_ptr& object, m_objects.values() ) + { + if ( object.isNull() ) + { + m_objects.remove( m_objects.key( object ) ); + break; + } + } } void -ScriptAccount::scriptPluginFactory( const QString& type, ScriptObject* object ) +ScriptAccount::scriptPluginFactory( const QString& type, const scriptobject_ptr& object ) { if ( type == "linkGenerator" ) { diff --git a/src/libtomahawk/resolvers/ScriptAccount.h b/src/libtomahawk/resolvers/ScriptAccount.h index a95ddcca2..6e9a0ef9a 100644 --- a/src/libtomahawk/resolvers/ScriptAccount.h +++ b/src/libtomahawk/resolvers/ScriptAccount.h @@ -21,6 +21,8 @@ #ifndef TOMAHAWK_SCRIPTACCOUNT_H #define TOMAHAWK_SCRIPTACCOUNT_H +#include "../Typedefs.h" + #include #include @@ -42,25 +44,25 @@ public: ScriptAccount( const QString& name ); virtual ~ScriptAccount() {} - ScriptJob* invoke( ScriptObject* scriptObject, const QString& methodName, const QVariantMap& arguments ); - virtual const QVariant syncInvoke( ScriptObject* scriptObject, const QString& methodName, const QVariantMap& arguments ) = 0; + ScriptJob* invoke( const scriptobject_ptr& scriptObject, const QString& methodName, const QVariantMap& arguments ); + virtual const QVariant syncInvoke( const scriptobject_ptr& scriptObject, const QString& methodName, const QVariantMap& arguments ) = 0; virtual void startJob( ScriptJob* scriptJob ) = 0; void reportScriptJobResult( const QVariantMap& result ); void registerScriptPlugin( const QString& type, const QString& objectId ); - virtual void scriptPluginFactory( const QString& type, ScriptObject* object ); + virtual void scriptPluginFactory( const QString& type, const scriptobject_ptr& object ); private slots: void onJobDeleted( const QString& jobId ); - void onScriptObjectDeleted( QObject* scriptObject ); + void onScriptObjectDeleted(); private: // TODO: pimple, might be renamed before tho QString m_name; QHash< QString, ScriptJob* > m_jobs; - QHash< QString, ScriptObject* > m_objects; + QHash< QString, scriptobject_ptr > m_objects; }; } // ns: Tomahawk diff --git a/src/libtomahawk/resolvers/ScriptInfoPlugin.cpp b/src/libtomahawk/resolvers/ScriptInfoPlugin.cpp index f7763f483..7fb7c3158 100644 --- a/src/libtomahawk/resolvers/ScriptInfoPlugin.cpp +++ b/src/libtomahawk/resolvers/ScriptInfoPlugin.cpp @@ -28,7 +28,7 @@ using namespace Tomahawk; -ScriptInfoPlugin::ScriptInfoPlugin( ScriptObject* scriptObject, const QString& name ) +ScriptInfoPlugin::ScriptInfoPlugin( const scriptobject_ptr& scriptObject, const QString& name ) : d_ptr( new ScriptInfoPluginPrivate( this ) ) , ScriptPlugin( scriptObject ) { @@ -40,7 +40,7 @@ ScriptInfoPlugin::ScriptInfoPlugin( ScriptObject* scriptObject, const QString& n setFriendlyName( QString( "ScriptInfoPlugin: %1" ).arg( name ) ); - connect( scriptObject, SIGNAL( destroyed( QObject* ) ), SLOT( onScriptObjectDeleted() ) ); + connect( scriptObject.data(), SIGNAL( destroyed( QObject* ) ), SLOT( onScriptObjectDeleted() ) ); } diff --git a/src/libtomahawk/resolvers/ScriptInfoPlugin.h b/src/libtomahawk/resolvers/ScriptInfoPlugin.h index 9365748b8..fff6051b4 100644 --- a/src/libtomahawk/resolvers/ScriptInfoPlugin.h +++ b/src/libtomahawk/resolvers/ScriptInfoPlugin.h @@ -41,7 +41,7 @@ public: /** * @param id unique identifier to identify an infoplugin in its scope */ - ScriptInfoPlugin( ScriptObject* scriptObject, const QString& name ); + ScriptInfoPlugin( const scriptobject_ptr& scriptObject, const QString& name ); virtual ~ScriptInfoPlugin(); protected slots: diff --git a/src/libtomahawk/resolvers/ScriptJob.cpp b/src/libtomahawk/resolvers/ScriptJob.cpp index 5b4b27dad..bc0ac82fa 100644 --- a/src/libtomahawk/resolvers/ScriptJob.cpp +++ b/src/libtomahawk/resolvers/ScriptJob.cpp @@ -24,8 +24,8 @@ using namespace Tomahawk; -ScriptJob::ScriptJob( const QString& id, ScriptObject* scriptObject, const QString& methodName, const QVariantMap& arguments ) - : QObject( scriptObject->thread() == QThread::currentThread() ? scriptObject : nullptr ) +ScriptJob::ScriptJob( const QString& id, const scriptobject_ptr& scriptObject, const QString& methodName, const QVariantMap& arguments ) + : QObject( scriptObject->thread() == QThread::currentThread() ? scriptObject.data() : nullptr ) , m_error( false ) , m_id( id ) , m_scriptObject( scriptObject ) @@ -54,7 +54,7 @@ ScriptJob::error() const } -ScriptObject* +const scriptobject_ptr ScriptJob::scriptObject() const { return m_scriptObject; diff --git a/src/libtomahawk/resolvers/ScriptJob.h b/src/libtomahawk/resolvers/ScriptJob.h index dd20ed14b..2f198230a 100644 --- a/src/libtomahawk/resolvers/ScriptJob.h +++ b/src/libtomahawk/resolvers/ScriptJob.h @@ -19,6 +19,8 @@ #ifndef TOMAHAWK_SCRIPTJOB_H #define TOMAHAWK_SCRIPTJOB_H +#include "../Typedefs.h" + #include #include @@ -34,7 +36,7 @@ class DLLEXPORT ScriptJob : public QObject Q_OBJECT public: - ScriptJob( const QString& id, ScriptObject* scriptObject, const QString& methodName, const QVariantMap& arguments = QVariantMap() ); + ScriptJob( const QString& id, const scriptobject_ptr& scriptObject, const QString& methodName, const QVariantMap& arguments = QVariantMap() ); virtual ~ScriptJob(); virtual void start(); @@ -42,7 +44,7 @@ public: bool error() const; const QString id() const; - ScriptObject* scriptObject() const; + const scriptobject_ptr scriptObject() const; const QString methodName() const; const QVariantMap arguments() const; @@ -60,7 +62,7 @@ protected: // TODO: pimple bool m_error; QString m_id; - ScriptObject* m_scriptObject; + scriptobject_ptr m_scriptObject; QVariantMap m_data; QString m_methodName; QVariantMap m_arguments; diff --git a/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.cpp b/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.cpp index 70d1ca14f..8ea5f8840 100644 --- a/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.cpp +++ b/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.cpp @@ -24,8 +24,8 @@ using namespace Tomahawk; -ScriptLinkGeneratorPlugin::ScriptLinkGeneratorPlugin( ScriptObject* scriptObject ) - : QObject( scriptObject ) +ScriptLinkGeneratorPlugin::ScriptLinkGeneratorPlugin( const scriptobject_ptr& scriptObject ) + : QObject( scriptObject.data() ) , ScriptPlugin( scriptObject ) , Utils::LinkGeneratorPlugin() , d_ptr( new ScriptLinkGeneratorPluginPrivate( this, scriptObject ) ) diff --git a/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.h b/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.h index ac713bb78..e51d7fa49 100644 --- a/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.h +++ b/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin.h @@ -37,7 +37,7 @@ class DLLEXPORT ScriptLinkGeneratorPlugin : public QObject, public ScriptPlugin, Q_OBJECT public: - ScriptLinkGeneratorPlugin( ScriptObject* scriptObject ); + ScriptLinkGeneratorPlugin( const scriptobject_ptr& scriptObject ); virtual ~ScriptLinkGeneratorPlugin(); ScriptJob* openLink( const QString& title, const QString& artist, const QString& album ) const override; diff --git a/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin_p.h b/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin_p.h index dda1507cd..7c1e5feb9 100644 --- a/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin_p.h +++ b/src/libtomahawk/resolvers/ScriptLinkGeneratorPlugin_p.h @@ -27,7 +27,7 @@ namespace Tomahawk class ScriptLinkGeneratorPluginPrivate { public: - ScriptLinkGeneratorPluginPrivate( ScriptLinkGeneratorPlugin* q, ScriptObject* scriptObject ) + ScriptLinkGeneratorPluginPrivate( ScriptLinkGeneratorPlugin* q, const scriptobject_ptr& scriptObject ) : q_ptr ( q ) , scriptObject( scriptObject ) { @@ -36,7 +36,7 @@ public: Q_DECLARE_PUBLIC ( ScriptLinkGeneratorPlugin ) private: - ScriptObject* scriptObject; + scriptobject_ptr scriptObject; }; } // ns: Tomahawk diff --git a/src/libtomahawk/resolvers/ScriptObject.cpp b/src/libtomahawk/resolvers/ScriptObject.cpp index d991c9aae..4a32c524d 100644 --- a/src/libtomahawk/resolvers/ScriptObject.cpp +++ b/src/libtomahawk/resolvers/ScriptObject.cpp @@ -31,6 +31,24 @@ ScriptObject::ScriptObject( const QString& id, ScriptAccount* parent ) ScriptObject::~ScriptObject() { + //TODO: Album clears the ownRef wptr explicitly ... why? +} + + +void +ScriptObject::setWeakRef(const scriptobject_wptr& weakRef) +{ + Q_D( ScriptObject ); + d->ownRef = weakRef; +} + + +const scriptobject_wptr +ScriptObject::weakRef() const +{ + Q_D( const ScriptObject ); + + return d->ownRef; } @@ -39,7 +57,7 @@ ScriptObject::invoke( const QString& methodName, const QVariantMap& arguments ) { Q_D( ScriptObject ); - return d->scriptAccount->invoke( this, methodName, arguments ); + return d->scriptAccount->invoke( weakRef().toStrongRef(), methodName, arguments ); } @@ -48,7 +66,7 @@ ScriptObject::syncInvoke(const QString& methodName, const QVariantMap& arguments { Q_D( ScriptObject ); - return d->scriptAccount->syncInvoke( this, methodName, arguments ); + return d->scriptAccount->syncInvoke( weakRef().toStrongRef(), methodName, arguments ); } diff --git a/src/libtomahawk/resolvers/ScriptObject.h b/src/libtomahawk/resolvers/ScriptObject.h index caec59db2..f6314d5fc 100644 --- a/src/libtomahawk/resolvers/ScriptObject.h +++ b/src/libtomahawk/resolvers/ScriptObject.h @@ -19,6 +19,8 @@ #ifndef TOMAHAWK_SCRIPTOBJECT_H #define TOMAHAWK_SCRIPTOBJECT_H +#include "../Typedefs.h" + #include #include @@ -40,6 +42,9 @@ public: ScriptObject( const QString& id, ScriptAccount* parent ); virtual ~ScriptObject(); + void setWeakRef( const scriptobject_wptr& weakRef ); + const scriptobject_wptr weakRef() const; + ScriptJob* invoke( const QString& methodName, const QVariantMap& arguments = QVariantMap() ); /** diff --git a/src/libtomahawk/resolvers/ScriptObject_p.h b/src/libtomahawk/resolvers/ScriptObject_p.h index 247ed62cf..44d297e2f 100644 --- a/src/libtomahawk/resolvers/ScriptObject_p.h +++ b/src/libtomahawk/resolvers/ScriptObject_p.h @@ -40,6 +40,7 @@ public: private: QString id; ScriptAccount* scriptAccount; + scriptobject_wptr ownRef; }; } // ns: Tomahawk diff --git a/src/libtomahawk/resolvers/ScriptPlugin.cpp b/src/libtomahawk/resolvers/ScriptPlugin.cpp index 1264705ea..c9e1f8a72 100644 --- a/src/libtomahawk/resolvers/ScriptPlugin.cpp +++ b/src/libtomahawk/resolvers/ScriptPlugin.cpp @@ -20,7 +20,7 @@ using namespace Tomahawk; -ScriptPlugin::ScriptPlugin( ScriptObject* object ) +ScriptPlugin::ScriptPlugin( const scriptobject_ptr& object ) : m_scriptObject( object ) { } @@ -32,7 +32,7 @@ ScriptPlugin::~ScriptPlugin() } -ScriptObject* +const scriptobject_ptr ScriptPlugin::scriptObject() const { return m_scriptObject; diff --git a/src/libtomahawk/resolvers/ScriptPlugin.h b/src/libtomahawk/resolvers/ScriptPlugin.h index b6889c363..fe8fbbb27 100644 --- a/src/libtomahawk/resolvers/ScriptPlugin.h +++ b/src/libtomahawk/resolvers/ScriptPlugin.h @@ -32,13 +32,13 @@ class ScriptObject; class DLLEXPORT ScriptPlugin { public: - ScriptPlugin( ScriptObject* object ); + ScriptPlugin( const scriptobject_ptr& object ); virtual ~ScriptPlugin(); - ScriptObject* scriptObject() const; + const scriptobject_ptr scriptObject() const; protected: // TODO: pimple - QPointer< ScriptObject > m_scriptObject; + scriptobject_ptr m_scriptObject; }; diff --git a/src/libtomahawk/resolvers/SyncScriptJob.cpp b/src/libtomahawk/resolvers/SyncScriptJob.cpp index 82a72dc1b..99f76ceb1 100644 --- a/src/libtomahawk/resolvers/SyncScriptJob.cpp +++ b/src/libtomahawk/resolvers/SyncScriptJob.cpp @@ -17,9 +17,11 @@ */ #include "SyncScriptJob.h" +#include "../Typedefs.h" +#include "ScriptObject.h" Tomahawk::SyncScriptJob::SyncScriptJob( const QVariantMap& resultData ) - : ScriptJob( QString(), nullptr, QString() ) + : ScriptJob( QString(), scriptobject_ptr(), QString() ) { m_data = resultData; }