Invalidate accessible interfaces on destruction of view or page

The RenderWidgetHostViewQtDelegate(Widget|Quick)Accessible interfaces
are forwarding their queries to the WebEngineView. In case of widget,
the view also forwards the query to the page.

The accessible interfaces may outlive the view and page. The interfaces
are not supposed to be used after the destruction of the underlying
objects. Thus, set the RenderWidgetHostViewQtDelegate and WebEngineView
accessible interfaces invalid if the corresponding pointers are null.

Also fix querying the root accessible interface of the web page when
the render frame host is not available.

This fixes crash when
QT_LOGGING_RULES="qt.accessibility.cache.debug=true"
is set and logger tries to pretty-print QAccessibleInterfaces during
destruction.

Task-number: QTBUG-78284
Change-Id: If18af0605061fcd82d019d0042dbf1c9d3a910be
Reviewed-by: Kirill Burtsev <kirill.burtsev@qt.io>

10 files changed, 78 insertions, 14 deletions

diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index a7579f916..31731fe99 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -1011,7 +1011,10 @@ QAccessibleInterface *WebContentsAdapter::browserAccessible()
     CHECK_INITIALIZED(nullptr);
     content::RenderViewHost *rvh = m_webContents->GetRenderViewHost();
     Q_ASSERT(rvh);
-    content::BrowserAccessibilityManager *manager = static_cast<content::RenderFrameHostImpl*>(rvh->GetMainFrame())->GetOrCreateBrowserAccessibilityManager();
+    content::RenderFrameHostImpl *rfh = static_cast<content::RenderFrameHostImpl *>(rvh->GetMainFrame());
+    if (!rfh)
+        return nullptr;
+    content::BrowserAccessibilityManager *manager = rfh->GetOrCreateBrowserAccessibilityManager();
     if (!manager) // FIXME!
         return nullptr;
     content::BrowserAccessibility *acc = manager->GetRoot();
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index 3ce202695..dead005b0 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -744,6 +744,17 @@ QQuickWebEngineViewAccessible::QQuickWebEngineViewAccessible(QQuickWebEngineView
     : QAccessibleObject(o)
 {}
 
+bool QQuickWebEngineViewAccessible::isValid() const
+{
+    if (!QAccessibleObject::isValid())
+        return false;
+
+    if (!engineView() || !engineView()->d_func())
+        return false;
+
+    return true;
+}
+
 QAccessibleInterface *QQuickWebEngineViewAccessible::parent() const
 {
     QQuickItem *parent = engineView()->parentItem();
@@ -767,9 +778,9 @@ int QQuickWebEngineViewAccessible::childCount() const
 
 QAccessibleInterface *QQuickWebEngineViewAccessible::child(int index) const
 {
-    if (index == 0 && engineView())
+    if (index == 0 && isValid())
         return engineView()->d_func()->adapter->browserAccessible();
-    return 0;
+    return nullptr;
 }
 
 int QQuickWebEngineViewAccessible::indexOfChild(const QAccessibleInterface *c) const
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index 6408767b3..3969ee244 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -234,6 +234,7 @@ class QQuickWebEngineViewAccessible : public QAccessibleObject
 {
 public:
     QQuickWebEngineViewAccessible(QQuickWebEngineView *o);
+    bool isValid() const override;
     QAccessibleInterface *parent() const override;
     QAccessibleInterface *focusChild() const override;
     int childCount() const override;
diff --git a/src/webengine/render_widget_host_view_qt_delegate_quick.cpp b/src/webengine/render_widget_host_view_qt_delegate_quick.cpp
index ede85ca69..e2ce08ebf 100644
--- a/src/webengine/render_widget_host_view_qt_delegate_quick.cpp
+++ b/src/webengine/render_widget_host_view_qt_delegate_quick.cpp
@@ -380,9 +380,17 @@ RenderWidgetHostViewQtDelegateQuickAccessible::RenderWidgetHostViewQtDelegateQui
 {
 }
 
+bool RenderWidgetHostViewQtDelegateQuickAccessible::isValid() const
+{
+    if (!viewAccessible() || !viewAccessible()->isValid())
+        return false;
+
+    return QAccessibleObject::isValid();
+}
+
 QAccessibleInterface *RenderWidgetHostViewQtDelegateQuickAccessible::parent() const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->parent();
+    return viewAccessible()->parent();
 }
 
 QString RenderWidgetHostViewQtDelegateQuickAccessible::text(QAccessible::Text) const
@@ -397,29 +405,33 @@ QAccessible::Role RenderWidgetHostViewQtDelegateQuickAccessible::role() const
 
 QAccessible::State RenderWidgetHostViewQtDelegateQuickAccessible::state() const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->state();
+    return viewAccessible()->state();
 }
 
 QAccessibleInterface *RenderWidgetHostViewQtDelegateQuickAccessible::focusChild() const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->focusChild();
+    return viewAccessible()->focusChild();
 }
 
 int RenderWidgetHostViewQtDelegateQuickAccessible::childCount() const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->childCount();
