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>

1 files changed, 37 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);