refactor: JSify BrowserWindow unresponsive handling (#37902)
This commit is contained in:
		
					parent
					
						
							
								b683754c16
							
						
					
				
			
			
				commit
				
					
						67ba30402b
					
				
			
		
					 3 changed files with 23 additions and 59 deletions
				
			
		|  | @ -36,6 +36,29 @@ BrowserWindow.prototype._init = function (this: BWT) { | ||||||
|     app.emit('browser-window-focus', event, this); |     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.
 |   // Subscribe to visibilityState changes and pass to renderer process.
 | ||||||
|   let isVisible = this.isVisible() && !this.isMinimized(); |   let isVisible = this.isVisible() && !this.isMinimized(); | ||||||
|   const visibilityChanged = () => { |   const visibilityChanged = () => { | ||||||
|  |  | ||||||
|  | @ -117,19 +117,6 @@ BrowserWindow::~BrowserWindow() { | ||||||
| 
 | 
 | ||||||
| void BrowserWindow::BeforeUnloadDialogCancelled() { | void BrowserWindow::BeforeUnloadDialogCancelled() { | ||||||
|   WindowList::WindowCloseCancelled(window()); |   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() { | void BrowserWindow::WebContentsDestroyed() { | ||||||
|  | @ -137,11 +124,6 @@ void BrowserWindow::WebContentsDestroyed() { | ||||||
|   CloseImmediately(); |   CloseImmediately(); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| void BrowserWindow::OnRendererResponsive(content::RenderProcessHost*) { |  | ||||||
|   window_unresponsive_closure_.Cancel(); |  | ||||||
|   Emit("responsive"); |  | ||||||
| } |  | ||||||
| 
 |  | ||||||
| void BrowserWindow::OnSetContentBounds(const gfx::Rect& rect) { | void BrowserWindow::OnSetContentBounds(const gfx::Rect& rect) { | ||||||
|   // window.resizeTo(...)
 |   // window.resizeTo(...)
 | ||||||
|   // window.moveTo(...)
 |   // 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.
 |   // first, and when the web page is closed the window will also be closed.
 | ||||||
|   *prevent_default = true; |   *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.
 |   // Already closed by renderer.
 | ||||||
|   if (!web_contents() || !api_web_contents_) |   if (!web_contents() || !api_web_contents_) | ||||||
|     return; |     return; | ||||||
|  | @ -250,9 +225,6 @@ void BrowserWindow::CloseImmediately() { | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|   BaseWindow::CloseImmediately(); |   BaseWindow::CloseImmediately(); | ||||||
| 
 |  | ||||||
|   // Do not sent "unresponsive" event after window is closed.
 |  | ||||||
|   window_unresponsive_closure_.Cancel(); |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| void BrowserWindow::Focus() { | void BrowserWindow::Focus() { | ||||||
|  | @ -362,24 +334,6 @@ void BrowserWindow::SetTitleBarOverlay(const gin_helper::Dictionary& options, | ||||||
| } | } | ||||||
| #endif | #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() { | void BrowserWindow::OnWindowShow() { | ||||||
|   web_contents()->WasShown(); |   web_contents()->WasShown(); | ||||||
|   BaseWindow::OnWindowShow(); |   BaseWindow::OnWindowShow(); | ||||||
|  |  | ||||||
|  | @ -43,9 +43,6 @@ class BrowserWindow : public BaseWindow, | ||||||
| 
 | 
 | ||||||
|   // content::WebContentsObserver:
 |   // content::WebContentsObserver:
 | ||||||
|   void BeforeUnloadDialogCancelled() override; |   void BeforeUnloadDialogCancelled() override; | ||||||
|   void OnRendererUnresponsive(content::RenderProcessHost*) override; |  | ||||||
|   void OnRendererResponsive( |  | ||||||
|       content::RenderProcessHost* render_process_host) override; |  | ||||||
|   void WebContentsDestroyed() override; |   void WebContentsDestroyed() override; | ||||||
| 
 | 
 | ||||||
|   // ExtendedWebContentsObserver:
 |   // ExtendedWebContentsObserver:
 | ||||||
|  | @ -84,16 +81,6 @@ class BrowserWindow : public BaseWindow, | ||||||
|  private: |  private: | ||||||
|   // Helpers.
 |   // 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_; | ||||||
|   v8::Global<v8::Value> web_contents_view_; |   v8::Global<v8::Value> web_contents_view_; | ||||||
|   base::WeakPtr<api::WebContents> api_web_contents_; |   base::WeakPtr<api::WebContents> api_web_contents_; | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Jeremy Rose
				Jeremy Rose