From 896fe069b6e1f0406ba483dd82480e32ffe2a5df Mon Sep 17 00:00:00 2001 From: Konstantinos Sideris Date: Fri, 7 Sep 2018 12:24:09 +0300 Subject: [PATCH] Use proxy objects on lambdas instead of raw pointers When the object is destroyed the connections will be removed automatically by Qt. fixes #433 --- CMakeLists.txt | 1 + src/MatrixClient.h | 11 ++++++ src/dialogs/RoomSettings.cpp | 56 ++++++++++++++++-------------- src/dialogs/RoomSettings.h | 6 ++-- src/timeline/widgets/AudioItem.cpp | 16 +++++---- src/timeline/widgets/AudioItem.h | 3 -- src/timeline/widgets/FileItem.cpp | 17 ++++----- src/timeline/widgets/FileItem.h | 3 -- src/timeline/widgets/ImageItem.cpp | 29 +++++++++------- src/timeline/widgets/ImageItem.h | 4 --- 10 files changed, 79 insertions(+), 67 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e6f93445..42990445 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -333,6 +333,7 @@ qt5_wrap_cpp(MOC_HEADERS src/CommunitiesList.h src/LoginPage.h src/MainWindow.h + src/MatrixClient.h src/InviteeItem.h src/QuickSwitcher.h src/RegisterPage.h diff --git a/src/MatrixClient.h b/src/MatrixClient.h index 12bba889..f0aca5c4 100644 --- a/src/MatrixClient.h +++ b/src/MatrixClient.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -17,6 +18,16 @@ Q_DECLARE_METATYPE(std::string) Q_DECLARE_METATYPE(std::vector) Q_DECLARE_METATYPE(std::vector) +class MediaProxy : public QObject +{ + Q_OBJECT + +signals: + void imageDownloaded(const QPixmap &); + void imageSaved(const QString &, const QByteArray &); + void fileDownloaded(const QByteArray &); +}; + namespace http { mtx::http::Client * client(); diff --git a/src/dialogs/RoomSettings.cpp b/src/dialogs/RoomSettings.cpp index 0a1c93e6..4e56b130 100644 --- a/src/dialogs/RoomSettings.cpp +++ b/src/dialogs/RoomSettings.cpp @@ -90,20 +90,6 @@ EditModal::EditModal(const QString &roomId, QWidget *parent) labelLayout->addWidget(errorField_); layout->addLayout(labelLayout); - connect(this, &EditModal::stateEventErrorCb, this, [this](const QString &msg) { - errorField_->setText(msg); - errorField_->show(); - }); - connect(this, &EditModal::nameEventSentCb, this, [this](const QString &newName) { - errorField_->hide(); - emit nameChanged(newName); - close(); - }); - connect(this, &EditModal::topicEventSentCb, this, [this]() { - errorField_->hide(); - close(); - }); - connect(applyBtn_, &QPushButton::clicked, [this]() { // Check if the values are changed from the originals. auto newName = nameInput_->text().trimmed(); @@ -117,6 +103,21 @@ EditModal::EditModal(const QString &roomId, QWidget *parent) } using namespace mtx::events; + auto proxy = std::make_shared(); + connect(proxy.get(), &ThreadProxy::topicEventSent, this, [this]() { + errorField_->hide(); + close(); + }); + connect( + proxy.get(), &ThreadProxy::nameEventSent, this, [this](const QString &newName) { + errorField_->hide(); + emit nameChanged(newName); + close(); + }); + connect(proxy.get(), &ThreadProxy::error, this, [this](const QString &msg) { + errorField_->setText(msg); + errorField_->show(); + }); if (newName != initialName_ && !newName.isEmpty()) { state::Name body; @@ -125,15 +126,15 @@ EditModal::EditModal(const QString &roomId, QWidget *parent) http::client()->send_state_event( roomId_.toStdString(), body, - [this, newName](const mtx::responses::EventId &, - mtx::http::RequestErr err) { + [proxy, newName](const mtx::responses::EventId &, + mtx::http::RequestErr err) { if (err) { - emit stateEventErrorCb( + emit proxy->error( QString::fromStdString(err->matrix_error.error)); return; } - emit nameEventSentCb(newName); + emit proxy->nameEventSent(newName); }); } @@ -144,14 +145,14 @@ EditModal::EditModal(const QString &roomId, QWidget *parent) http::client()->send_state_event( roomId_.toStdString(), body, - [this](const mtx::responses::EventId &, mtx::http::RequestErr err) { + [proxy](const mtx::responses::EventId &, mtx::http::RequestErr err) { if (err) { - emit stateEventErrorCb( + emit proxy->error( QString::fromStdString(err->matrix_error.error)); return; } - emit topicEventSentCb(); + emit proxy->topicEventSent(); }); } }); @@ -366,15 +367,15 @@ RoomSettings::RoomSettings(const QString &room_id, QWidget *parent) connect(filter, &ClickableFilter::clicked, this, &RoomSettings::updateAvatar); } - auto roomNameLabel = new QLabel(QString::fromStdString(info_.name), this); - roomNameLabel->setFont(doubleFont); + roomNameLabel_ = new QLabel(QString::fromStdString(info_.name), this); + roomNameLabel_->setFont(doubleFont); auto membersLabel = new QLabel(tr("%n member(s)", "", info_.member_count), this); auto textLayout = new QVBoxLayout; - textLayout->addWidget(roomNameLabel); + textLayout->addWidget(roomNameLabel_); textLayout->addWidget(membersLabel); - textLayout->setAlignment(roomNameLabel, Qt::AlignCenter | Qt::AlignTop); + textLayout->setAlignment(roomNameLabel_, Qt::AlignCenter | Qt::AlignTop); textLayout->setAlignment(membersLabel, Qt::AlignCenter | Qt::AlignTop); textLayout->setSpacing(TEXT_SPACING); textLayout->setMargin(0); @@ -458,8 +459,9 @@ RoomSettings::setupEditButton() modal->setFields(QString::fromStdString(info_.name), QString::fromStdString(info_.topic)); modal->show(); - connect(modal, &EditModal::nameChanged, this, [](const QString &newName) { - Q_UNUSED(newName); + connect(modal, &EditModal::nameChanged, this, [this](const QString &newName) { + if (roomNameLabel_) + roomNameLabel_->setText(newName); }); }); diff --git a/src/dialogs/RoomSettings.h b/src/dialogs/RoomSettings.h index ac9097dd..dd729250 100644 --- a/src/dialogs/RoomSettings.h +++ b/src/dialogs/RoomSettings.h @@ -53,6 +53,8 @@ class ThreadProxy : public QObject signals: void error(const QString &msg); void avatarChanged(const QImage &img); + void nameEventSent(const QString &); + void topicEventSent(); }; class EditModal : public QWidget @@ -66,9 +68,6 @@ public: signals: void nameChanged(const QString &roomName); - void nameEventSentCb(const QString &newName); - void topicEventSentCb(); - void stateEventErrorCb(const QString &msg); private: QString roomId_; @@ -138,6 +137,7 @@ private: QString room_id_; QImage avatarImg_; + QLabel *roomNameLabel_ = nullptr; QLabel *errorLabel_ = nullptr; LoadingIndicator *spinner_ = nullptr; diff --git a/src/timeline/widgets/AudioItem.cpp b/src/timeline/widgets/AudioItem.cpp index 1e3eb0f0..0c4bf9e4 100644 --- a/src/timeline/widgets/AudioItem.cpp +++ b/src/timeline/widgets/AudioItem.cpp @@ -55,7 +55,6 @@ AudioItem::init() player_->setVolume(100); player_->setNotifyInterval(1000); - connect(this, &AudioItem::fileDownloadedCb, this, &AudioItem::fileDownloaded); connect(player_, &QMediaPlayer::stateChanged, this, [this](QMediaPlayer::State state) { if (state == QMediaPlayer::StoppedState) { state_ = AudioState::Play; @@ -120,19 +119,22 @@ AudioItem::mousePressEvent(QMouseEvent *event) if (filenameToSave_.isEmpty()) return; + auto proxy = std::make_shared(); + connect(proxy.get(), &MediaProxy::fileDownloaded, this, &AudioItem::fileDownloaded); + http::client()->download( url_.toString().toStdString(), - [this](const std::string &data, - const std::string &, - const std::string &, - mtx::http::RequestErr err) { + [proxy = std::move(proxy), url = url_](const std::string &data, + const std::string &, + const std::string &, + mtx::http::RequestErr err) { if (err) { nhlog::net()->info("failed to retrieve m.audio content: {}", - url_.toString().toStdString()); + url.toString().toStdString()); return; } - emit fileDownloadedCb(QByteArray(data.data(), data.size())); + emit proxy->fileDownloaded(QByteArray(data.data(), data.size())); }); } } diff --git a/src/timeline/widgets/AudioItem.h b/src/timeline/widgets/AudioItem.h index 7b0781a2..c32b7731 100644 --- a/src/timeline/widgets/AudioItem.h +++ b/src/timeline/widgets/AudioItem.h @@ -69,9 +69,6 @@ protected: void resizeEvent(QResizeEvent *event) override; void mousePressEvent(QMouseEvent *event) override; -signals: - void fileDownloadedCb(const QByteArray &data); - private slots: void fileDownloaded(const QByteArray &data); diff --git a/src/timeline/widgets/FileItem.cpp b/src/timeline/widgets/FileItem.cpp index f8d3272d..850941ae 100644 --- a/src/timeline/widgets/FileItem.cpp +++ b/src/timeline/widgets/FileItem.cpp @@ -50,8 +50,6 @@ FileItem::init() icon_.addFile(":/icons/icons/ui/arrow-pointing-down.png"); setFixedHeight(Height); - - connect(this, &FileItem::fileDownloadedCb, this, &FileItem::fileDownloaded); } FileItem::FileItem(const mtx::events::RoomEvent &event, QWidget *parent) @@ -110,19 +108,22 @@ FileItem::mousePressEvent(QMouseEvent *event) if (filenameToSave_.isEmpty()) return; + auto proxy = std::make_shared(); + connect(proxy.get(), &MediaProxy::fileDownloaded, this, &FileItem::fileDownloaded); + http::client()->download( url_.toString().toStdString(), - [this](const std::string &data, - const std::string &, - const std::string &, - mtx::http::RequestErr err) { + [proxy = std::move(proxy), url = url_](const std::string &data, + const std::string &, + const std::string &, + mtx::http::RequestErr err) { if (err) { nhlog::ui()->warn("failed to retrieve m.file content: {}", - url_.toString().toStdString()); + url.toString().toStdString()); return; } - emit fileDownloadedCb(QByteArray(data.data(), data.size())); + emit proxy->fileDownloaded(QByteArray(data.data(), data.size())); }); } else { openUrl(); diff --git a/src/timeline/widgets/FileItem.h b/src/timeline/widgets/FileItem.h index 66543e79..d63cce88 100644 --- a/src/timeline/widgets/FileItem.h +++ b/src/timeline/widgets/FileItem.h @@ -52,9 +52,6 @@ public: QColor iconColor() const { return iconColor_; } QColor backgroundColor() const { return backgroundColor_; } -signals: - void fileDownloadedCb(const QByteArray &data); - protected: void paintEvent(QPaintEvent *event) override; void mousePressEvent(QMouseEvent *event) override; diff --git a/src/timeline/widgets/ImageItem.cpp b/src/timeline/widgets/ImageItem.cpp index b66b82ae..4c68d683 100644 --- a/src/timeline/widgets/ImageItem.cpp +++ b/src/timeline/widgets/ImageItem.cpp @@ -33,11 +33,14 @@ void ImageItem::downloadMedia(const QUrl &url) { + auto proxy = std::make_shared(); + connect(proxy.get(), &MediaProxy::imageDownloaded, this, &ImageItem::setImage); + http::client()->download(url.toString().toStdString(), - [this, url](const std::string &data, - const std::string &, - const std::string &, - mtx::http::RequestErr err) { + [proxy = std::move(proxy), url](const std::string &data, + const std::string &, + const std::string &, + mtx::http::RequestErr err) { if (err) { nhlog::net()->warn( "failed to retrieve image {}: {} {}", @@ -49,7 +52,8 @@ ImageItem::downloadMedia(const QUrl &url) QPixmap img; img.loadFromData(QByteArray(data.data(), data.size())); - emit imageDownloaded(img); + + emit proxy->imageDownloaded(img); }); } @@ -76,8 +80,6 @@ ImageItem::init() setCursor(Qt::PointingHandCursor); setAttribute(Qt::WA_Hover, true); - connect(this, &ImageItem::imageDownloaded, this, &ImageItem::setImage); - connect(this, &ImageItem::imageSaved, this, &ImageItem::saveImage); downloadMedia(url_); } @@ -240,12 +242,15 @@ ImageItem::saveAs() const auto url = url_.toString().toStdString(); + auto proxy = std::make_shared(); + connect(proxy.get(), &MediaProxy::imageSaved, this, &ImageItem::saveImage); + http::client()->download( url, - [this, filename, url](const std::string &data, - const std::string &, - const std::string &, - mtx::http::RequestErr err) { + [proxy = std::move(proxy), filename, url](const std::string &data, + const std::string &, + const std::string &, + mtx::http::RequestErr err) { if (err) { nhlog::net()->warn("failed to retrieve image {}: {} {}", url, @@ -254,6 +259,6 @@ ImageItem::saveAs() return; } - emit imageSaved(filename, QByteArray(data.data(), data.size())); + emit proxy->imageSaved(filename, QByteArray(data.data(), data.size())); }); } diff --git a/src/timeline/widgets/ImageItem.h b/src/timeline/widgets/ImageItem.h index e9d823f4..65bd962d 100644 --- a/src/timeline/widgets/ImageItem.h +++ b/src/timeline/widgets/ImageItem.h @@ -48,10 +48,6 @@ public slots: void setImage(const QPixmap &image); void saveImage(const QString &filename, const QByteArray &data); -signals: - void imageDownloaded(const QPixmap &img); - void imageSaved(const QString &filename, const QByteArray &data); - protected: void paintEvent(QPaintEvent *event) override; void mousePressEvent(QMouseEvent *event) override;