From 67ba30402bcbf6942fa0846927f2d088cf0a749d Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Thu, 18 Apr 2024 13:14:07 -0700 Subject: [PATCH] refactor: JSify BrowserWindow unresponsive handling (#37902) --- lib/browser/api/browser-window.ts | 23 ++++++++++ .../api/electron_api_browser_window.cc | 46 ------------------- .../browser/api/electron_api_browser_window.h | 13 ------ 3 files changed, 23 insertions(+), 59 deletions(-) diff --git a/lib/browser/api/browser-window.ts b/lib/browser/api/browser-window.ts index b6ec4e60b6d5..365258200f2c 100644 --- a/lib/browser/api/browser-window.ts +++ b/lib/browser/api/browser-window.ts @@ -36,6 +36,29 @@ 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', () => { + if (!unresponsiveEvent) { 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 = () => { diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc index acbbd88cb468..234dedb643ea 100644 --- a/shell/browser/api/electron_api_browser_window.cc +++ b/shell/browser/api/electron_api_browser_window.cc @@ -117,19 +117,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() { @@ -137,11 +124,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(...) @@ -177,13 +159,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; @@ -250,9 +225,6 @@ void BrowserWindow::CloseImmediately() { } BaseWindow::CloseImmediately(); - - // Do not sent "unresponsive" event after window is closed. - window_unresponsive_closure_.Cancel(); } void BrowserWindow::Focus() { @@ -362,24 +334,6 @@ void BrowserWindow::SetTitleBarOverlay(const gin_helper::Dictionary& options, } #endif -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(); diff --git a/shell/browser/api/electron_api_browser_window.h b/shell/browser/api/electron_api_browser_window.h index df9f9754ce0f..c9de1334a40c 100644 --- a/shell/browser/api/electron_api_browser_window.h +++ b/shell/browser/api/electron_api_browser_window.h @@ -43,9 +43,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: @@ -84,16 +81,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 web_contents_; v8::Global web_contents_view_; base::WeakPtr api_web_contents_;