Merge pull request #3533

Nils Fenner (2):
      fix(screen-grabber): fix crash
      refactor(chatform): cleanup if block and use positive comparison

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)
+    // note: grabber is self-managed and will destroy itself when done
-        screenshotGrabber = new ScreenshotGrabber(this);
+    ScreenshotGrabber* screenshotGrabber = new ScreenshotGrabber;
 
     connect(screenshotGrabber, &ScreenshotGrabber::screenshotTaken,
             this, &ChatForm::onScreenshotTaken);
@@ -821,17 +820,8 @@ void ChatForm::onScreenshotTaken(const QPixmap &pixmap) {
 
     QFile file(filepath);
 
-    if (!file.open(QFile::ReadWrite))
+    if (file.open(QFile::ReadWrite))
     {
-        QMessageBox::warning(this,
-                             tr("Failed to open temporary file", "Temporary file for screenshot"),
-                             tr("qTox wasn't able to save the screenshot"));
-
-        delete screenshotGrabber;
-        screenshotGrabber = nullptr;
-        return;
-    }
-
         pixmap.save(&file, "PNG");
 
         qint64 filesize = file.size();
@@ -839,8 +829,13 @@ void ChatForm::onScreenshotTaken(const QPixmap &pixmap) {
         QFileInfo fi(file);
 
         emit sendFile(f->getFriendID(), fi.fileName(), fi.filePath(), filesize);
-    delete screenshotGrabber;
+    }
-    screenshotGrabber = nullptr;
+    else
+    {
+        QMessageBox::warning(this,
+                             tr("Failed to open temporary file", "Temporary file for screenshot"),
+                             tr("qTox wasn't able to save the screenshot"));
+    }
 }
 
 void ChatForm::onLoadHistory()

src/widget/form/chatform.h

@@ -121,7 +121,6 @@ private:
     QAction* loadHistoryAction;
     QAction* copyStatusAction;
 
-    ScreenshotGrabber* screenshotGrabber;
     QHash<uint, FileTransferInstance*> ftransWidgets;
     CallConfirmWidget *callConfirm;
     bool isTyping;

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);

src/widget/tool/screenshotgrabber.cpp

@@ -35,14 +35,13 @@
 #include "toolboxgraphicsitem.h"
 #include "src/widget/widget.h"
 
-ScreenshotGrabber::ScreenshotGrabber(QObject* parent)
+ScreenshotGrabber::ScreenshotGrabber()
-    : QObject(parent)
+    : 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()

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;