Skip to content

Commit

Permalink
refactor: JSify BrowserWindow unresponsive handling (#37902)
Browse files Browse the repository at this point in the history
  • Loading branch information
nornagon committed Apr 18, 2024
1 parent b683754 commit 67ba304
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 59 deletions.
23 changes: 23 additions & 0 deletions lib/browser/api/browser-window.ts
Expand Up @@ -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 = () => {
Expand Down
46 changes: 0 additions & 46 deletions shell/browser/api/electron_api_browser_window.cc
Expand Up @@ -117,31 +117,13 @@ 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() {
api_web_contents_ = nullptr;
CloseImmediately();
}

void BrowserWindow::OnRendererResponsive(content::RenderProcessHost*) {
window_unresponsive_closure_.Cancel();
Emit("responsive");
}

void BrowserWindow::OnSetContentBounds(const gfx::Rect& rect) {
// window.resizeTo(...)
// window.moveTo(...)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
Expand Down
13 changes: 0 additions & 13 deletions shell/browser/api/electron_api_browser_window.h
Expand Up @@ -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:
Expand Down Expand Up @@ -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<v8::Value> web_contents_;
v8::Global<v8::Value> web_contents_view_;
base::WeakPtr<api::WebContents> api_web_contents_;
Expand Down

0 comments on commit 67ba304

Please sign in to comment.