Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from Dec 4, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions browser/api/atom_api_window.cc
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
}

Expand Down
41 changes: 23 additions & 18 deletions browser/native_window.cc
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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(
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -340,16 +352,16 @@ bool NativeWindow::IsPopupOrPanel(const content::WebContents* source) const {
}

void NativeWindow::RendererUnresponsive(content::WebContents* source) {
not_responding_ = true;
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::Bind(&NativeWindow::RendererUnresponsiveDelayed,
weak_factory_.GetWeakPtr()),
base::TimeDelta::FromMilliseconds(500));
window_unresposive_closure_.Cancel();

if (!HasModalDialog())
FOR_EACH_OBSERVER(NativeWindowObserver,
observers_,
OnRendererUnresponsive());
}

void NativeWindow::RendererResponsive(content::WebContents* source) {
not_responding_ = false;
window_unresposive_closure_.Cancel();
FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererResponsive());
}

Expand All @@ -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) {
Expand Down
13 changes: 9 additions & 4 deletions browser/native_window.h
Expand Up @@ -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"
Expand Down Expand Up @@ -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_; }
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -214,8 +218,9 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
// The windows has been closed.
bool is_closed_;

// The window is not responding.
bool not_responding_;
// 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::CancelableClosure window_unresposive_closure_;

base::WeakPtrFactory<NativeWindow> weak_factory_;

Expand Down
1 change: 1 addition & 0 deletions browser/native_window_mac.h
Expand Up @@ -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(); }
Expand Down
4 changes: 4 additions & 0 deletions browser/native_window_mac.mm
Expand Up @@ -387,6 +387,10 @@ - (void)mouseDragged:(NSEvent*)event {
return is_kiosk_;
}

bool NativeWindowMac::HasModalDialog() {
return [window() attachedSheet] != nil;
}

gfx::NativeWindow NativeWindowMac::GetNativeWindow() {
return window();
}
Expand Down
16 changes: 16 additions & 0 deletions browser/native_window_win.cc
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
}
Expand Down
1 change: 1 addition & 0 deletions browser/native_window_win.h
Expand Up @@ -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);
Expand Down