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

---------

Co-authored-by: Keeley Hammond <vertedinde@electronjs.org>
This commit is contained in:
Sam Maddock 2024-11-13 20:59:13 -05:00 коммит произвёл GitHub
Родитель 946ab5f1d7
Коммит 41b5d7e312
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
7 изменённых файлов: 103 добавлений и 60 удалений

Просмотреть файл

@ -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 = () => {

Просмотреть файл

@ -920,6 +920,13 @@ WebContents.prototype._init = function () {
this.emit('console-message', event, (event as any)._level, event.message, event.lineNumber, event.sourceId);
});
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

Просмотреть файл

@ -133,4 +133,5 @@ osr_shared_texture_remove_keyed_mutex_on_win_dxgi.patch
feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch
chore_partial_revert_of.patch
fix_software_compositing_infinite_loop.patch
refactor_unfilter_unresponsive_events.patch
build_disable_thin_lto_mac.patch

Просмотреть файл

@ -0,0 +1,47 @@
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 319800cec84a968b0e442fc760c8e1d701bda2ed..459663af86272fe1e23a6a163e01c67fc5f8a66d 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -9464,25 +9464,13 @@ 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;
RecordRendererUnresponsiveMetrics(visible, render_widget_host);
- // 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;
- }
-
- if (!render_widget_host->renderer_initialized()) {
- return;
- }
-
CrashRepHandlingOutcome outcome =
base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kNoErrorDialogs)

Просмотреть файл

@ -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();

Просмотреть файл

@ -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_;

Просмотреть файл

@ -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"
@ -1436,7 +1438,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(