refactor: unfilter unresponsive events (#44667)

refactor: unfilter unresponsive events (#44629)

* feat: internal -unresponsive event

* Reland "refactor: JSify BrowserWindow unresponsive handling"

This reverts commit ef7ae78ed4.

* fix: emit unresponsive if close not prevented

---------

lib/browser/api/browser-window.ts

@@ -37,6 +37,33 @@ BrowserWindow.prototype._init = function (this: BWT) {
     app.emit('browser-window-focus', event, this);
   });
 
+  let unresponsiveEvent: NodeJS.Timeout | null = null;
+  const emitUnresponsiveEvent = () => {
+    unresponsiveEvent = null;
+    if (!this.isDestroyed() && this.isEnabled()) { this.emit('unresponsive'); }
+  };
+  this.webContents.on('unresponsive', () => {
+    if (!unresponsiveEvent) { unresponsiveEvent = setTimeout(emitUnresponsiveEvent, 50); }
+  });
+  this.webContents.on('responsive', () => {
+    if (unresponsiveEvent) {
+      clearTimeout(unresponsiveEvent);
+      unresponsiveEvent = null;
+    }
+    this.emit('responsive');
+  });
+  this.on('close', (event) => {
+    queueMicrotask(() => {
+      if (!unresponsiveEvent && !event.defaultPrevented) {
+        unresponsiveEvent = setTimeout(emitUnresponsiveEvent, 5000);
+      }
+    });
+  });
+  this.webContents.on('destroyed', () => {
+    if (unresponsiveEvent) clearTimeout(unresponsiveEvent);
+    unresponsiveEvent = null;
+  });
+
   // Subscribe to visibilityState changes and pass to renderer process.
   let isVisible = this.isVisible() && !this.isMinimized();
   const visibilityChanged = () => {

lib/browser/api/web-contents.ts

@@ -909,6 +909,13 @@ WebContents.prototype._init = function () {
     openDialogs.clear();
   });
 
+  this.on('-unresponsive' as any, (event: Electron.Event<any>) => {
+    const shouldEmit = !event.shouldIgnore && event.visible && event.rendererInitialized;
+    if (shouldEmit) {
+      this.emit('unresponsive', event);
+    }
+  });
+
   app.emit('web-contents-created', { sender: this, preventDefault () {}, get defaultPrevented () { return false; } }, this);
 
   // Properties

patches/chromium/.patches

@@ -134,3 +134,4 @@ fix_software_compositing_infinite_loop.patch
 ui_add_missing_shortcut_text_for_vkey_command_on_linux.patch
 osr_shared_texture_remove_keyed_mutex_on_win_dxgi.patch
 cherry-pick-923797bac925.patch
+refactor_unfilter_unresponsive_events.patch

patches/chromium/refactor_unfilter_unresponsive_events.patch

@@ -0,0 +1,51 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Samuel Maddock <smaddock@slack-corp.com>
+Date: Tue, 12 Nov 2024 17:30:42 -0500
+Subject: refactor: unfilter unresponsive events
+
+RendererUnresponsive events are filtered out when
+* DisableHangMonitor switch is set
+* WebContents is awaiting a clipboard scan
+* Any remote debugger is attached
+* Any local debugger is attached
+* Visibility state is not visible
+* Renderer process is not initialized
+
+This CL removes these filters so the unresponsive event can still be
+accessed from our JS event. The filtering is moved into Electron's code.
+
+diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
+index ad1f4c8f5dbe0da06955205e5c8a34908c04a0cd..fc720ea3ef4a45da258075f87da239fa9c3b0e82 100644
+--- a/content/browser/web_contents/web_contents_impl.cc
++++ b/content/browser/web_contents/web_contents_impl.cc
+@@ -9410,29 +9410,17 @@ void WebContentsImpl::RendererUnresponsive(
+     base::RepeatingClosure hang_monitor_restarter) {
+   OPTIONAL_TRACE_EVENT1("content", "WebContentsImpl::RendererUnresponsive",
+                         "render_widget_host", render_widget_host);
+-  if (ShouldIgnoreUnresponsiveRenderer()) {
++  if (IsBeingDestroyed()) {
+     return;
+   }
+ 
+   bool visible = GetVisibility() == Visibility::VISIBLE;
+   base::UmaHistogramBoolean("Renderer.Unresponsive.Visibility", visible);
+ 
+-  // Do not report hangs (to task manager, to hang renderer dialog, etc.) for
+-  // invisible tabs (like extension background page, background tabs).  See
+-  // https://crbug.com/881812 for rationale and for choosing the visibility
+-  // (rather than process priority) as the signal here.
+-  if (!visible) {
+-    return;
+-  }
+-
+   base::UmaHistogramBoolean(
+       "Renderer.Unresponsive.PageVisible.WidgetVisibility",
+       !render_widget_host->is_hidden());
+ 
+-  if (!render_widget_host->renderer_initialized()) {
+-    return;
+-  }
+-
+   CrashRepHandlingOutcome outcome =
+       base::CommandLine::ForCurrentProcess()->HasSwitch(
+           switches::kNoErrorDialogs)

shell/browser/api/electron_api_browser_window.cc

@@ -108,19 +108,6 @@ BrowserWindow::~BrowserWindow() {
 
 void BrowserWindow::BeforeUnloadDialogCancelled() {
   WindowList::WindowCloseCancelled(window());
-  // Cancel unresponsive event when window close is cancelled.
-  window_unresponsive_closure_.Cancel();
-}
-
-void BrowserWindow::OnRendererUnresponsive(content::RenderProcessHost*) {
-  // Schedule the unresponsive shortly later, since we may receive the
-  // responsive event soon. This could happen after the whole application had
-  // blocked for a while.
-  // Also notice that when closing this event would be ignored because we have
-  // explicitly started a close timeout counter. This is on purpose because we
-  // don't want the unresponsive event to be sent too early when user is closing
-  // the window.
-  ScheduleUnresponsiveEvent(50);
 }
 
 void BrowserWindow::WebContentsDestroyed() {
@@ -128,11 +115,6 @@ void BrowserWindow::WebContentsDestroyed() {
   CloseImmediately();
 }
 
-void BrowserWindow::OnRendererResponsive(content::RenderProcessHost*) {
-  window_unresponsive_closure_.Cancel();
-  Emit("responsive");
-}
-
 void BrowserWindow::OnSetContentBounds(const gfx::Rect& rect) {
   // window.resizeTo(...)
   // window.moveTo(...)
@@ -168,13 +150,6 @@ void BrowserWindow::OnCloseButtonClicked(bool* prevent_default) {
   // first, and when the web page is closed the window will also be closed.
   *prevent_default = true;
 
-  // Assume the window is not responding if it doesn't cancel the close and is
-  // not closed in 5s, in this way we can quickly show the unresponsive
-  // dialog when the window is busy executing some script without waiting for
-  // the unresponsive timeout.
-  if (window_unresponsive_closure_.IsCancelled())
-    ScheduleUnresponsiveEvent(5000);
-
   // Already closed by renderer.
   if (!web_contents() || !api_web_contents_)
     return;
@@ -241,9 +216,6 @@ void BrowserWindow::CloseImmediately() {
   }
 
   BaseWindow::CloseImmediately();
-
-  // Do not sent "unresponsive" event after window is closed.
-  window_unresponsive_closure_.Cancel();
 }
 
 void BrowserWindow::Focus() {
@@ -294,24 +266,6 @@ v8::Local<v8::Value> BrowserWindow::GetWebContents(v8::Isolate* isolate) {
   return v8::Local<v8::Value>::New(isolate, web_contents_);
 }
 
-void BrowserWindow::ScheduleUnresponsiveEvent(int ms) {
-  if (!window_unresponsive_closure_.IsCancelled())
-    return;
-
-  window_unresponsive_closure_.Reset(base::BindRepeating(
-      &BrowserWindow::NotifyWindowUnresponsive, GetWeakPtr()));
-  base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
-      FROM_HERE, window_unresponsive_closure_.callback(),
-      base::Milliseconds(ms));
-}
-
-void BrowserWindow::NotifyWindowUnresponsive() {
-  window_unresponsive_closure_.Cancel();
-  if (!window_->IsClosed() && window_->IsEnabled()) {
-    Emit("unresponsive");
-  }
-}
-
 void BrowserWindow::OnWindowShow() {
   web_contents()->WasShown();
   BaseWindow::OnWindowShow();

shell/browser/api/electron_api_browser_window.h

@@ -46,9 +46,6 @@ class BrowserWindow : public BaseWindow,
 
   // content::WebContentsObserver:
   void BeforeUnloadDialogCancelled() override;
-  void OnRendererUnresponsive(content::RenderProcessHost*) override;
-  void OnRendererResponsive(
-      content::RenderProcessHost* render_process_host) override;
   void WebContentsDestroyed() override;
 
   // ExtendedWebContentsObserver:
@@ -83,16 +80,6 @@ class BrowserWindow : public BaseWindow,
  private:
   // Helpers.
 
-  // Schedule a notification unresponsive event.
-  void ScheduleUnresponsiveEvent(int ms);
-
-  // Dispatch unresponsive event to observers.
-  void NotifyWindowUnresponsive();
-
-  // Closure that would be called when window is unresponsive when closing,
-  // it should be cancelled when we can prove that the window is responsive.
-  base::CancelableRepeatingClosure window_unresponsive_closure_;
-
   v8::Global<v8::Value> web_contents_;
   v8::Global<v8::Value> web_contents_view_;
   base::WeakPtr<api::WebContents> api_web_contents_;

shell/browser/api/electron_api_web_contents.cc

@@ -43,6 +43,7 @@
 #include "content/browser/renderer_host/render_frame_host_manager.h"  // nogncheck
 #include "content/browser/renderer_host/render_widget_host_impl.h"  // nogncheck
 #include "content/browser/renderer_host/render_widget_host_view_base.h"  // nogncheck
+#include "content/browser/web_contents/web_contents_impl.h"  // nogncheck
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/context_menu_params.h"
 #include "content/public/browser/desktop_media_id.h"
@@ -62,6 +63,7 @@
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/storage_partition.h"
+#include "content/public/browser/visibility.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/referrer_type_converters.h"
 #include "content/public/common/result_codes.h"
@@ -1418,7 +1420,25 @@ void WebContents::RendererUnresponsive(
     content::WebContents* source,
     content::RenderWidgetHost* render_widget_host,
     base::RepeatingClosure hang_monitor_restarter) {
-  Emit("unresponsive");
+  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+  gin::Handle<gin_helper::internal::Event> event =
+      gin_helper::internal::Event::New(isolate);
+  v8::Local<v8::Object> event_object = event.ToV8().As<v8::Object>();
+  gin::Dictionary dict(isolate, event_object);
+
+  auto* web_contents_impl = static_cast<content::WebContentsImpl*>(source);
+  bool should_ignore = web_contents_impl->ShouldIgnoreUnresponsiveRenderer();
+  dict.Set("shouldIgnore", should_ignore);
+
+  bool visible = source->GetVisibility() == content::Visibility::VISIBLE;
+  dict.Set("visible", visible);
+
+  auto* rwh_impl =
+      static_cast<content::RenderWidgetHostImpl*>(render_widget_host);
+  dict.Set("rendererInitialized", rwh_impl->renderer_initialized());
+
+  EmitWithoutEvent("-unresponsive", event);
 }
 
 void WebContents::RendererResponsive(