From 780a017928cfa5613fddada6e787bd6f3965baa9 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Thu, 21 Jul 2016 10:49:50 +0200 Subject: [PATCH 1/2] fix(screen-grabber): fix crash --- src/widget/form/chatform.cpp | 9 ++------- src/widget/form/chatform.h | 1 - src/widget/form/settings/avform.cpp | 4 ++-- src/widget/tool/screenshotgrabber.cpp | 12 ++++++------ src/widget/tool/screenshotgrabber.h | 2 +- 5 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/widget/form/chatform.cpp b/src/widget/form/chatform.cpp index 27b63ba45..4f50a6caa 100644 --- a/src/widget/form/chatform.cpp +++ b/src/widget/form/chatform.cpp @@ -67,7 +67,6 @@ ChatForm::ChatForm(Friend* chatFriend) : f(chatFriend) - , screenshotGrabber(nullptr) , isTyping(false) { Core* core = Core::getInstance(); @@ -797,8 +796,8 @@ void ChatForm::onScreenshotClicked() void ChatForm::doScreenshot() { - if (!screenshotGrabber) - screenshotGrabber = new ScreenshotGrabber(this); + // note: grabber is self-managed and will destroy itself when done + ScreenshotGrabber* screenshotGrabber = new ScreenshotGrabber; connect(screenshotGrabber, &ScreenshotGrabber::screenshotTaken, this, &ChatForm::onScreenshotTaken); @@ -827,8 +826,6 @@ void ChatForm::onScreenshotTaken(const QPixmap &pixmap) { tr("Failed to open temporary file", "Temporary file for screenshot"), tr("qTox wasn't able to save the screenshot")); - delete screenshotGrabber; - screenshotGrabber = nullptr; return; } @@ -839,8 +836,6 @@ void ChatForm::onScreenshotTaken(const QPixmap &pixmap) { QFileInfo fi(file); emit sendFile(f->getFriendID(), fi.fileName(), fi.filePath(), filesize); - delete screenshotGrabber; - screenshotGrabber = nullptr; } void ChatForm::onLoadHistory() diff --git a/src/widget/form/chatform.h b/src/widget/form/chatform.h index 2ed951dc4..7e1b3a1d3 100644 --- a/src/widget/form/chatform.h +++ b/src/widget/form/chatform.h @@ -121,7 +121,6 @@ private: QAction* loadHistoryAction; QAction* copyStatusAction; - ScreenshotGrabber* screenshotGrabber; QHash ftransWidgets; CallConfirmWidget *callConfirm; bool isTyping; diff --git a/src/widget/form/settings/avform.cpp b/src/widget/form/settings/avform.cpp index 374867bdd..f0dca8a02 100644 --- a/src/widget/form/settings/avform.cpp +++ b/src/widget/form/settings/avform.cpp @@ -158,7 +158,8 @@ void AVForm::on_videoModescomboBox_currentIndexChanged(int index) return; } - ScreenshotGrabber* screenshotGrabber = new ScreenshotGrabber(this); + // note: grabber is self-managed and will destroy itself when done + ScreenshotGrabber* screenshotGrabber = new ScreenshotGrabber; auto onGrabbed = [screenshotGrabber, devName, this] (QRect region) { @@ -170,7 +171,6 @@ void AVForm::on_videoModescomboBox_currentIndexChanged(int index) Settings::getInstance().setScreenGrabbed(true); open(devName, mode); - delete screenshotGrabber; }; connect(screenshotGrabber, &ScreenshotGrabber::regionChosen, this, onGrabbed, Qt::QueuedConnection); diff --git a/src/widget/tool/screenshotgrabber.cpp b/src/widget/tool/screenshotgrabber.cpp index 4d404927a..2a1246daf 100644 --- a/src/widget/tool/screenshotgrabber.cpp +++ b/src/widget/tool/screenshotgrabber.cpp @@ -35,14 +35,13 @@ #include "toolboxgraphicsitem.h" #include "src/widget/widget.h" -ScreenshotGrabber::ScreenshotGrabber(QObject* parent) - : QObject(parent) +ScreenshotGrabber::ScreenshotGrabber() + : QObject() , mKeysBlocked(false) , scene(0) , mQToxVisible(true) { window = new QGraphicsView (scene); // Top-level widget - window->setAttribute(Qt::WA_DeleteOnClose); window->setWindowFlags(Qt::FramelessWindowHint | Qt::BypassWindowManagerHint); window->setContentsMargins(0, 0, 0, 0); window->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); @@ -71,6 +70,7 @@ void ScreenshotGrabber::reInit() ScreenshotGrabber::~ScreenshotGrabber() { delete scene; + delete window; } bool ScreenshotGrabber::eventFilter(QObject* object, QEvent* event) @@ -134,9 +134,10 @@ void ScreenshotGrabber::acceptRegion() emit regionChosen(rect); qDebug() << "Screenshot accepted, chosen region" << rect; QPixmap pixmap = this->screenGrab.copy(rect); - this->window->close(); restoreHiddenWindows(); emit screenshotTaken(pixmap); + + deleteLater(); } void ScreenshotGrabber::setupScene() @@ -210,9 +211,8 @@ void ScreenshotGrabber::adjustTooltipPosition() void ScreenshotGrabber::reject() { - qDebug() << "Rejected screenshot"; - this->window->close(); restoreHiddenWindows(); + deleteLater(); } QPixmap ScreenshotGrabber::grabScreen() diff --git a/src/widget/tool/screenshotgrabber.h b/src/widget/tool/screenshotgrabber.h index e983aba13..6808e7d3e 100644 --- a/src/widget/tool/screenshotgrabber.h +++ b/src/widget/tool/screenshotgrabber.h @@ -39,7 +39,7 @@ class ScreenshotGrabber : public QObject Q_OBJECT public: - explicit ScreenshotGrabber(QObject* parent); + ScreenshotGrabber(); ~ScreenshotGrabber() override; bool eventFilter(QObject* object, QEvent* event) override; From 2d520322a75a03a2f4ca5b386d229ad0a5e9b759 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Thu, 21 Jul 2016 10:51:50 +0200 Subject: [PATCH 2/2] refactor(chatform): cleanup if block and use positive comparison --- src/widget/form/chatform.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/widget/form/chatform.cpp b/src/widget/form/chatform.cpp index 4f50a6caa..428817822 100644 --- a/src/widget/form/chatform.cpp +++ b/src/widget/form/chatform.cpp @@ -820,22 +820,22 @@ void ChatForm::onScreenshotTaken(const QPixmap &pixmap) { QFile file(filepath); - if (!file.open(QFile::ReadWrite)) + if (file.open(QFile::ReadWrite)) + { + pixmap.save(&file, "PNG"); + + qint64 filesize = file.size(); + file.close(); + QFileInfo fi(file); + + emit sendFile(f->getFriendID(), fi.fileName(), fi.filePath(), filesize); + } + else { QMessageBox::warning(this, tr("Failed to open temporary file", "Temporary file for screenshot"), tr("qTox wasn't able to save the screenshot")); - - return; } - - pixmap.save(&file, "PNG"); - - qint64 filesize = file.size(); - file.close(); - QFileInfo fi(file); - - emit sendFile(f->getFriendID(), fi.fileName(), fi.filePath(), filesize); } void ChatForm::onLoadHistory()