From a0a5b20ef18a26857c67feb6dc51d456e6455035 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 29 Nov 2013 14:52:12 +0800 Subject: [PATCH 1/3] Send the "unresponsive" event if window is not closed in 500ms when closing it. --- browser/native_window.cc | 19 ++++++++++++++++--- browser/native_window.h | 3 +++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/browser/native_window.cc b/browser/native_window.cc index b3bb5efc5d0c..c656edbf6667 100644 --- a/browser/native_window.cc +++ b/browser/native_window.cc @@ -45,6 +45,7 @@ NativeWindow::NativeWindow(content::WebContents* web_contents, has_frame_(true), is_closed_(false), not_responding_(false), + weak_factory_(this), inspectable_web_contents_( brightray::InspectableWebContents::Create(web_contents)) { options->GetBoolean(switches::kFrame, &has_frame_); @@ -205,7 +206,7 @@ void NativeWindow::CapturePage(const gfx::Rect& rect, rect, gfx::Size(), base::Bind(&NativeWindow::OnCapturePageDone, - base::Unretained(this), + weak_factory_.GetWeakPtr(), callback)); } @@ -221,6 +222,12 @@ void NativeWindow::CloseWebContents() { content::WebContents* web_contents(GetWebContents()); + // Assume the window is not responding if it doesn't cancel the close and is + // not closed in 500ms, in this way we can quickly show the unresponsive + // dialog when the window is busy executing some script withouth waiting for + // the unresponsive timeout. + RendererUnresponsive(web_contents); + if (web_contents->NeedToFireBeforeUnload()) web_contents->GetRenderViewHost()->FirePageBeforeUnload(false); else @@ -277,6 +284,9 @@ void NativeWindow::BeforeUnloadFired(content::WebContents* tab, if (!proceed) WindowList::WindowCloseCancelled(this); + + // When the "beforeunload" callback is fired the window is certainly live. + not_responding_ = false; } void NativeWindow::RequestToLockMouse(content::WebContents* web_contents, @@ -319,6 +329,9 @@ void NativeWindow::CloseContents(content::WebContents* source) { CloseImmediately(); NotifyWindowClosed(); + + // Do not sent "unresponsive" event after window is closed. + not_responding_ = false; } bool NativeWindow::IsPopupOrPanel(const content::WebContents* source) const { @@ -331,8 +344,8 @@ void NativeWindow::RendererUnresponsive(content::WebContents* source) { base::MessageLoop::current()->PostDelayedTask( FROM_HERE, base::Bind(&NativeWindow::RendererUnresponsiveDelayed, - base::Unretained(this)), - base::TimeDelta::FromSeconds(1)); + weak_factory_.GetWeakPtr()), + base::TimeDelta::FromMilliseconds(500)); } void NativeWindow::RendererResponsive(content::WebContents* source) { diff --git a/browser/native_window.h b/browser/native_window.h index 2506afb55071..316f6f5fe18e 100644 --- a/browser/native_window.h +++ b/browser/native_window.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "browser/native_window_observer.h" #include "content/public/browser/notification_registrar.h" @@ -216,6 +217,8 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, // The window is not responding. bool not_responding_; + base::WeakPtrFactory weak_factory_; + scoped_ptr dialog_manager_; scoped_ptr inspectable_web_contents_; From 02d14ed23b782db1b9302a33791c8395cce06898 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 29 Nov 2013 15:19:00 +0800 Subject: [PATCH 2/3] Fix returning the chosen option for synchronous dialogs. --- browser/api/atom_api_dialog.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index e5d52c43472a..063ed0d4cffb 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -59,9 +59,8 @@ v8::Handle ShowMessageBox(const v8::Arguments &args) { return node::ThrowTypeError("Bad argument"); NativeWindow* native_window = FromV8Value(args[5]); - v8::Persistent callback = FromV8Value(args[6]); - if (callback.IsEmpty()) { + if (!args[6]->IsFunction()) { int chosen = atom::ShowMessageBox( native_window, (MessageBoxType)type, @@ -71,6 +70,7 @@ v8::Handle ShowMessageBox(const v8::Arguments &args) { detail); return scope.Close(v8::Integer::New(chosen)); } else { + v8::Persistent callback = FromV8Value(args[6]); atom::ShowMessageBox( native_window, (MessageBoxType)type, @@ -93,9 +93,8 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { return node::ThrowTypeError("Bad argument"); NativeWindow* native_window = FromV8Value(args[3]); - v8::Persistent callback = FromV8Value(args[4]); - if (callback.IsEmpty()) { + if (!args[4]->IsFunction()) { std::vector paths; if (!file_dialog::ShowOpenDialog(native_window, title, @@ -110,6 +109,7 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { return scope.Close(result); } else { + v8::Persistent callback = FromV8Value(args[4]); file_dialog::ShowOpenDialog( native_window, title, @@ -130,9 +130,8 @@ v8::Handle ShowSaveDialog(const v8::Arguments &args) { return node::ThrowTypeError("Bad argument"); NativeWindow* native_window = FromV8Value(args[2]); - v8::Persistent callback = FromV8Value(args[3]); - if (callback.IsEmpty()) { + if (!args[3]->IsFunction()) { base::FilePath path; if (!file_dialog::ShowSaveDialog(native_window, title, @@ -142,6 +141,7 @@ v8::Handle ShowSaveDialog(const v8::Arguments &args) { return scope.Close(ToV8Value(path)); } else { + v8::Persistent callback = FromV8Value(args[3]); file_dialog::ShowSaveDialog( native_window, title, From adc5495d2ba9f1eca925adff4fad105a5283a9d8 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 29 Nov 2013 15:19:30 +0800 Subject: [PATCH 3/3] Prompt unresponsive state in spec window. --- spec/main.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/main.js b/spec/main.js index f9d77d0c2380..d89365124a8e 100644 --- a/spec/main.js +++ b/spec/main.js @@ -1,5 +1,6 @@ var app = require('app'); var ipc = require('ipc'); +var dialog = require('dialog'); var BrowserWindow = require('browser-window'); var Menu = require('menu'); @@ -103,4 +104,13 @@ app.on('finish-launching', function() { height: 600 }); window.loadUrl('file://' + __dirname + '/index.html'); + window.on('unresponsive', function() { + var chosen = dialog.showMessageBox(window, { + type: 'warning', + buttons: ['Close', 'Keep Waiting'], + message: 'Window is not responsing', + detail: 'The window is not responding. Would you like to force close it or just keep waiting?' + }); + if (chosen == 0) window.destroy(); + }); });