Allow QWebEnginePage::createWindow to return this

Consider a QWebEnginePage subclass that does "return this;" in
createWindow. Commit 1f07d2929a made this a no-op to prevent QtWebEngine
from crashing. The reason for the crash was access to deleted memory
after destroying the current adapter in adoptNewWindow.
  Defer the adoption in this case to whenever we hit the event loop
again.

Change-Id: I9674d80ef8b2f301c1446ff505b2486649451ba6
Task-number: QTBUG-42216
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

2 files changed, 23 insertions, 7 deletions

diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 9685d19f1..dbbac1aed 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -75,6 +75,7 @@
 #include <QMimeData>
 #include <QStandardPaths>
 #include <QStyle>
+#include <QTimer>
 #include <QUrl>
 
 #include <private/qguiapplication_p.h>
@@ -275,6 +276,20 @@ void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> ne
     if (!newPage)
         return;
 
+    if (newPage->d_func() == this) {
+        // If createWindow returns /this/ we must delay the adoption.
+        Q_ASSERT(q == newPage);
+        QTimer::singleShot(0, q, [this, newPage, newWebContents, initialGeometry] () {
+            adoptNewWindowImpl(newPage, newWebContents, initialGeometry);
+        });
+    } else {
+        adoptNewWindowImpl(newPage, newWebContents, initialGeometry);
+    }
+}
+
+void QWebEnginePagePrivate::adoptNewWindowImpl(QWebEnginePage *newPage,
+        const QSharedPointer<WebContentsAdapter> &newWebContents, const QRect &initialGeometry)
+{
     // Mark the new page as being in the process of being adopted, so that a second mouse move event
     // sent by newWebContents->initialize() gets filtered in RenderWidgetHostViewQt::forwardEvent.
     // The first mouse move event is being sent by q->createWindow(). This is necessary because
@@ -287,13 +302,11 @@ void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> ne
     newPage->d_func()->m_isBeingAdopted = true;
 
     // Overwrite the new page's WebContents with ours.
-    if (newPage->d_func() != this) {
-        newPage->d_func()->adapter = newWebContents;
-        newWebContents->initialize(newPage->d_func());
-        newPage->d_func()->scriptCollection.d->rebindToContents(newWebContents);
-        if (!initialGeometry.isEmpty())
-            emit newPage->geometryChangeRequested(initialGeometry);
-    }
+    newPage->d_func()->adapter = newWebContents;
+    newWebContents->initialize(newPage->d_func());
+    newPage->d_func()->scriptCollection.d->rebindToContents(newWebContents);
+    if (!initialGeometry.isEmpty())
+        emit newPage->geometryChangeRequested(initialGeometry);
 
     // Page has finished the adoption process.
     newPage->d_func()->m_isBeingAdopted = false;
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index 41ba84dd0..27f582c30 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -96,6 +96,9 @@ public:
     virtual void focusContainer() Q_DECL_OVERRIDE;
     virtual void unhandledKeyEvent(QKeyEvent *event) Q_DECL_OVERRIDE;
     virtual void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry) Q_DECL_OVERRIDE;
+    void adoptNewWindowImpl(QWebEnginePage *newPage,
+            const QSharedPointer<QtWebEngineCore::WebContentsAdapter> &newWebContents,
+            const QRect &initialGeometry);
     virtual bool isBeingAdopted() Q_DECL_OVERRIDE;
     virtual void close() Q_DECL_OVERRIDE;
     virtual void windowCloseRejected() Q_DECL_OVERRIDE;