From 75a624434cc30d445751bf45f95ef9138d0ea1a5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Apr 2018 17:23:08 +0900 Subject: [PATCH 1/4] refactor: use views::Widget on macOS --- atom/browser/native_window_mac.h | 15 +++++-- atom/browser/native_window_mac.mm | 42 +++++++++++++------ .../browser/ui/cocoa/atom_native_widget_mac.h | 5 ++- .../ui/cocoa/atom_native_widget_mac.mm | 13 +++++- atom/browser/ui/cocoa/atom_ns_window.h | 2 +- .../ui/cocoa/atom_ns_window_delegate.h | 6 +-- .../ui/cocoa/atom_ns_window_delegate.mm | 12 +++++- atom/browser/ui/cocoa/views_delegate_mac.mm | 3 +- 8 files changed, 72 insertions(+), 26 deletions(-) diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 42f24ccf169f..89bdec886d2a 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -23,7 +23,8 @@ namespace atom { -class NativeWindowMac : public NativeWindow { +class NativeWindowMac : public NativeWindow, + public views::WidgetDelegate { public: NativeWindowMac(const mate::Dictionary& options, NativeWindow* parent); ~NativeWindowMac() override; @@ -144,19 +145,25 @@ class NativeWindowMac : public NativeWindow { bool fullscreen_window_title() const { return fullscreen_window_title_; } bool simple_fullscreen() const { return always_simple_fullscreen_; } + protected: + // views::WidgetDelegate: + void DeleteDelegate() override; + views::Widget* GetWidget() override; + const views::Widget* GetWidget() const override; + private: void InternalSetParentWindow(NativeWindow* parent, bool attach); void ShowWindowButton(NSWindowButton button); void SetForwardMouseMessages(bool forward); - base::scoped_nsobject window_; + std::unique_ptr widget_; + AtomNSWindow* window_; // Weak ref, managed by widget_. + base::scoped_nsobject window_delegate_; base::scoped_nsobject preview_item_; base::scoped_nsobject touch_bar_; - std::unique_ptr widget_; - // Event monitor for scroll wheel event. id wheel_event_monitor_; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 35677e8c36fe..46b6da002fc1 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -9,6 +9,7 @@ #include #include "atom/browser/native_browser_view_mac.h" +#include "atom/browser/ui/cocoa/atom_native_widget_mac.h" #include "atom/browser/ui/cocoa/atom_ns_window.h" #include "atom/browser/ui/cocoa/atom_ns_window_delegate.h" #include "atom/browser/ui/cocoa/atom_preview_item.h" @@ -247,9 +248,10 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, options.Get(options::kHeight, &height); NSRect main_screen_rect = [[[NSScreen screens] firstObject] frame]; - NSRect cocoa_bounds = NSMakeRect( - round((NSWidth(main_screen_rect) - width) / 2), - round((NSHeight(main_screen_rect) - height) / 2), width, height); + gfx::Rect bounds(round((NSWidth(main_screen_rect) - width) / 2), + round((NSHeight(main_screen_rect) - height) / 2), + width, + height); bool resizable = true; options.Get(options::kResizable, &resizable); @@ -303,10 +305,18 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, styleMask |= NSResizableWindowMask; } - window_.reset([[AtomNSWindow alloc] initWithContentRect:cocoa_bounds - styleMask:styleMask - backing:NSBackingStoreBuffered - defer:YES]); + // Create views::Widget and assign window_ with it. + // TODO(zcbenz): Get rid of the window_ in future. + widget_.reset(new views::Widget()); + views::Widget::InitParams params; + params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; + params.bounds = bounds; + params.delegate = this; + params.type = views::Widget::InitParams::TYPE_WINDOW; + params.native_widget = new AtomNativeWidgetMac(styleMask, widget_.get()); + widget_->Init(params); + window_ = static_cast(widget_->GetNativeWindow()); + [window_ setShell:this]; [window_ setEnableLargerThanScreen:enable_larger_than_screen()]; @@ -357,9 +367,6 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, } } - // We will manage window's lifetime ourselves. - [window_ setReleasedWhenClosed:NO]; - // Hide the title bar background if (title_bar_style_ != NORMAL) { if (@available(macOS 10.10, *)) { @@ -384,7 +391,7 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, bool use_content_size = false; options.Get(options::kUseContentSize, &use_content_size); if (!has_frame() || !use_content_size) - SetSize(gfx::Size(width, height)); + NativeWindow::SetSize(gfx::Size(width, height)); options.Get(options::kZoomToPageWidth, &zoom_to_page_width_); @@ -1145,7 +1152,7 @@ void NativeWindowMac::ToggleTabBar() { } bool NativeWindowMac::AddTabbedWindow(NativeWindow* window) { - if (window_.get() == window->GetNativeWindow()) { + if (window_ == window->GetNativeWindow()) { return false; } else { if (@available(macOS 10.12, *)) @@ -1289,6 +1296,17 @@ gfx::Rect NativeWindowMac::WindowBoundsToContentBounds( } } +void NativeWindowMac::DeleteDelegate() { +} + +views::Widget* NativeWindowMac::GetWidget() { + return widget_.get(); +} + +const views::Widget* NativeWindowMac::GetWidget() const { + return widget_.get(); +} + void NativeWindowMac::InternalSetParentWindow(NativeWindow* parent, bool attach) { if (is_modal()) diff --git a/atom/browser/ui/cocoa/atom_native_widget_mac.h b/atom/browser/ui/cocoa/atom_native_widget_mac.h index 49250239f3b1..e5f711e80b65 100644 --- a/atom/browser/ui/cocoa/atom_native_widget_mac.h +++ b/atom/browser/ui/cocoa/atom_native_widget_mac.h @@ -11,7 +11,8 @@ namespace atom { class AtomNativeWidgetMac : public views::NativeWidgetMac { public: - explicit AtomNativeWidgetMac(views::internal::NativeWidgetDelegate* delegate); + AtomNativeWidgetMac(NSUInteger style_mask, + views::internal::NativeWidgetDelegate* delegate); ~AtomNativeWidgetMac() override; protected: @@ -20,6 +21,8 @@ class AtomNativeWidgetMac : public views::NativeWidgetMac { const views::Widget::InitParams& params) override; private: + NSUInteger style_mask_; + DISALLOW_COPY_AND_ASSIGN(AtomNativeWidgetMac); }; diff --git a/atom/browser/ui/cocoa/atom_native_widget_mac.mm b/atom/browser/ui/cocoa/atom_native_widget_mac.mm index eb7117790a69..9cb5da48bc11 100644 --- a/atom/browser/ui/cocoa/atom_native_widget_mac.mm +++ b/atom/browser/ui/cocoa/atom_native_widget_mac.mm @@ -4,17 +4,26 @@ #include "atom/browser/ui/cocoa/atom_native_widget_mac.h" +#include "atom/browser/ui/cocoa/atom_ns_window.h" +#include "ui/base/cocoa/window_size_constants.h" + namespace atom { AtomNativeWidgetMac::AtomNativeWidgetMac( + NSUInteger style_mask, views::internal::NativeWidgetDelegate* delegate) - : views::NativeWidgetMac(delegate) {} + : views::NativeWidgetMac(delegate), + style_mask_(style_mask) {} AtomNativeWidgetMac::~AtomNativeWidgetMac() {} NativeWidgetMacNSWindow* AtomNativeWidgetMac::CreateNSWindow( const views::Widget::InitParams& params) { - return views::NativeWidgetMac::CreateNSWindow(params); + return [[[AtomNSWindow alloc] + initWithContentRect:ui::kWindowSizeDeterminedLater + styleMask:style_mask_ + backing:NSBackingStoreBuffered + defer:YES] autorelease]; } } // namespace atom diff --git a/atom/browser/ui/cocoa/atom_ns_window.h b/atom/browser/ui/cocoa/atom_ns_window.h index 62d690b57e31..885e19c6dcc4 100644 --- a/atom/browser/ui/cocoa/atom_ns_window.h +++ b/atom/browser/ui/cocoa/atom_ns_window.h @@ -26,7 +26,7 @@ class ScopedDisableResize { } // namespace atom -@interface AtomNSWindow : EventDispatchingWindow { +@interface AtomNSWindow : NativeWidgetMacNSWindow { @private atom::NativeWindowMac* shell_; CGFloat windowButtonsInterButtonSpacing_; diff --git a/atom/browser/ui/cocoa/atom_ns_window_delegate.h b/atom/browser/ui/cocoa/atom_ns_window_delegate.h index 0947b0392147..e86754bcf32c 100644 --- a/atom/browser/ui/cocoa/atom_ns_window_delegate.h +++ b/atom/browser/ui/cocoa/atom_ns_window_delegate.h @@ -13,9 +13,9 @@ namespace atom { class NativeWindowMac; } -@interface AtomNSWindowDelegate : NSObject { +@interface AtomNSWindowDelegate : + ViewsNSWindowDelegate { @private atom::NativeWindowMac* shell_; bool is_zooming_; diff --git a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm index 92ab2293050f..b20a350cc7f2 100644 --- a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm +++ b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm @@ -9,11 +9,21 @@ #include "atom/browser/ui/cocoa/atom_preview_item.h" #include "atom/browser/ui/cocoa/atom_touch_bar.h" #include "base/mac/mac_util.h" +#include "ui/views/widget/native_widget_mac.h" @implementation AtomNSWindowDelegate - (id)initWithShell:(atom::NativeWindowMac*)shell { - if ((self = [super init])) { + // The views library assumes the window delegate must be an instance of + // ViewsNSWindowDelegate, since we don't have a way to override the creation + // of NSWindowDelegate, we have to dynamically replace the window delegate + // on the fly. + // TODO(zcbenz): Add interface in NativeWidgetMac to allow overriding creating + // window delegate. + views::BridgedNativeWidget* bridget_view = + views::NativeWidgetMac::GetBridgeForNativeWindow( + shell->GetNativeWindow()); + if ((self = [super initWithBridgedNativeWidget:bridget_view])) { shell_ = shell; is_zooming_ = false; level_ = [shell_->GetNativeWindow() level]; diff --git a/atom/browser/ui/cocoa/views_delegate_mac.mm b/atom/browser/ui/cocoa/views_delegate_mac.mm index 5e5ed6732267..25e72980367b 100644 --- a/atom/browser/ui/cocoa/views_delegate_mac.mm +++ b/atom/browser/ui/cocoa/views_delegate_mac.mm @@ -16,8 +16,7 @@ ViewsDelegateMac::~ViewsDelegateMac() {} void ViewsDelegateMac::OnBeforeWidgetInit( views::Widget::InitParams* params, views::internal::NativeWidgetDelegate* delegate) { - if (!params->native_widget) - params->native_widget = new views::NativeWidgetMac(delegate); + DCHECK(params->native_widget); } ui::ContextFactory* ViewsDelegateMac::GetContextFactory() { From 39242c978f8bbc8ec3cda9f3c6e75aaa62b775f4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Apr 2018 19:00:44 +0900 Subject: [PATCH 2/4] fix failed BrowserWindow tests --- atom/browser/native_window_mac.h | 5 ++- atom/browser/native_window_mac.mm | 35 +++++++++---------- .../ui/cocoa/atom_ns_window_delegate.mm | 4 +++ 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 89bdec886d2a..3c3de72a8071 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -150,6 +150,7 @@ class NativeWindowMac : public NativeWindow, void DeleteDelegate() override; views::Widget* GetWidget() override; const views::Widget* GetWidget() const override; + bool CanResize() const override; private: void InternalSetParentWindow(NativeWindow* parent, bool attach); @@ -174,12 +175,10 @@ class NativeWindowMac : public NativeWindow, NSView* content_view_; bool is_kiosk_; - bool was_fullscreen_; - bool zoom_to_page_width_; - bool fullscreen_window_title_; + bool resizable_; NSInteger attention_request_id_; // identifier from requestUserAttention diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 46b6da002fc1..bb28bff0676f 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -26,6 +26,7 @@ #include "skia/ext/skia_utils_mac.h" #include "ui/gfx/skia_util.h" #include "ui/gl/gpu_switching_manager.h" +#include "ui/views/background.h" #include "ui/views/widget/widget.h" // Custom Quit, Minimize and Full Screen button container for frameless @@ -239,6 +240,7 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, was_fullscreen_(false), zoom_to_page_width_(false), fullscreen_window_title_(false), + resizable_(true), attention_request_id_(0), title_bar_style_(NORMAL), always_simple_fullscreen_(false), @@ -253,8 +255,11 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, width, height); - bool resizable = true; - options.Get(options::kResizable, &resizable); + options.Get(options::kResizable, &resizable_); + options.Get(options::kTitleBarStyle, &title_bar_style_); + options.Get(options::kZoomToPageWidth, &zoom_to_page_width_); + options.Get(options::kFullscreenWindowTitle, &fullscreen_window_title_); + options.Get(options::kSimpleFullScreen, &always_simple_fullscreen_); bool minimizable = true; options.Get(options::kMinimizable, &minimizable); @@ -265,8 +270,6 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, bool closable = true; options.Get(options::kClosable, &closable); - options.Get(options::kTitleBarStyle, &title_bar_style_); - std::string tabbingIdentifier; options.Get(options::kTabbingIdentifier, &tabbingIdentifier); @@ -301,9 +304,6 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, if (!useStandardWindow || transparent() || !has_frame()) { styleMask |= NSTexturedBackgroundWindowMask; } - if (resizable) { - styleMask |= NSResizableWindowMask; - } // Create views::Widget and assign window_ with it. // TODO(zcbenz): Get rid of the window_ in future. @@ -387,17 +387,11 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, } } - // On macOS the initial window size doesn't include window frame. + // Resize to content bounds. bool use_content_size = false; options.Get(options::kUseContentSize, &use_content_size); - if (!has_frame() || !use_content_size) - NativeWindow::SetSize(gfx::Size(width, height)); - - options.Get(options::kZoomToPageWidth, &zoom_to_page_width_); - - options.Get(options::kFullscreenWindowTitle, &fullscreen_window_title_); - - options.Get(options::kSimpleFullScreen, &always_simple_fullscreen_); + if (!has_frame() || use_content_size) + SetContentSize(gfx::Size(width, height)); // Enable the NSView to accept first mouse event. bool acceptsFirstMouse = false; @@ -719,6 +713,7 @@ void NativeWindowMac::SetContentSizeConstraints( void NativeWindowMac::MoveTop() { [window_ orderWindow:NSWindowAbove relativeTo:0]; } + void NativeWindowMac::SetResizable(bool resizable) { SetStyleMask(resizable, NSResizableWindowMask); } @@ -979,9 +974,7 @@ bool NativeWindowMac::IsKiosk() { } void NativeWindowMac::SetBackgroundColor(SkColor color) { - base::ScopedCFTypeRef cgcolor( - skia::CGColorCreateFromSkColor(color)); - [[[window_ contentView] layer] setBackgroundColor:cgcolor]; + widget_->GetRootView()->SetBackground(views::CreateSolidBackground(color)); } void NativeWindowMac::SetHasShadow(bool has_shadow) { @@ -1307,6 +1300,10 @@ const views::Widget* NativeWindowMac::GetWidget() const { return widget_.get(); } +bool NativeWindowMac::CanResize() const { + return resizable_; +} + void NativeWindowMac::InternalSetParentWindow(NativeWindow* parent, bool attach) { if (is_modal()) diff --git a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm index b20a350cc7f2..5ddfab46ef7c 100644 --- a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm +++ b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm @@ -116,10 +116,12 @@ } - (void)windowDidResize:(NSNotification*)notification { + [super windowDidResize:notification]; shell_->NotifyWindowResize(); } - (void)windowDidMove:(NSNotification*)notification { + [super windowDidMove:notification]; // TODO(zcbenz): Remove the alias after figuring out a proper // way to dispatch move. shell_->NotifyWindowMove(); @@ -135,10 +137,12 @@ } - (void)windowDidMiniaturize:(NSNotification*)notification { + [super windowDidMiniaturize:notification]; shell_->NotifyWindowMinimize(); } - (void)windowDidDeminiaturize:(NSNotification*)notification { + [super windowDidDeminiaturize:notification]; [shell_->GetNativeWindow() setLevel:level_]; shell_->NotifyWindowRestore(); } From 5e48dd9d4571208cba63afa9173ca259d08fbe9c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Apr 2018 20:08:14 +0900 Subject: [PATCH 3/4] be aware of views::Widget's layer --- atom/browser/native_window_mac.mm | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index bb28bff0676f..60d6b93a87cb 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -27,6 +27,7 @@ #include "ui/gfx/skia_util.h" #include "ui/gl/gpu_switching_manager.h" #include "ui/views/background.h" +#include "ui/views/cocoa/bridged_native_widget.h" #include "ui/views/widget/widget.h" // Custom Quit, Minimize and Full Screen button container for frameless @@ -449,8 +450,13 @@ void NativeWindowMac::SetContentView( // Make sure the bottom corner is rounded for non-modal windows: // http://crbug.com/396264. But do not enable it on OS X 10.9 for transparent // window, otherwise a semi-transparent frame would show. - if (!(transparent() && base::mac::IsOS10_9()) && !is_modal()) + if (!(transparent() && base::mac::IsOS10_9()) && !is_modal()) { + base::scoped_nsobject background_layer([[CALayer alloc] init]); + [background_layer + setAutoresizingMask:kCALayerWidthSizable | kCALayerHeightSizable]; + [[window_ contentView] setLayer:background_layer]; [[window_ contentView] setWantsLayer:YES]; + } if (has_frame()) { [content_view_ setFrame:[[window_ contentView] bounds]]; @@ -974,7 +980,17 @@ bool NativeWindowMac::IsKiosk() { } void NativeWindowMac::SetBackgroundColor(SkColor color) { - widget_->GetRootView()->SetBackground(views::CreateSolidBackground(color)); + base::ScopedCFTypeRef cgcolor( + skia::CGColorCreateFromSkColor(color)); + // views::Widget adds a layer for the content view. + auto* bridge = views::NativeWidgetMac::GetBridgeForNativeWindow(window_); + NSView* compositor_superview = + static_cast(bridge)-> + AcceleratedWidgetGetNSView(); + [[compositor_superview layer] setBackgroundColor:cgcolor]; + // When using WebContents as content view, the contentView also has layer. + if ([[window_ contentView] wantsLayer]) + [[[window_ contentView] layer] setBackgroundColor:cgcolor]; } void NativeWindowMac::SetHasShadow(bool has_shadow) { From cf70267871ecf09ecff07973f5a85c69bf95c113 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 26 Apr 2018 09:48:12 +0900 Subject: [PATCH 4/4] remove unnecessary overrides --- atom/browser/native_window_mac.h | 1 - atom/browser/native_window_mac.mm | 3 --- atom/browser/ui/cocoa/atom_ns_window_delegate.mm | 4 ++-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 3c3de72a8071..19a0edc00a41 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -147,7 +147,6 @@ class NativeWindowMac : public NativeWindow, protected: // views::WidgetDelegate: - void DeleteDelegate() override; views::Widget* GetWidget() override; const views::Widget* GetWidget() const override; bool CanResize() const override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 60d6b93a87cb..182e3d79134c 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -1305,9 +1305,6 @@ gfx::Rect NativeWindowMac::WindowBoundsToContentBounds( } } -void NativeWindowMac::DeleteDelegate() { -} - views::Widget* NativeWindowMac::GetWidget() { return widget_.get(); } diff --git a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm index 5ddfab46ef7c..78bfe1fcd7b5 100644 --- a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm +++ b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm @@ -20,10 +20,10 @@ // on the fly. // TODO(zcbenz): Add interface in NativeWidgetMac to allow overriding creating // window delegate. - views::BridgedNativeWidget* bridget_view = + views::BridgedNativeWidget* bridged_view = views::NativeWidgetMac::GetBridgeForNativeWindow( shell->GetNativeWindow()); - if ((self = [super initWithBridgedNativeWidget:bridget_view])) { + if ((self = [super initWithBridgedNativeWidget:bridged_view])) { shell_ = shell; is_zooming_ = false; level_ = [shell_->GetNativeWindow() level];