Don't crash when using onEditingFinished

In order to stay in line with the behavior expected by Chromium, we
focus the view on load. This is problematic when relying on the
editingFinished signal of text inputs in QML, as it is fired both when
pressing enter and when losing focus.

In our case, this would lead to reentering into load and in turn
QQuickWindowPrivate::setFocusInScope, and when returning from the outer
call, QQuickWindow would try to access the RWHVQtDelegateQuick from the
first load through a now dangling pointer.

It seems preferable to guard WebContentsAdapter::load against recursion.

Adds a simple autotest that covers the crash scenario.

Task-number: QTBUG-42929
Change-Id: Ib3bf9f421b1a91645b3e0e9aa658f2a3646d9caf
Reviewed-by: Andras Becsi <andras.becsi@theqtcompany.com>
Reviewed-by: Zeno Albisser <zeno.albisser@digia.com>

3 files changed, 128 insertions, 0 deletions

diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index 010ce042c..dc20ea180 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -292,6 +292,30 @@ static void deserializeNavigationHistory(QDataStream &input, int *currentIndex,
     }
 }
 
+namespace {
+static QList<WebContentsAdapter *> recursive_guard_loading_adapters;
+
+class LoadRecursionGuard {
+    public:
+    static bool isGuarded(WebContentsAdapter *adapter)
+    {
+        return recursive_guard_loading_adapters.contains(adapter);
+    }
+    LoadRecursionGuard(WebContentsAdapter *adapter)
+        : m_adapter(adapter)
+    {
+        recursive_guard_loading_adapters.append(adapter);
+    }
+
+    ~LoadRecursionGuard() {
+        recursive_guard_loading_adapters.removeOne(m_adapter);
+    }
+
+    private:
+        WebContentsAdapter *m_adapter;
+};
+} // Anonymous namespace
+
 WebContentsAdapterPrivate::WebContentsAdapterPrivate()
     // This has to be the first thing we create, and the last we destroy.
     : engineContext(WebEngineContext::current())
@@ -424,6 +448,19 @@ void WebContentsAdapter::reload()
 
 void WebContentsAdapter::load(const QUrl &url)
 {
+    // The situation can occur when relying on the editingFinished signal in QML to set the url
+    // of the WebView.
+    // When enter is pressed, onEditingFinished fires and the url of the webview is set, which
+    // calls into this and focuses the webview, taking the focus from the TextField/TextInput,
+    // which in turn leads to editingFinished firing again. This scenario would cause a crash
+    // down the line when unwinding as the first RenderWidgetHostViewQtDelegateQuick instance is
+    // a dangling pointer by that time.
+
+    if (LoadRecursionGuard::isGuarded(this))
+        return;
+    LoadRecursionGuard guard(this);
+    Q_UNUSED(guard);
+
     Q_D(WebContentsAdapter);
     content::NavigationController::LoadURLParams params(toGurl(url));
     params.transition_type = content::PageTransitionFromInt(content::PAGE_TRANSITION_TYPED | content::PAGE_TRANSITION_FROM_ADDRESS_BAR);
diff --git a/tests/auto/quick/qmltests/data/tst_loadRecursionCrash.qml b/tests/auto/quick/qmltests/data/tst_loadRecursionCrash.qml
new file mode 100644
index 000000000..f9b95ac19
--- /dev/null
+++ b/tests/auto/quick/qmltests/data/tst_loadRecursionCrash.qml
@@ -0,0 +1,90 @@
+/****************************************************************************
+**
+** Copyright (C) 2014 Digia Plc and/or its subsidiary(-ies).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the Qt Quick Controls module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick 2.3
+import QtTest 1.0
+import QtWebEngine 1.0
+
+Item {
+width: 300
+height: 400
+    TextInput {
+        id: textInput
+        anchors {
+            top: parent.top
+            left: parent.left
+            right: parent.right
+        }
+        focus: true
+        text: Qt.resolvedUrl("test1.html")
+        onEditingFinished: webEngineView.url = text
+    }
+
+    TestWebEngineView {
+        id: webEngineView
+        anchors {
+            top: textInput.bottom
+            left: parent.left
+            right: parent.right
+            bottom: parent.bottom
+        }
+
+        TestCase {
+            name: "WebEngineViewLoadRecursionCrash"
+            when:windowShown
+
+            function test_QTBUG_42929() {
+                textInput.forceActiveFocus()
+                keyClick(Qt.Key_Return)
+                verify(webEngineView.waitForLoadSucceeded())
+                textInput.text = "about:blank"
+                textInput.forceActiveFocus()
+                keyClick(Qt.Key_Return)
+                verify(webEngineView.waitForLoadSucceeded())
+                textInput.text = Qt.resolvedUrl("test4.html")
+                textInput.forceActiveFocus()
+                // Don't crash now
+                keyClick(Qt.Key_Return)
+                verify(webEngineView.waitForLoadSucceeded())
+            }
+        }
+    }
+}
diff --git a/tests/auto/quick/qmltests/qmltests.pro b/tests/auto/quick/qmltests/qmltests.pro
index b40ef3b8c..33a864cf1 100644
--- a/tests/auto/quick/qmltests/qmltests.pro
+++ b/tests/auto/quick/qmltests/qmltests.pro
@@ -23,6 +23,7 @@ OTHER_FILES += \
     $$PWD/data/tst_loadHtml.qml \
     $$PWD/data/tst_loadProgress.qml \
     $$PWD/data/tst_loadProgressSignal.qml \
+    $$PWD/data/tst_loadRecursionCrash.qml \
     $$PWD/data/tst_loadUrl.qml \
     $$PWD/data/tst_navigationHistory.qml \
     $$PWD/data/tst_navigationRequested.qml \