Fix renderProcessTerminated signal

With the adaptations for Chromium 76, RenderWidgetHostViewQt was changed to
become a RenderProcessHostObserver with the renderProcessTerminated signal being
emitted from the override of RenderProcessHostObserver::RenderProcessExited.

The problem with this can be seen by setting a breakpoint on the
RenderProcessGone override in RenderWidgetHostViewQt. We then get the trace:

  QtWebEngineCore::RenderWidgetHostViewQt::RenderProcessGone
  content::RenderWidgetHostImpl::RendererExited()
  content::RenderViewHostImpl::RenderProcessExited
  content::RenderProcessHostImpl::ProcessDied

ProcessDied iterates over all the observers and calls RenderProcessExited. Both
the RenderViewHostImpl and our RWHVQt are observers, but the RVHImpl comes
first. The RVHImpl then calls RendererExited, which calls our RenderProcessGone,
which does a 'delete this'. Now our RenderProcessExited override can never be
called because we have already deleted our observer.

Fix by moving the RenderProcessGone code to WebContentsDelegateQt and getting
the exit code from WebContents::GetCrashedErrorCode. Also add test.

Task-number: QTBUG-80085
Change-Id: I434744286df97a37b64722d7c15a1d4ee11c8af6
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

4 files changed, 30 insertions, 21 deletions

diff --git a/src/core/render_widget_host_view_qt.cpp b/src/core/render_widget_host_view_qt.cpp
index e2fd074ae..15cc5174e 100644
--- a/src/core/render_widget_host_view_qt.cpp
+++ b/src/core/render_widget_host_view_qt.cpp
@@ -340,7 +340,6 @@ RenderWidgetHostViewQt::RenderWidgetHostViewQt(content::RenderWidgetHost *widget
 
     // May call SetNeedsBeginFrames
     host()->SetView(this);
-    host()->GetProcess()->AddObserver(this);
 }
 
 RenderWidgetHostViewQt::~RenderWidgetHostViewQt()
@@ -354,7 +353,6 @@ RenderWidgetHostViewQt::~RenderWidgetHostViewQt()
 
     if (text_input_manager_)
         text_input_manager_->RemoveObserver(this);
-    host()->GetProcess()->RemoveObserver(this);
 
     m_touchSelectionController.reset();
     m_touchSelectionControllerClient.reset();
@@ -704,18 +702,6 @@ void RenderWidgetHostViewQt::ImeCompositionRangeChanged(const gfx::Range&, const
     QT_NOT_YET_IMPLEMENTED
 }
 
-void RenderWidgetHostViewQt::RenderProcessExited(content::RenderProcessHost *host,
-                                                 const content::ChildProcessTerminationInfo &info)
-{
-    Q_UNUSED(host);
-    // RenderProcessHost::FastShutdownIfPossible results in TERMINATION_STATUS_STILL_RUNNING
-    if (m_adapterClient && info.status != base::TERMINATION_STATUS_STILL_RUNNING) {
-        m_adapterClient->renderProcessTerminated(
-                    m_adapterClient->renderProcessExitStatus(info.status),
-                    info.exit_code);
-    }
-}
-
 void RenderWidgetHostViewQt::RenderProcessGone()
 {
     Destroy();
diff --git a/src/core/render_widget_host_view_qt.h b/src/core/render_widget_host_view_qt.h
index 76807b37a..0e9d54b19 100644
--- a/src/core/render_widget_host_view_qt.h
+++ b/src/core/render_widget_host_view_qt.h
@@ -102,7 +102,6 @@ struct MultipleMouseClickHelper
 
 class RenderWidgetHostViewQt
     : public content::RenderWidgetHostViewBase
-    , public content::RenderProcessHostObserver
     , public ui::GestureProviderClient
     , public RenderWidgetHostViewQtDelegateClient
     , public base::SupportsWeakPtr<RenderWidgetHostViewQt>
@@ -176,10 +175,6 @@ public:
     void DidStopFlinging() override;
     std::unique_ptr<content::SyntheticGestureTarget> CreateSyntheticGestureTarget() override;
 
-    // RenderProcessHostObserver implementation.
-    void RenderProcessExited(content::RenderProcessHost *host,
-                             const content::ChildProcessTerminationInfo &info) override;
-
     // Overridden from ui::GestureProviderClient.
     void OnGestureEvent(const ui::GestureEventData& gesture) override;
 
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index 255ff0034..2a89556cf 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -286,10 +286,17 @@ void WebContentsDelegateQt::RenderFrameDeleted(content::RenderFrameHost *render_
 
 void WebContentsDelegateQt::RenderProcessGone(base::TerminationStatus status)
 {
+    // RenderProcessHost::FastShutdownIfPossible results in TERMINATION_STATUS_STILL_RUNNING
+    if (status != base::TERMINATION_STATUS_STILL_RUNNING) {
+        m_viewClient->renderProcessTerminated(
+                m_viewClient->renderProcessExitStatus(status),
+                web_contents()->GetCrashedErrorCode());
+    }
+
     // Based one TabLoadTracker::RenderProcessGone
 
-    if (status == base::TerminationStatus::TERMINATION_STATUS_NORMAL_TERMINATION
-        || status == base::TerminationStatus::TERMINATION_STATUS_STILL_RUNNING) {
+    if (status == base::TERMINATION_STATUS_NORMAL_TERMINATION
+        || status == base::TERMINATION_STATUS_STILL_RUNNING) {
         return;
     }
 
diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
index 27aa7a1f7..88cdcbb96 100644
--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
+++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
@@ -225,6 +225,7 @@ private Q_SLOTS:
     void editActionsWithoutSelection();
 
     void customUserAgentInNewTab();
+    void renderProcessCrashed();
 
 private:
     static QPoint elementCenter(QWebEnginePage *page, const QString &id);
@@ -4410,6 +4411,26 @@ void tst_QWebEnginePage::customUserAgentInNewTab()
     QCOMPARE(lastUserAgent, profile2.httpUserAgent().toUtf8());
 }
 
+void tst_QWebEnginePage::renderProcessCrashed()
+{
+    using Status = QWebEnginePage::RenderProcessTerminationStatus;
+    QWebEngineProfile profile;
+    QWebEnginePage page(&profile);
+    bool done = false;
+    Status status;
+    connect(&page, &QWebEnginePage::renderProcessTerminated, [&](Status newStatus) {
+        status = newStatus;
+        done = true;
+    });
+    page.load(QUrl("chrome://crash"));
+    QTRY_VERIFY_WITH_TIMEOUT(done, 20000);
+    // The status depends on whether stack traces are enabled. With
+    // --disable-in-process-stack-traces we get an AbnormalTerminationStatus,
+    // otherwise a CrashedTerminationStatus.
+    QVERIFY(status == QWebEnginePage::CrashedTerminationStatus ||
+            status == QWebEnginePage::AbnormalTerminationStatus);
+}
+
 static QByteArrayList params = {QByteArrayLiteral("--use-fake-device-for-media-stream")};
 W_QTEST_MAIN(tst_QWebEnginePage, params)