From 7848608198b49ee1d54faaa79b6d4309c6408129 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 11 Jul 2016 15:29:03 +0900 Subject: [PATCH 1/2] Replace DialogScope with UnresponsiveSuppressor The latter is global-wide. --- atom/browser/native_window.cc | 8 ++----- atom/browser/native_window.h | 29 +------------------------ atom/browser/native_window_mac.h | 1 - atom/browser/native_window_mac.mm | 4 ---- atom/browser/ui/file_dialog_gtk.cc | 4 ++-- atom/browser/ui/file_dialog_win.cc | 3 ++- atom/browser/ui/message_box_gtk.cc | 6 ++--- atom/browser/ui/message_box_win.cc | 4 +++- atom/browser/unresponsive_suppressor.cc | 27 +++++++++++++++++++++++ atom/browser/unresponsive_suppressor.h | 25 +++++++++++++++++++++ filenames.gypi | 2 ++ 11 files changed, 67 insertions(+), 46 deletions(-) create mode 100644 atom/browser/unresponsive_suppressor.cc create mode 100644 atom/browser/unresponsive_suppressor.h diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 0e536c113cc..c084f448a38 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -11,6 +11,7 @@ #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" #include "atom/browser/browser.h" +#include "atom/browser/unresponsive_suppressor.h" #include "atom/browser/window_list.h" #include "atom/common/api/api_messages.h" #include "atom/common/native_mate_converters/file_path_converter.h" @@ -52,7 +53,6 @@ NativeWindow::NativeWindow( transparent_(false), enable_larger_than_screen_(false), is_closed_(false), - has_dialog_attached_(false), sheet_offset_x_(0.0), sheet_offset_y_(0.0), aspect_ratio_(0.0), @@ -293,10 +293,6 @@ void NativeWindow::SetFocusable(bool focusable) { void NativeWindow::SetMenu(AtomMenuModel* menu) { } -bool NativeWindow::HasModalDialog() { - return has_dialog_attached_; -} - void NativeWindow::SetParentWindow(NativeWindow* parent) { parent_ = parent; } @@ -590,7 +586,7 @@ void NativeWindow::ScheduleUnresponsiveEvent(int ms) { void NativeWindow::NotifyWindowUnresponsive() { window_unresposive_closure_.Cancel(); - if (!is_closed_ && !HasModalDialog() && IsEnabled()) + if (!is_closed_ && !IsUnresponsiveEventSuppressed() && IsEnabled()) FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererUnresponsive()); diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 1828c332b22..fb4f0217b91 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -51,26 +51,7 @@ struct DraggableRegion; class NativeWindow : public base::SupportsUserData, public content::WebContentsObserver { public: - class DialogScope { - public: - explicit DialogScope(NativeWindow* window) - : window_(window) { - if (window_ != NULL) - window_->set_has_dialog_attached(true); - } - - ~DialogScope() { - if (window_ != NULL) - window_->set_has_dialog_attached(false); - } - - private: - NativeWindow* window_; - - DISALLOW_COPY_AND_ASSIGN(DialogScope); - }; - - virtual ~NativeWindow(); + ~NativeWindow() override; // Create window with existing WebContents, the caller is responsible for // managing the window's live. @@ -155,7 +136,6 @@ class NativeWindow : public base::SupportsUserData, virtual void SetContentProtection(bool enable) = 0; virtual void SetFocusable(bool focusable); virtual void SetMenu(AtomMenuModel* menu); - virtual bool HasModalDialog(); virtual void SetParentWindow(NativeWindow* parent); virtual gfx::NativeWindow GetNativeWindow() = 0; virtual gfx::AcceleratedWidget GetAcceleratedWidget() = 0; @@ -245,10 +225,6 @@ class NativeWindow : public base::SupportsUserData, SkRegion* draggable_region() const { return draggable_region_.get(); } bool enable_larger_than_screen() const { return enable_larger_than_screen_; } - void set_has_dialog_attached(bool has_dialog_attached) { - has_dialog_attached_ = has_dialog_attached; - } - NativeWindow* parent() const { return parent_; } bool is_modal() const { return is_modal_; } @@ -305,9 +281,6 @@ class NativeWindow : public base::SupportsUserData, // The windows has been closed. bool is_closed_; - // There is a dialog that has been attached to window. - bool has_dialog_attached_; - // 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_; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index d07d586a843..d09faa18f6c 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -80,7 +80,6 @@ class NativeWindowMac : public NativeWindow { bool IsDocumentEdited() override; void SetIgnoreMouseEvents(bool ignore) override; void SetContentProtection(bool enable) override; - bool HasModalDialog() override; void SetParentWindow(NativeWindow* parent) override; gfx::NativeWindow GetNativeWindow() override; gfx::AcceleratedWidget GetAcceleratedWidget() override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 3a8284e1bee..6f93b084968 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -960,10 +960,6 @@ void NativeWindowMac::SetContentProtection(bool enable) { : NSWindowSharingReadOnly]; } -bool NativeWindowMac::HasModalDialog() { - return [window_ attachedSheet] != nil; -} - void NativeWindowMac::SetParentWindow(NativeWindow* parent) { if (is_modal()) return; diff --git a/atom/browser/ui/file_dialog_gtk.cc b/atom/browser/ui/file_dialog_gtk.cc index 50c38fe0c38..b6233d2054c 100644 --- a/atom/browser/ui/file_dialog_gtk.cc +++ b/atom/browser/ui/file_dialog_gtk.cc @@ -5,6 +5,7 @@ #include "atom/browser/ui/file_dialog.h" #include "atom/browser/native_window_views.h" +#include "atom/browser/unresponsive_suppressor.h" #include "base/callback.h" #include "base/files/file_util.h" #include "base/strings/string_util.h" @@ -41,7 +42,6 @@ class FileChooserDialog { const base::FilePath& default_path, const Filters& filters) : parent_(static_cast(parent_window)), - dialog_scope_(parent_window), filters_(filters) { const char* confirm_text = GTK_STOCK_OK; @@ -153,7 +153,7 @@ class FileChooserDialog { base::FilePath AddExtensionForFilename(const gchar* filename) const; atom::NativeWindowViews* parent_; - atom::NativeWindow::DialogScope dialog_scope_; + atom::UnresponsiveSuppressor unresponsive_suppressor_; GtkWidget* dialog_; diff --git a/atom/browser/ui/file_dialog_win.cc b/atom/browser/ui/file_dialog_win.cc index 88c847efc8c..ff7d8f97235 100644 --- a/atom/browser/ui/file_dialog_win.cc +++ b/atom/browser/ui/file_dialog_win.cc @@ -10,6 +10,7 @@ #include #include "atom/browser/native_window_views.h" +#include "atom/browser/unresponsive_suppressor.h" #include "base/files/file_util.h" #include "base/i18n/case_conversion.h" #include "base/strings/string_util.h" @@ -108,7 +109,7 @@ class FileDialog { } bool Show(atom::NativeWindow* parent_window) { - atom::NativeWindow::DialogScope dialog_scope(parent_window); + atom::UnresponsiveSuppressor suppressor; HWND window = parent_window ? static_cast( parent_window)->GetAcceleratedWidget() : NULL; diff --git a/atom/browser/ui/message_box_gtk.cc b/atom/browser/ui/message_box_gtk.cc index d08171c6e42..8f11c94b7d3 100644 --- a/atom/browser/ui/message_box_gtk.cc +++ b/atom/browser/ui/message_box_gtk.cc @@ -6,6 +6,7 @@ #include "atom/browser/browser.h" #include "atom/browser/native_window_views.h" +#include "atom/browser/unresponsive_suppressor.h" #include "base/callback.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -35,8 +36,7 @@ class GtkMessageBox { const std::string& message, const std::string& detail, const gfx::ImageSkia& icon) - : dialog_scope_(parent_window), - cancel_id_(cancel_id), + : cancel_id_(cancel_id), parent_(static_cast(parent_window)) { // Create dialog. dialog_ = gtk_message_dialog_new( @@ -147,7 +147,7 @@ class GtkMessageBox { CHROMEGTK_CALLBACK_1(GtkMessageBox, void, OnResponseDialog, int); private: - atom::NativeWindow::DialogScope dialog_scope_; + atom::UnresponsiveSuppressor unresponsive_suppressor_; // The id to return when the dialog is closed without pressing buttons. int cancel_id_; diff --git a/atom/browser/ui/message_box_win.cc b/atom/browser/ui/message_box_win.cc index b966ef92a8f..e5615fb1fc1 100644 --- a/atom/browser/ui/message_box_win.cc +++ b/atom/browser/ui/message_box_win.cc @@ -12,6 +12,7 @@ #include "atom/browser/browser.h" #include "atom/browser/native_window_views.h" +#include "atom/browser/unresponsive_suppressor.h" #include "base/callback.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -198,7 +199,7 @@ int ShowMessageBox(NativeWindow* parent, static_cast(parent)->GetAcceleratedWidget() : NULL; - NativeWindow::DialogScope dialog_scope(parent); + atom::UnresponsiveSuppressor suppressor; return ShowMessageBoxUTF16(hwnd_parent, type, utf16_buttons, @@ -239,6 +240,7 @@ void ShowMessageBox(NativeWindow* parent, } void ShowErrorBox(const base::string16& title, const base::string16& content) { + atom::UnresponsiveSuppressor suppressor; ShowMessageBoxUTF16(NULL, MESSAGE_BOX_TYPE_ERROR, {}, -1, 0, 0, L"Error", title, content, gfx::ImageSkia()); } diff --git a/atom/browser/unresponsive_suppressor.cc b/atom/browser/unresponsive_suppressor.cc new file mode 100644 index 00000000000..89bfa6b9003 --- /dev/null +++ b/atom/browser/unresponsive_suppressor.cc @@ -0,0 +1,27 @@ +// Copyright (c) 2016 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/unresponsive_suppressor.h" + +namespace atom { + +namespace { + +int g_suppress_level = 0; + +} // namespace + +bool IsUnresponsiveEventSuppressed() { + return g_suppress_level > 0; +} + +UnresponsiveSuppressor::UnresponsiveSuppressor() { + g_suppress_level++; +} + +UnresponsiveSuppressor::~UnresponsiveSuppressor() { + g_suppress_level--; +} + +} // namespace atom diff --git a/atom/browser/unresponsive_suppressor.h b/atom/browser/unresponsive_suppressor.h new file mode 100644 index 00000000000..9fb3819237d --- /dev/null +++ b/atom/browser/unresponsive_suppressor.h @@ -0,0 +1,25 @@ +// Copyright (c) 2016 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_UNRESPONSIVE_SUPPRESSOR_H_ +#define ATOM_BROWSER_UNRESPONSIVE_SUPPRESSOR_H_ + +#include "base/macros.h" + +namespace atom { + +bool IsUnresponsiveEventSuppressed(); + +class UnresponsiveSuppressor { + public: + UnresponsiveSuppressor(); + ~UnresponsiveSuppressor(); + + private: + DISALLOW_COPY_AND_ASSIGN(UnresponsiveSuppressor); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_UNRESPONSIVE_SUPPRESSOR_H_ diff --git a/filenames.gypi b/filenames.gypi index c1327122031..c809a2e05a8 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -299,6 +299,8 @@ 'atom/browser/ui/x/window_state_watcher.h', 'atom/browser/ui/x/x_window_utils.cc', 'atom/browser/ui/x/x_window_utils.h', + 'atom/browser/unresponsive_suppressor.cc', + 'atom/browser/unresponsive_suppressor.h', 'atom/browser/web_contents_permission_helper.cc', 'atom/browser/web_contents_permission_helper.h', 'atom/browser/web_contents_preferences.cc', From 8269e7b1efb91c1c1be539fbc859a56eeb7d1906 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 11 Jul 2016 15:31:24 +0900 Subject: [PATCH 2/2] Suppress unresponsive event when showing menu --- atom/browser/api/atom_api_menu_mac.mm | 4 ++++ atom/browser/api/atom_api_menu_views.cc | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/atom/browser/api/atom_api_menu_mac.mm b/atom/browser/api/atom_api_menu_mac.mm index 22a624988b7..6a7d43b4da2 100644 --- a/atom/browser/api/atom_api_menu_mac.mm +++ b/atom/browser/api/atom_api_menu_mac.mm @@ -5,6 +5,7 @@ #import "atom/browser/api/atom_api_menu_mac.h" #include "atom/browser/native_window.h" +#include "atom/browser/unresponsive_suppressor.h" #include "base/message_loop/message_loop.h" #include "base/strings/sys_string_conversions.h" #include "brightray/browser/inspectable_web_contents.h" @@ -67,6 +68,9 @@ void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { if (rightmostMenuPoint > screenRight) position.x = position.x - [menu size].width; + // Don't emit unresponsive event when showing menu. + atom::UnresponsiveSuppressor suppressor; + // Show the menu. [menu popUpMenuPositioningItem:item atLocation:position inView:view]; } diff --git a/atom/browser/api/atom_api_menu_views.cc b/atom/browser/api/atom_api_menu_views.cc index 9340bc76732..ceee46360d7 100644 --- a/atom/browser/api/atom_api_menu_views.cc +++ b/atom/browser/api/atom_api_menu_views.cc @@ -5,6 +5,7 @@ #include "atom/browser/api/atom_api_menu_views.h" #include "atom/browser/native_window_views.h" +#include "atom/browser/unresponsive_suppressor.h" #include "content/public/browser/render_widget_host_view.h" #include "ui/gfx/screen.h" #include "ui/views/controls/menu/menu_runner.h" @@ -36,6 +37,9 @@ void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { location = gfx::Point(origin.x() + x, origin.y() + y); } + // Don't emit unresponsive event when showing menu. + atom::UnresponsiveSuppressor suppressor; + // Show the menu. views::MenuRunner menu_runner( model(),