From 6701ff310933ec33f9199b53d78299764f646387 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCri=20Valdmann?= <juri.valdmann@qt.io>
Date: Fri, 15 Dec 2017 13:04:02 +0100
Subject: Allow deleting a channel during method invocation

A real-life use case is the session restore page in KDE Falkon or, more
generally, any kind of closeTab() method exposed to JS in a QWebEnginePage.

The approach taken in this patch will only work if the transport can also deal
with deletion during a messageReceived signal emission. Alternatively, method
invocation could be delayed via the event loop, but this would come with an
obvious performance penalty.

Change-Id: I2940f61e07c77365f6e3e7cd29463d4cb5b525a6
Reviewed-by: Milian Wolff <milian.wolff@kdab.com>
---
 src/webchannel/qmetaobjectpublisher.cpp  | 12 ++++++--
 tests/auto/webchannel/tst_webchannel.cpp | 52 ++++++++++++++++++++++++++++++++
 tests/auto/webchannel/tst_webchannel.h   | 13 +++++++-
 3 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/src/webchannel/qmetaobjectpublisher.cpp b/src/webchannel/qmetaobjectpublisher.cpp
index cfc6ad2..8e83237 100644
--- a/src/webchannel/qmetaobjectpublisher.cpp
+++ b/src/webchannel/qmetaobjectpublisher.cpp
@@ -664,9 +664,15 @@ void QMetaObjectPublisher::handleMessage(const QJsonObject &message, QWebChannel
                 return;
             }
 
-            transport->sendMessage(createResponse(message.value(KEY_ID),
-                wrapResult(invokeMethod(object, message.value(KEY_METHOD).toInt(-1),
-                             message.value(KEY_ARGS).toArray()), transport)));
+            QPointer<QMetaObjectPublisher> publisherExists(this);
+            QPointer<QWebChannelAbstractTransport> transportExists(transport);
+            QVariant result =
+                invokeMethod(object,
+                             message.value(KEY_METHOD).toInt(-1),
+                             message.value(KEY_ARGS).toArray());
+            if (!publisherExists || !transportExists)
+                return;
+            transport->sendMessage(createResponse(message.value(KEY_ID), wrapResult(result, transport)));
         } else if (type == TypeConnectToSignal) {
             signalHandler.connectTo(object, message.value(KEY_SIGNAL).toInt(-1));
         } else if (type == TypeDisconnectFromSignal) {
diff --git a/tests/auto/webchannel/tst_webchannel.cpp b/tests/auto/webchannel/tst_webchannel.cpp
index f214b7e..602a101 100644
--- a/tests/auto/webchannel/tst_webchannel.cpp
+++ b/tests/auto/webchannel/tst_webchannel.cpp
@@ -814,6 +814,58 @@ void TestWebChannel::testAsyncObject()
     thread.wait();
 }
 
+class FunctionWrapper : public QObject
+{
+    Q_OBJECT
+    std::function<void()> m_fun;
+public:
+    FunctionWrapper(std::function<void()> fun) : m_fun(std::move(fun)) {}
+public slots:
+    void invoke()
+    {
+        m_fun();
+    }
+};
+
+void TestWebChannel::testDeletionDuringMethodInvocation_data()
+{
+    QTest::addColumn<bool>("deleteChannel");
+    QTest::addColumn<bool>("deleteTransport");
+    QTest::newRow("delete neither")   << false << false;
+    QTest::newRow("delete channel")   << true  << false;
+    QTest::newRow("delete transport") << false << true;
+    QTest::newRow("delete both")      << true  << true;
+}
+
+void TestWebChannel::testDeletionDuringMethodInvocation()
+{
+    QFETCH(bool, deleteChannel);
+    QFETCH(bool, deleteTransport);
+
+    QScopedPointer<QWebChannel> channel(new QWebChannel);
+    QScopedPointer<DummyTransport> transport(new DummyTransport(nullptr));
+    FunctionWrapper deleter([&](){
+        if (deleteChannel)
+            channel.reset();
+        if (deleteTransport)
+            transport.reset();
+    });
+    channel->registerObject("deleter", &deleter);
+    channel->connectTo(transport.data());
+
+    transport->emitMessageReceived({
+        {"type", TypeInvokeMethod},
+        {"object", "deleter"},
+        {"method", deleter.metaObject()->indexOfMethod("invoke()")},
+        {"id", 42}
+    });
+
+    QCOMPARE(deleteChannel, !channel);
+    QCOMPARE(deleteTransport, !transport);
+    if (!deleteTransport)
+        QCOMPARE(transport->messagesSent().size(), deleteChannel ? 0 : 1);
+}
+
 static QHash<QString, QObject*> createObjects(QObject *parent)
 {
     const int num = 100;
diff --git a/tests/auto/webchannel/tst_webchannel.h b/tests/auto/webchannel/tst_webchannel.h
index d2597e5..85a9f39 100644
--- a/tests/auto/webchannel/tst_webchannel.h
+++ b/tests/auto/webchannel/tst_webchannel.h
@@ -31,6 +31,7 @@
 
 #include <QObject>
 #include <QVariant>
+#include <QVector>
 #include <QJsonValue>
 #include <QJsonObject>
 #include <QJsonArray>
@@ -53,10 +54,18 @@ public:
         emit messageReceived(message, this);
     }
 
+    QVector<QJsonObject> messagesSent() const
+    {
+        return mMessagesSent;
+    }
+
 public slots:
-    void sendMessage(const QJsonObject &/*message*/) Q_DECL_OVERRIDE
+    void sendMessage(const QJsonObject &message) Q_DECL_OVERRIDE
     {
+        mMessagesSent.push_back(message);
     }
+private:
+    QVector<QJsonObject> mMessagesSent;
 };
 
 class TestObject : public QObject
@@ -296,6 +305,8 @@ private slots:
     void testPassWrappedObjectBack();
     void testInfiniteRecursion();
     void testAsyncObject();
+    void testDeletionDuringMethodInvocation_data();
+    void testDeletionDuringMethodInvocation();
 
     void benchClassInfo();
     void benchInitializeClients();
-- 
cgit v1.2.1