+    return viewAccessible()->childCount();
 }
 
 QAccessibleInterface *RenderWidgetHostViewQtDelegateQuickAccessible::child(int index) const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->child(index);
+    return viewAccessible()->child(index);
 }
 
 int RenderWidgetHostViewQtDelegateQuickAccessible::indexOfChild(const QAccessibleInterface *c) const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->indexOfChild(c);
+    return viewAccessible()->indexOfChild(c);
 }
 
+QQuickWebEngineViewAccessible *RenderWidgetHostViewQtDelegateQuickAccessible::viewAccessible() const
+{
+    return static_cast<QQuickWebEngineViewAccessible *>(QAccessible::queryAccessibleInterface(m_view));
+}
 #endif // QT_CONFIG(accessibility)
 
 } // namespace QtWebEngineCore
diff --git a/src/webengine/render_widget_host_view_qt_delegate_quick.h b/src/webengine/render_widget_host_view_qt_delegate_quick.h
index 6874aac2b..f70de50d7 100644
--- a/src/webengine/render_widget_host_view_qt_delegate_quick.h
+++ b/src/webengine/render_widget_host_view_qt_delegate_quick.h
@@ -47,6 +47,7 @@
 
 QT_BEGIN_NAMESPACE
 class QQuickWebEngineView;
+class QQuickWebEngineViewAccessible;
 class QQuickWebEngineViewPrivate;
 QT_END_NAMESPACE
 
@@ -122,6 +123,7 @@ class RenderWidgetHostViewQtDelegateQuickAccessible : public QAccessibleObject
 public:
     RenderWidgetHostViewQtDelegateQuickAccessible(RenderWidgetHostViewQtDelegateQuick *o, QQuickWebEngineView *view);
 
+    bool isValid() const override;
     QAccessibleInterface *parent() const override;
     QString text(QAccessible::Text t) const override;
     QAccessible::Role role() const override;
@@ -133,6 +135,7 @@ public:
     int indexOfChild(const QAccessibleInterface *) const override;
 
 private:
+    QQuickWebEngineViewAccessible *viewAccessible() const;
     QQuickWebEngineView *m_view;
 };
 #endif // QT_CONFIG(accessibility)
diff --git a/src/webenginewidgets/api/qwebengineview.cpp b/src/webenginewidgets/api/qwebengineview.cpp
index a51f9b7a5..b06d99478 100644
--- a/src/webenginewidgets/api/qwebengineview.cpp
+++ b/src/webenginewidgets/api/qwebengineview.cpp
@@ -471,6 +471,17 @@ void QWebEngineView::dropEvent(QDropEvent *e)
 #endif // QT_CONFIG(draganddrop)
 
 #ifndef QT_NO_ACCESSIBILITY
