From 1a4bfd3ebebfd8d6fb621eddff7fd62346c25af9 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Mon, 19 Mar 2012 18:38:38 +0100 Subject: [PATCH 1/2] Add small unit test for closure. --- tests/CMakeLists.txt | 2 ++ tests/closure_test.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ tests/test_utils.cpp | 13 +++++++++++++ tests/test_utils.h | 19 +++++++++++++++++++ 4 files changed, 74 insertions(+) create mode 100644 tests/closure_test.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 36fabaadf..1dd447a7c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -63,6 +63,7 @@ set(TESTUTILS-SOURCES set(TESTUTILS-MOC-HEADERS mock_networkaccessmanager.h + test_utils.h testobjectdecorators.h ) @@ -138,6 +139,7 @@ add_test_file(song_test.cpp false) add_test_file(translations_test.cpp false) add_test_file(utilities_test.cpp false) #add_test_file(xspfparser_test.cpp false) +add_test_file(closure_test.cpp false) #if(LINUX AND HAVE_DBUS) # add_test_file(mpris1_test.cpp true) diff --git a/tests/closure_test.cpp b/tests/closure_test.cpp new file mode 100644 index 000000000..80e3bf8e7 --- /dev/null +++ b/tests/closure_test.cpp @@ -0,0 +1,40 @@ +#include "gtest/gtest.h" + +#include +#include + +#include "core/closure.h" +#include "test_utils.h" + +class ClosureTest : public ::testing::Test { + +}; + +TEST_F(ClosureTest, ClosureInvokesReceiver) { + TestQObject sender; + TestQObject receiver; + Closure* closure = NewClosure( + &sender, SIGNAL(Emitted()), + &receiver, SLOT(Invoke())); + EXPECT_EQ(0, receiver.invoked()); + sender.Emit(); + EXPECT_EQ(1, receiver.invoked()); +} + +TEST_F(ClosureTest, ClosureDeletesSelf) { + TestQObject sender; + TestQObject receiver; + Closure* closure = NewClosure( + &sender, SIGNAL(Emitted()), + &receiver, SLOT(Invoke())); + QSignalSpy spy(closure, SIGNAL(destroyed())); + EXPECT_EQ(0, receiver.invoked()); + sender.Emit(); + EXPECT_EQ(1, receiver.invoked()); + + EXPECT_EQ(0, spy.count()); + QEventLoop loop; + QObject::connect(closure, SIGNAL(destroyed()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(1, spy.count()); +} diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 5a9610174..f404b543a 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -65,3 +65,16 @@ TemporaryResource::TemporaryResource(const QString& filename) { reset(); } + +TestQObject::TestQObject(QObject* parent) + : QObject(parent), + invoked_(0) { +} + +void TestQObject::Emit() { + emit Emitted(); +} + +void TestQObject::Invoke() { + ++invoked_; +} diff --git a/tests/test_utils.h b/tests/test_utils.h index 26cbab828..29730dbef 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -62,4 +62,23 @@ public: TemporaryResource(const QString& filename); }; +class TestQObject : public QObject { + Q_OBJECT + public: + TestQObject(QObject* parent = 0); + + void Emit(); + + int invoked() const { return invoked_; } + + signals: + void Emitted(); + + public slots: + void Invoke(); + + private: + int invoked_; +}; + #endif // TEST_UTILS_H From e1d77e0124b6b2504c6740d432fae76639e0af25 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Mon, 19 Mar 2012 19:36:52 +0100 Subject: [PATCH 2/2] Add support for QSharedPointer in Closure. --- ext/libclementine-common/core/closure.cpp | 3 ++ ext/libclementine-common/core/closure.h | 53 +++++++++++++++++++++++ tests/closure_test.cpp | 27 ++++++++++++ 3 files changed, 83 insertions(+) diff --git a/ext/libclementine-common/core/closure.cpp b/ext/libclementine-common/core/closure.cpp index 0688903de..f8e51274d 100644 --- a/ext/libclementine-common/core/closure.cpp +++ b/ext/libclementine-common/core/closure.cpp @@ -50,6 +50,9 @@ Closure::Closure(QObject* sender, Connect(sender, signal); } +Closure::~Closure() { +} + void Closure::Connect(QObject* sender, const char* signal) { bool success = connect(sender, signal, SLOT(Invoked())); Q_ASSERT(success); diff --git a/ext/libclementine-common/core/closure.h b/ext/libclementine-common/core/closure.h index 6f9c3626f..319155ba4 100644 --- a/ext/libclementine-common/core/closure.h +++ b/ext/libclementine-common/core/closure.h @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -62,6 +63,8 @@ class Closure : public QObject, boost::noncopyable { Closure(QObject* sender, const char* signal, std::tr1::function callback); + virtual ~Closure(); + private slots: void Invoked(); void Cleanup(); @@ -78,6 +81,46 @@ class Closure : public QObject, boost::noncopyable { boost::scoped_ptr val3_; }; +class SharedPointerWrapper { + public: + virtual ~SharedPointerWrapper() {} + virtual void* data() const = 0; +}; + +template +class SharedPointer : public SharedPointerWrapper { + public: + SharedPointer(QSharedPointer ptr) + : ptr_(ptr) { + } + void* data() const { + return ptr_.data(); + } + private: + QSharedPointer ptr_; +}; + +// For use with a QSharedPointer as a sender. +class SharedClosure : public Closure { + Q_OBJECT + + public: + SharedClosure(SharedPointerWrapper* sender, const char* signal, + QObject* receiver, const char* slot, + const ClosureArgumentWrapper* val0 = 0, + const ClosureArgumentWrapper* val1 = 0, + const ClosureArgumentWrapper* val2 = 0, + const ClosureArgumentWrapper* val3 = 0) + : Closure(reinterpret_cast(sender->data()), signal, + receiver, slot, + val0, val1, val2, val3), + shared_sender_(sender) { + } + + private: + boost::scoped_ptr shared_sender_; +}; + #define C_ARG(type, data) new ClosureArgument(data) Closure* NewClosure( @@ -86,6 +129,16 @@ Closure* NewClosure( QObject* receiver, const char* slot); +template +Closure* NewClosure( + QSharedPointer sender, + const char* signal, + QObject* receiver, + const char* slot) { + Q_ASSERT(sender.data()->metaObject()); // Static check for something QObject-like. + return new SharedClosure(new SharedPointer(sender), signal, receiver, slot); +} + template Closure* NewClosure( QObject* sender, diff --git a/tests/closure_test.cpp b/tests/closure_test.cpp index 80e3bf8e7..2cb3abb1d 100644 --- a/tests/closure_test.cpp +++ b/tests/closure_test.cpp @@ -1,6 +1,8 @@ #include "gtest/gtest.h" #include +#include +#include #include #include "core/closure.h" @@ -38,3 +40,28 @@ TEST_F(ClosureTest, ClosureDeletesSelf) { loop.exec(); EXPECT_EQ(1, spy.count()); } + +TEST_F(ClosureTest, ClosureDoesNotCrashWithSharedPointerSender) { + TestQObject receiver; + TestQObject* sender; + boost::scoped_ptr spy; + QPointer closure; + { + QSharedPointer sender_shared(new TestQObject); + sender = sender_shared.data(); + closure = QPointer(NewClosure( + sender_shared, SIGNAL(Emitted()), + &receiver, SLOT(Invoke()))); + spy.reset(new QSignalSpy(sender, SIGNAL(destroyed()))); + } + EXPECT_EQ(0, receiver.invoked()); + sender->Emit(); + EXPECT_EQ(1, receiver.invoked()); + + EXPECT_EQ(0, spy->count()); + QEventLoop loop; + QObject::connect(sender, SIGNAL(destroyed()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(1, spy->count()); + EXPECT_TRUE(closure.isNull()); +}