Merge pull request #133 from atom/sheet-unresponsive

Do not send "unresponsive" message if window is showing a modal dialog.

browser/api/atom_api_window.cc

@@ -4,6 +4,7 @@
 
 #include "browser/api/atom_api_window.h"
 
+#include "base/process_util.h"
 #include "base/values.h"
 #include "browser/native_window.h"
 #include "common/v8_conversions.h"
@@ -126,8 +127,15 @@ v8::Handle<v8::Value> Window::New(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::Destroy(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
+  base::ProcessHandle handle = self->window_->GetRenderProcessHandle();
   delete self;
 
+  // Check if the render process is terminated, it could happen that the render
+  // became a zombie.
+  if (!base::WaitForSingleProcess(handle,
+                                  base::TimeDelta::FromMilliseconds(500)))
+    base::KillProcess(handle, 0, true);
+
   return v8::Undefined();
 }
 

browser/native_window.cc

@@ -44,7 +44,6 @@ NativeWindow::NativeWindow(content::WebContents* web_contents,
     : content::WebContentsObserver(web_contents),
       has_frame_(true),
       is_closed_(false),
-      not_responding_(false),
       weak_factory_(this),
       inspectable_web_contents_(
           brightray::InspectableWebContents::Create(web_contents)) {
@@ -200,6 +199,10 @@ bool NativeWindow::SetIcon(const std::string& str_path) {
   return true;
 }
 
+base::ProcessHandle NativeWindow::GetRenderProcessHandle() {
+  return GetWebContents()->GetRenderProcessHost()->GetHandle();
+}
+
 void NativeWindow::CapturePage(const gfx::Rect& rect,
                                const CapturePageCallback& callback) {
   GetWebContents()->GetRenderViewHost()->CopyFromBackingStore(
@@ -226,7 +229,16 @@ void NativeWindow::CloseWebContents() {
   // 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 (window_unresposive_closure_.IsCancelled()) {
+    window_unresposive_closure_.Reset(
+        base::Bind(&NativeWindow::RendererUnresponsive,
+                   weak_factory_.GetWeakPtr(),
+                   web_contents));
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        window_unresposive_closure_.callback(),
+        base::TimeDelta::FromMilliseconds(500));
+  }
 
   if (web_contents->NeedToFireBeforeUnload())
     web_contents->GetRenderViewHost()->FirePageBeforeUnload(false);
@@ -286,7 +298,7 @@ void NativeWindow::BeforeUnloadFired(content::WebContents* tab,
     WindowList::WindowCloseCancelled(this);
 
   // When the "beforeunload" callback is fired the window is certainly live.
-  not_responding_ = false;
+  window_unresposive_closure_.Cancel();
 }
 
 void NativeWindow::RequestToLockMouse(content::WebContents* web_contents,
@@ -331,7 +343,7 @@ void NativeWindow::CloseContents(content::WebContents* source) {
   NotifyWindowClosed();
 
   // Do not sent "unresponsive" event after window is closed.
-  not_responding_ = false;
+  window_unresposive_closure_.Cancel();
 }
 
 bool NativeWindow::IsPopupOrPanel(const content::WebContents* source) const {
@@ -340,16 +352,16 @@ bool NativeWindow::IsPopupOrPanel(const content::WebContents* source) const {
 }
 
 void NativeWindow::RendererUnresponsive(content::WebContents* source) {
-  not_responding_ = true;
+  window_unresposive_closure_.Cancel();
-  base::MessageLoop::current()->PostDelayedTask(
+
-      FROM_HERE,
+  if (!HasModalDialog())
-      base::Bind(&NativeWindow::RendererUnresponsiveDelayed,
+    FOR_EACH_OBSERVER(NativeWindowObserver,
-                 weak_factory_.GetWeakPtr()),
+                      observers_,
-      base::TimeDelta::FromMilliseconds(500));
+                      OnRendererUnresponsive());
 }
 
 void NativeWindow::RendererResponsive(content::WebContents* source) {
-  not_responding_ = false;
+  window_unresposive_closure_.Cancel();
   FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererResponsive());
 }
 
@@ -371,13 +383,6 @@ void NativeWindow::RenderViewGone(base::TerminationStatus status) {
   FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererCrashed());
 }
 
-void NativeWindow::RendererUnresponsiveDelayed() {
-  if (not_responding_)
-    FOR_EACH_OBSERVER(NativeWindowObserver,
-                      observers_,
-                      OnRendererUnresponsive());
-}
-
 void NativeWindow::Observe(int type,
                            const content::NotificationSource& source,
                            const content::NotificationDetails& details) {

browser/native_window.h

@@ -6,6 +6,7 @@
 #define ATOM_BROWSER_NATIVE_WINDOW_H_
 
 #include "base/basictypes.h"
+#include "base/cancelable_callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -99,6 +100,7 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
   virtual void FlashFrame(bool flash) = 0;
   virtual void SetKiosk(bool kiosk) = 0;
   virtual bool IsKiosk() = 0;
+  virtual bool HasModalDialog() = 0;
   virtual gfx::NativeWindow GetNativeWindow() = 0;
 
   virtual bool IsClosed() const { return is_closed_; }
@@ -112,6 +114,10 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
   virtual void RestartHangMonitorTimeout();
   virtual bool SetIcon(const std::string& path);
 
+  // Returns the process handle of render process, useful for killing the
+  // render process manually
+  virtual base::ProcessHandle GetRenderProcessHandle();
+
   // Captures the page with |rect|, |callback| would be called when capturing is
   // done.
   virtual void CapturePage(const gfx::Rect& rect,
@@ -191,8 +197,6 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
   gfx::Image icon_;
 
  private:
-  void RendererUnresponsiveDelayed();
-
   // Called when CapturePage has done.
   void OnCapturePageDone(const CapturePageCallback& callback,
                          bool succeed,
@@ -214,8 +218,9 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
   // The windows has been closed.
   bool is_closed_;
 
-  // The window is not responding.
+  // Closure that would be called when window is unresponsive when closing,
-  bool not_responding_;
+  // it should be cancelled when we can prove that the window is responsive.
+  base::CancelableClosure window_unresposive_closure_;
 
   base::WeakPtrFactory<NativeWindow> weak_factory_;
 

browser/native_window_mac.h

@@ -53,6 +53,7 @@ class NativeWindowMac : public NativeWindow {
   virtual void FlashFrame(bool flash) OVERRIDE;
   virtual void SetKiosk(bool kiosk) OVERRIDE;
   virtual bool IsKiosk() OVERRIDE;
+  virtual bool HasModalDialog() OVERRIDE;
   virtual gfx::NativeWindow GetNativeWindow() OVERRIDE;
 
   void NotifyWindowBlur() { NativeWindow::NotifyWindowBlur(); }

browser/native_window_mac.mm

@@ -387,6 +387,10 @@ bool NativeWindowMac::IsKiosk() {
   return is_kiosk_;
 }
 
+bool NativeWindowMac::HasModalDialog() {
+  return [window() attachedSheet] != nil;
+}
+
 gfx::NativeWindow NativeWindowMac::GetNativeWindow() {
   return window();
 }

browser/native_window_win.cc

@@ -194,6 +194,17 @@ class NativeWindowFramelessView : public views::NonClientFrameView {
   DISALLOW_COPY_AND_ASSIGN(NativeWindowFramelessView);
 };
 
+bool WindowHasModalDialog(HWND parent, HWND except, HWND after = NULL) {
+  HWND hwnd = ::FindWindowEx(parent, after, NULL, NULL);
+  if (hwnd != except &&
+      (::GetWindowLong(hwnd, GWL_STYLE) & (WS_VISIBLE | WS_POPUP)))
+    return true;
+  else if (hwnd == NULL)
+    return false;
+  else
+    return WindowHasModalDialog(parent, except, hwnd);
+}
+
 }  // namespace
 
 NativeWindowWin::NativeWindowWin(content::WebContents* web_contents,
@@ -359,6 +370,11 @@ bool NativeWindowWin::IsKiosk() {
   return IsFullscreen();
 }
 
+bool NativeWindowWin::HasModalDialog() {
+  return WindowHasModalDialog(GetNativeWindow(),
+                              GetWebContents()->GetView()->GetNativeView());
+}
+
 gfx::NativeWindow NativeWindowWin::GetNativeWindow() {
   return window_->GetNativeView();
 }

browser/native_window_win.h

@@ -65,6 +65,7 @@ class NativeWindowWin : public NativeWindow,
   virtual void FlashFrame(bool flash) OVERRIDE;
   virtual void SetKiosk(bool kiosk) OVERRIDE;
   virtual bool IsKiosk() OVERRIDE;
+  virtual bool HasModalDialog() OVERRIDE;
   virtual gfx::NativeWindow GetNativeWindow() OVERRIDE;
 
   void OnMenuCommand(int position, HMENU menu);