+bool QWebEngineViewAccessible::isValid() const
+{
+    if (!QAccessibleWidget::isValid())
+        return false;
+
+    if (!view() || !view()->d_func() || !view()->d_func()->page || !view()->d_func()->page->d_func())
+        return false;
+
+    return true;
+}
+
 QAccessibleInterface *QWebEngineViewAccessible::focusChild() const
 {
     if (child(0) && child(0)->focusChild())
@@ -485,7 +496,7 @@ int QWebEngineViewAccessible::childCount() const
 
 QAccessibleInterface *QWebEngineViewAccessible::child(int index) const
 {
-    if (index == 0 && view() && view()->page())
+    if (index == 0 && isValid())
         return view()->page()->d_func()->adapter->browserAccessible();
     return nullptr;
 }
diff --git a/src/webenginewidgets/api/qwebengineview.h b/src/webenginewidgets/api/qwebengineview.h
index 63a68f46c..04c8ece74 100644
--- a/src/webenginewidgets/api/qwebengineview.h
+++ b/src/webenginewidgets/api/qwebengineview.h
@@ -53,6 +53,7 @@ class QContextMenuEvent;
 class QUrl;
 class QWebEnginePage;
 class QWebEngineSettings;
+class QWebEngineViewAccessible;
 class QWebEngineViewPrivate;
 
 class QWEBENGINEWIDGETS_EXPORT QWebEngineView : public QWidget {
@@ -141,6 +142,9 @@ private:
 
     friend class QWebEnginePage;
     friend class QWebEnginePagePrivate;
+#if QT_CONFIG(accessibility)
+    friend class QWebEngineViewAccessible;
+#endif
 };
 
 QT_END_NAMESPACE
diff --git a/src/webenginewidgets/api/qwebengineview_p.h b/src/webenginewidgets/api/qwebengineview_p.h
index dd0a5bedf..a3c619cea 100644
--- a/src/webenginewidgets/api/qwebengineview_p.h
+++ b/src/webenginewidgets/api/qwebengineview_p.h
@@ -87,6 +87,7 @@ public:
     QWebEngineViewAccessible(QWebEngineView *o) : QAccessibleWidget(o)
     {}
 
+    bool isValid() const override;
     QAccessibleInterface *focusChild() const override;
     int childCount() const override;
     QAccessibleInterface *child(int index) const override;
diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
index 8ba312822..fdd6d1c4f 100644
--- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
+++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
@@ -41,6 +41,7 @@
 
 #include "qwebenginepage_p.h"
 #include "qwebengineview.h"
+#include "qwebengineview_p.h"
 #include <QGuiApplication>
 #include <QLayout>
 #include <QMouseEvent>
@@ -490,24 +491,37 @@ RenderWidgetHostViewQtDelegateWidgetAccessible::RenderWidgetHostViewQtDelegateWi
 {
 }
 
+bool RenderWidgetHostViewQtDelegateWidgetAccessible::isValid() const
+{
+    if (!viewAccessible() || !viewAccessible()->isValid())
+        return false;
+
+    return QAccessibleWidget::isValid();
+}
+
 QAccessibleInterface *RenderWidgetHostViewQtDelegateWidgetAccessible::focusChild() const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->focusChild();
+    return viewAccessible()->focusChild();
 }
 
 int RenderWidgetHostViewQtDelegateWidgetAccessible::childCount() const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->childCount();
+    return viewAccessible()->childCount();
 }
 
 QAccessibleInterface *RenderWidgetHostViewQtDelegateWidgetAccessible::child(int index) const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->child(index);
+    return viewAccessible()->child(index);
 }
 
 int RenderWidgetHostViewQtDelegateWidgetAccessible::indexOfChild(const QAccessibleInterface *c) const
 {
-    return QAccessible::queryAccessibleInterface(m_view)->indexOfChild(c);
+    return viewAccessible()->indexOfChild(c);
+}
+
+QWebEngineViewAccessible *RenderWidgetHostViewQtDelegateWidgetAccessible::viewAccessible() const
+{
+    return static_cast<QWebEngineViewAccessible *>(QAccessible::queryAccessibleInterface(m_view));
 }
 #endif // QT_CONFIG(accessibility)
 
diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
index df1806b6f..c7783117a 100644
--- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
+++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
@@ -50,6 +50,7 @@
 QT_BEGIN_NAMESPACE
 class QWebEnginePage;
 class QWebEngineView;
+class QWebEngineViewAccessible;
 class QWebEnginePagePrivate;
 QT_END_NAMESPACE
 
@@ -123,11 +124,14 @@ class RenderWidgetHostViewQtDelegateWidgetAccessible : public QAccessibleWidget
 public:
     RenderWidgetHostViewQtDelegateWidgetAccessible(RenderWidgetHostViewQtDelegateWidget *o, QWebEngineView *view);
 
+    bool isValid() const override;
     QAccessibleInterface *focusChild() const override;
     int childCount() const override;
     QAccessibleInterface *child(int index) const override;
     int indexOfChild(const QAccessibleInterface *child) const override;
+
 private:
+    QWebEngineViewAccessible *viewAccessible() const;
     QWebEngineView *m_view;
 };
 #endif // QT_CONFIG(accessibility)