Do not use flags to mark whether the window is unresponsive.

It's error prone.
This commit is contained in:
Cheng Zhao 2013-12-04 17:10:59 +08:00
parent d6d1604623
commit bfe8928ab1
2 changed files with 22 additions and 22 deletions

View file

@ -44,7 +44,6 @@ NativeWindow::NativeWindow(content::WebContents* web_contents,
: content::WebContentsObserver(web_contents), : content::WebContentsObserver(web_contents),
has_frame_(true), has_frame_(true),
is_closed_(false), is_closed_(false),
not_responding_(false),
weak_factory_(this), weak_factory_(this),
inspectable_web_contents_( inspectable_web_contents_(
brightray::InspectableWebContents::Create(web_contents)) { brightray::InspectableWebContents::Create(web_contents)) {
@ -226,7 +225,16 @@ void NativeWindow::CloseWebContents() {
// not closed in 500ms, in this way we can quickly show the unresponsive // 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 // dialog when the window is busy executing some script withouth waiting for
// the unresponsive timeout. // 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()) if (web_contents->NeedToFireBeforeUnload())
web_contents->GetRenderViewHost()->FirePageBeforeUnload(false); web_contents->GetRenderViewHost()->FirePageBeforeUnload(false);
@ -286,7 +294,7 @@ void NativeWindow::BeforeUnloadFired(content::WebContents* tab,
WindowList::WindowCloseCancelled(this); WindowList::WindowCloseCancelled(this);
// When the "beforeunload" callback is fired the window is certainly live. // 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, void NativeWindow::RequestToLockMouse(content::WebContents* web_contents,
@ -331,7 +339,7 @@ void NativeWindow::CloseContents(content::WebContents* source) {
NotifyWindowClosed(); NotifyWindowClosed();
// Do not sent "unresponsive" event after window is closed. // Do not sent "unresponsive" event after window is closed.
not_responding_ = false; window_unresposive_closure_.Cancel();
} }
bool NativeWindow::IsPopupOrPanel(const content::WebContents* source) const { bool NativeWindow::IsPopupOrPanel(const content::WebContents* source) const {
@ -340,16 +348,15 @@ bool NativeWindow::IsPopupOrPanel(const content::WebContents* source) const {
} }
void NativeWindow::RendererUnresponsive(content::WebContents* source) { void NativeWindow::RendererUnresponsive(content::WebContents* source) {
not_responding_ = true; LOG(ERROR) << "NativeWindow::RendererUnresponsive";
base::MessageLoop::current()->PostDelayedTask( window_unresposive_closure_.Cancel();
FROM_HERE, FOR_EACH_OBSERVER(NativeWindowObserver,
base::Bind(&NativeWindow::RendererUnresponsiveDelayed, observers_,
weak_factory_.GetWeakPtr()), OnRendererUnresponsive());
base::TimeDelta::FromMilliseconds(500));
} }
void NativeWindow::RendererResponsive(content::WebContents* source) { void NativeWindow::RendererResponsive(content::WebContents* source) {
not_responding_ = false; window_unresposive_closure_.Cancel();
FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererResponsive()); FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererResponsive());
} }
@ -371,13 +378,6 @@ void NativeWindow::RenderViewGone(base::TerminationStatus status) {
FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererCrashed()); FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererCrashed());
} }
void NativeWindow::RendererUnresponsiveDelayed() {
if (not_responding_)
FOR_EACH_OBSERVER(NativeWindowObserver,
observers_,
OnRendererUnresponsive());
}
void NativeWindow::Observe(int type, void NativeWindow::Observe(int type,
const content::NotificationSource& source, const content::NotificationSource& source,
const content::NotificationDetails& details) { const content::NotificationDetails& details) {

View file

@ -6,6 +6,7 @@
#define ATOM_BROWSER_NATIVE_WINDOW_H_ #define ATOM_BROWSER_NATIVE_WINDOW_H_
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/cancelable_callback.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
@ -192,8 +193,6 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
gfx::Image icon_; gfx::Image icon_;
private: private:
void RendererUnresponsiveDelayed();
// Called when CapturePage has done. // Called when CapturePage has done.
void OnCapturePageDone(const CapturePageCallback& callback, void OnCapturePageDone(const CapturePageCallback& callback,
bool succeed, bool succeed,
@ -215,8 +214,9 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
// The windows has been closed. // The windows has been closed.
bool is_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_; base::WeakPtrFactory<NativeWindow> weak_factory_;