fix: revert BrowserWindow unresponsive handling refactor (#43053)

* Revert "refactor: JSify BrowserWindow unresponsive handling (#37902)"

This reverts commit 67ba30402b.

Co-authored-by: Keeley Hammond <khammond@slack-corp.com>

* chore: remove BrowserWindow::SetTitleBarOverlay

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

lib/browser/api/browser-window.ts

@@ -36,29 +36,6 @@ 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 = () => {

shell/browser/api/electron_api_browser_window.cc

@@ -112,6 +112,19 @@ 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() {
@@ -119,6 +132,11 @@ 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(...)
@@ -154,6 +172,13 @@ 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;
@@ -220,6 +245,9 @@ void BrowserWindow::CloseImmediately() {
   }
 
   BaseWindow::CloseImmediately();
+
+  // Do not sent "unresponsive" event after window is closed.
+  window_unresponsive_closure_.Cancel();
 }
 
 void BrowserWindow::Focus() {
@@ -270,6 +298,24 @@ 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

@@ -43,6 +43,9 @@ 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:
@@ -77,6 +80,16 @@ 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_;