Merge pull request #130 from atom/force-closing

Send the "unresponsive" event if window does not close or cancel the close in 500ms when closing it.

browser/api/atom_api_dialog.cc

@@ -59,9 +59,8 @@ v8::Handle<v8::Value> ShowMessageBox(const v8::Arguments &args) {
     return node::ThrowTypeError("Bad argument");
 
   NativeWindow* native_window = FromV8Value(args[5]);
-  v8::Persistent<v8::Function> 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<v8::Value> ShowMessageBox(const v8::Arguments &args) {
         detail);
     return scope.Close(v8::Integer::New(chosen));
   } else {
+    v8::Persistent<v8::Function> callback = FromV8Value(args[6]);
     atom::ShowMessageBox(
         native_window,
         (MessageBoxType)type,
@@ -93,9 +93,8 @@ v8::Handle<v8::Value> ShowOpenDialog(const v8::Arguments &args) {
     return node::ThrowTypeError("Bad argument");
 
   NativeWindow* native_window = FromV8Value(args[3]);
-  v8::Persistent<v8::Function> callback = FromV8Value(args[4]);
 
-  if (callback.IsEmpty()) {
+  if (!args[4]->IsFunction()) {
     std::vector<base::FilePath> paths;
     if (!file_dialog::ShowOpenDialog(native_window,
                                      title,
@@ -110,6 +109,7 @@ v8::Handle<v8::Value> ShowOpenDialog(const v8::Arguments &args) {
 
     return scope.Close(result);
   } else {
+    v8::Persistent<v8::Function> callback = FromV8Value(args[4]);
     file_dialog::ShowOpenDialog(
         native_window,
         title,
@@ -130,9 +130,8 @@ v8::Handle<v8::Value> ShowSaveDialog(const v8::Arguments &args) {
     return node::ThrowTypeError("Bad argument");
 
   NativeWindow* native_window = FromV8Value(args[2]);
-  v8::Persistent<v8::Function> 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<v8::Value> ShowSaveDialog(const v8::Arguments &args) {
 
     return scope.Close(ToV8Value(path));
   } else {
+    v8::Persistent<v8::Function> callback = FromV8Value(args[3]);
     file_dialog::ShowSaveDialog(
         native_window,
         title,

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) {

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<NativeWindow> weak_factory_;
+
   scoped_ptr<AtomJavaScriptDialogManager> dialog_manager_;
   scoped_ptr<brightray::InspectableWebContents> inspectable_web_contents_;
 

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();
+  });
 });