From 61160ff9e5ddb4ae06d896f383908186c2889a50 Mon Sep 17 00:00:00 2001 From: Birunthan Mohanathas Date: Mon, 19 Mar 2018 09:50:30 +0200 Subject: [PATCH 1/5] Store InspectableWebContents instead of InspectableWebContentsView in NativeBrowserView --- atom/browser/api/atom_api_browser_view.cc | 4 ++-- atom/browser/native_browser_view.cc | 11 ++++++++--- atom/browser/native_browser_view.h | 13 ++++++++----- atom/browser/native_browser_view_mac.h | 3 ++- atom/browser/native_browser_view_mac.mm | 9 +++++---- atom/browser/native_browser_view_views.cc | 8 ++++---- atom/browser/native_browser_view_views.h | 2 +- 7 files changed, 30 insertions(+), 20 deletions(-) diff --git a/atom/browser/api/atom_api_browser_view.cc b/atom/browser/api/atom_api_browser_view.cc index c64054cdac2e..82e13a385f43 100644 --- a/atom/browser/api/atom_api_browser_view.cc +++ b/atom/browser/api/atom_api_browser_view.cc @@ -68,8 +68,8 @@ void BrowserView::Init(v8::Isolate* isolate, web_contents_.Reset(isolate, web_contents.ToV8()); api_web_contents_ = web_contents.get(); - view_.reset(NativeBrowserView::Create( - api_web_contents_->managed_web_contents()->GetView())); + view_.reset( + NativeBrowserView::Create(api_web_contents_->managed_web_contents())); InitWith(isolate, wrapper); } diff --git a/atom/browser/native_browser_view.cc b/atom/browser/native_browser_view.cc index c97fe683e6c5..4fd5889597b9 100644 --- a/atom/browser/native_browser_view.cc +++ b/atom/browser/native_browser_view.cc @@ -7,14 +7,19 @@ #include "atom/browser/native_browser_view.h" #include "atom/browser/api/atom_api_web_contents.h" -#include "brightray/browser/inspectable_web_contents_view.h" +#include "brightray/browser/inspectable_web_contents.h" namespace atom { NativeBrowserView::NativeBrowserView( - brightray::InspectableWebContentsView* web_contents_view) - : web_contents_view_(web_contents_view) {} + brightray::InspectableWebContents* inspectable_web_contents) + : inspectable_web_contents_(inspectable_web_contents) {} NativeBrowserView::~NativeBrowserView() {} +brightray::InspectableWebContentsView* +NativeBrowserView::GetInspectableWebContentsView() { + return inspectable_web_contents_->GetView(); +} + } // namespace atom diff --git a/atom/browser/native_browser_view.h b/atom/browser/native_browser_view.h index 7c154b60b00e..b58af0fd834b 100644 --- a/atom/browser/native_browser_view.h +++ b/atom/browser/native_browser_view.h @@ -12,6 +12,7 @@ #include "third_party/skia/include/core/SkColor.h" namespace brightray { +class InspectableWebContents; class InspectableWebContentsView; } @@ -31,12 +32,14 @@ class NativeBrowserView { virtual ~NativeBrowserView(); static NativeBrowserView* Create( - brightray::InspectableWebContentsView* web_contents_view); + brightray::InspectableWebContents* inspectable_web_contents); - brightray::InspectableWebContentsView* GetInspectableWebContentsView() { - return web_contents_view_; + brightray::InspectableWebContents* GetInspectableWebContents() { + return inspectable_web_contents_; } + brightray::InspectableWebContentsView* GetInspectableWebContentsView(); + virtual void SetAutoResizeFlags(uint8_t flags) = 0; virtual void SetBounds(const gfx::Rect& bounds) = 0; virtual void SetBackgroundColor(SkColor color) = 0; @@ -47,9 +50,9 @@ class NativeBrowserView { protected: explicit NativeBrowserView( - brightray::InspectableWebContentsView* web_contents_view); + brightray::InspectableWebContents* inspectable_web_contents); - brightray::InspectableWebContentsView* web_contents_view_; + brightray::InspectableWebContents* inspectable_web_contents_; private: DISALLOW_COPY_AND_ASSIGN(NativeBrowserView); diff --git a/atom/browser/native_browser_view_mac.h b/atom/browser/native_browser_view_mac.h index 0b04edad729f..726ca75433e8 100644 --- a/atom/browser/native_browser_view_mac.h +++ b/atom/browser/native_browser_view_mac.h @@ -17,12 +17,13 @@ namespace atom { class NativeBrowserViewMac : public NativeBrowserView { public: explicit NativeBrowserViewMac( - brightray::InspectableWebContentsView* web_contents_view); + brightray::InspectableWebContents* inspectable_web_contents); ~NativeBrowserViewMac() override; void SetAutoResizeFlags(uint8_t flags) override; void SetBounds(const gfx::Rect& bounds) override; void SetBackgroundColor(SkColor color) override; + void UpdateDraggableRegions( const std::vector& system_drag_exclude_areas) override; diff --git a/atom/browser/native_browser_view_mac.mm b/atom/browser/native_browser_view_mac.mm index 17b0a1ee9006..9634f20f9828 100644 --- a/atom/browser/native_browser_view_mac.mm +++ b/atom/browser/native_browser_view_mac.mm @@ -4,6 +4,7 @@ #include "atom/browser/native_browser_view_mac.h" +#include "brightray/browser/inspectable_web_contents.h" #include "brightray/browser/inspectable_web_contents_view.h" #include "skia/ext/skia_utils_mac.h" #include "ui/gfx/geometry/rect.h" @@ -156,8 +157,8 @@ const NSAutoresizingMaskOptions kDefaultAutoResizingMask = namespace atom { NativeBrowserViewMac::NativeBrowserViewMac( - brightray::InspectableWebContentsView* web_contents_view) - : NativeBrowserView(web_contents_view) { + brightray::InspectableWebContents* inspectable_web_contents) + : NativeBrowserView(inspectable_web_contents) { auto* view = GetInspectableWebContentsView()->GetNativeView(); view.autoresizingMask = kDefaultAutoResizingMask; } @@ -247,8 +248,8 @@ void NativeBrowserViewMac::UpdateDraggableRegions( // static NativeBrowserView* NativeBrowserView::Create( - brightray::InspectableWebContentsView* web_contents_view) { - return new NativeBrowserViewMac(web_contents_view); + brightray::InspectableWebContents* inspectable_web_contents) { + return new NativeBrowserViewMac(inspectable_web_contents); } } // namespace atom diff --git a/atom/browser/native_browser_view_views.cc b/atom/browser/native_browser_view_views.cc index 78e788b3019c..def64d12e352 100644 --- a/atom/browser/native_browser_view_views.cc +++ b/atom/browser/native_browser_view_views.cc @@ -12,8 +12,8 @@ namespace atom { NativeBrowserViewViews::NativeBrowserViewViews( - brightray::InspectableWebContentsView* web_contents_view) - : NativeBrowserView(web_contents_view) {} + brightray::InspectableWebContents* inspectable_web_contents) + : NativeBrowserView(inspectable_web_contents) {} NativeBrowserViewViews::~NativeBrowserViewViews() {} @@ -29,8 +29,8 @@ void NativeBrowserViewViews::SetBackgroundColor(SkColor color) { // static NativeBrowserView* NativeBrowserView::Create( - brightray::InspectableWebContentsView* web_contents_view) { - return new NativeBrowserViewViews(web_contents_view); + brightray::InspectableWebContents* inspectable_web_contents) { + return new NativeBrowserViewViews(inspectable_web_contents); } } // namespace atom diff --git a/atom/browser/native_browser_view_views.h b/atom/browser/native_browser_view_views.h index 5dcda13447cd..abf47178302a 100644 --- a/atom/browser/native_browser_view_views.h +++ b/atom/browser/native_browser_view_views.h @@ -12,7 +12,7 @@ namespace atom { class NativeBrowserViewViews : public NativeBrowserView { public: explicit NativeBrowserViewViews( - brightray::InspectableWebContentsView* web_contents_view); + brightray::InspectableWebContents* inspectable_web_contents); ~NativeBrowserViewViews() override; uint8_t GetAutoResizeFlags() { return auto_resize_flags_; } From 377e6c3210c8fe100ee505f6b2a758b9be6fb3c5 Mon Sep 17 00:00:00 2001 From: Birunthan Mohanathas Date: Mon, 19 Mar 2018 13:21:03 +0200 Subject: [PATCH 2/5] Rename system_drag_exclude_areas => drag_exclude_rects --- .../api/atom_api_browser_window_mac.mm | 20 +++++++------------ atom/browser/native_browser_view_mac.mm | 14 ++++++------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/atom/browser/api/atom_api_browser_window_mac.mm b/atom/browser/api/atom_api_browser_window_mac.mm index 95c6c43e826c..46cabd5062e7 100644 --- a/atom/browser/api/atom_api_browser_window_mac.mm +++ b/atom/browser/api/atom_api_browser_window_mac.mm @@ -80,30 +80,24 @@ void BrowserWindow::UpdateDraggableRegions( // Draggable regions is implemented by having the whole web view draggable // (mouseDownCanMoveWindow) and overlaying regions that are not draggable. draggable_regions_ = regions; - std::vector system_drag_exclude_areas; + std::vector drag_exclude_rects; if (regions.empty()) { - system_drag_exclude_areas.push_back( - gfx::Rect(0, 0, webViewWidth, webViewHeight)); + drag_exclude_rects.push_back(gfx::Rect(0, 0, webViewWidth, webViewHeight)); } else { - system_drag_exclude_areas = CalculateNonDraggableRegions( + drag_exclude_rects = CalculateNonDraggableRegions( DraggableRegionsToSkRegion(regions), webViewWidth, webViewHeight); } if (window_->browser_view()) - window_->browser_view()->UpdateDraggableRegions(system_drag_exclude_areas); + window_->browser_view()->UpdateDraggableRegions(drag_exclude_rects); // Create and add a ControlRegionView for each region that needs to be // excluded from the dragging. - for (std::vector::const_iterator iter = - system_drag_exclude_areas.begin(); - iter != system_drag_exclude_areas.end(); - ++iter) { + for (const auto& rect : drag_exclude_rects) { base::scoped_nsobject controlRegion( [[ControlRegionView alloc] initWithFrame:NSZeroRect]); - [controlRegion setFrame:NSMakeRect(iter->x(), - webViewHeight - iter->bottom(), - iter->width(), - iter->height())]; + [controlRegion setFrame:NSMakeRect(rect.x(), webViewHeight - rect.bottom(), + rect.width(), rect.height())]; [webView addSubview:controlRegion]; } diff --git a/atom/browser/native_browser_view_mac.mm b/atom/browser/native_browser_view_mac.mm index 9634f20f9828..1bb73e788362 100644 --- a/atom/browser/native_browser_view_mac.mm +++ b/atom/browser/native_browser_view_mac.mm @@ -194,7 +194,7 @@ void NativeBrowserViewMac::SetBackgroundColor(SkColor color) { } void NativeBrowserViewMac::UpdateDraggableRegions( - const std::vector& system_drag_exclude_areas) { + const std::vector& drag_exclude_rects) { NSView* webView = GetInspectableWebContentsView()->GetNativeView(); NSInteger superViewHeight = NSHeight([webView.superview bounds]); @@ -230,15 +230,13 @@ void NativeBrowserViewMac::UpdateDraggableRegions( webViewHeight)]; // Then, on top of that, add "exclusion zones" - for (auto iter = system_drag_exclude_areas.begin(); - iter != system_drag_exclude_areas.end(); - ++iter) { + for (const auto& rect : drag_exclude_rects) { base::scoped_nsobject controlRegion( [[ExcludeDragRegionView alloc] initWithFrame:NSZeroRect]); - [controlRegion setFrame:NSMakeRect(iter->x() - webViewX, - webViewHeight - iter->bottom() + webViewY, - iter->width(), - iter->height())]; + [controlRegion setFrame:NSMakeRect(rect.x() - webViewX, + webViewHeight - rect.bottom() + webViewY, + rect.width(), + rect.height())]; [dragRegion addSubview:controlRegion]; } From 3b8ddd09977d81c690aa6c9feea0b67630251e03 Mon Sep 17 00:00:00 2001 From: Birunthan Mohanathas Date: Mon, 19 Mar 2018 13:39:58 +0200 Subject: [PATCH 3/5] Use NSView convertRect:toView: for BrowserView DragRegionView positioning --- atom/browser/native_browser_view_mac.mm | 63 ++++++++++--------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/atom/browser/native_browser_view_mac.mm b/atom/browser/native_browser_view_mac.mm index 1bb73e788362..853652cc084d 100644 --- a/atom/browser/native_browser_view_mac.mm +++ b/atom/browser/native_browser_view_mac.mm @@ -195,53 +195,38 @@ void NativeBrowserViewMac::SetBackgroundColor(SkColor color) { void NativeBrowserViewMac::UpdateDraggableRegions( const std::vector& drag_exclude_rects) { - NSView* webView = GetInspectableWebContentsView()->GetNativeView(); + NSView* inspectable_view = GetInspectableWebContentsView()->GetNativeView(); + NSView* window_content_view = inspectable_view.superview; + const auto window_content_view_height = NSHeight(window_content_view.bounds); - NSInteger superViewHeight = NSHeight([webView.superview bounds]); - NSInteger webViewHeight = NSHeight([webView bounds]); - NSInteger webViewWidth = NSWidth([webView bounds]); - NSInteger webViewX = NSMinX([webView frame]); - NSInteger webViewY = 0; - - // Apple's NSViews have their coordinate system originate at the bottom left, - // meaning that we need to be a bit smarter when it comes to calculating our - // current top offset - if (webViewHeight > superViewHeight) { - webViewY = std::abs(webViewHeight - superViewHeight - (std::abs(NSMinY([webView frame])))); - } else { - webViewY = superViewHeight - NSMaxY([webView frame]); + // Remove all DragRegionViews that were added last time. Note that we need + // to copy the `subviews` array to avoid mutation during iteration. + base::scoped_nsobject subviews([[inspectable_view subviews] copy]); + for (NSView* subview in subviews.get()) { + if ([subview isKindOfClass:[DragRegionView class]]) { + [subview removeFromSuperview]; + } } - // Remove all DraggableRegionViews that are added last time. - // Note that [webView subviews] returns the view's mutable internal array and - // it should be copied to avoid mutating the original array while enumerating - // it. - base::scoped_nsobject subviews([[webView subviews] copy]); - for (NSView* subview in subviews.get()) - if ([subview isKindOfClass:[DragRegionView class]]) - [subview removeFromSuperview]; - // Create one giant NSView that is draggable. - base::scoped_nsobject dragRegion( - [[DragRegionView alloc] initWithFrame:NSZeroRect]); - [dragRegion setFrame:NSMakeRect(0, - 0, - webViewWidth, - webViewHeight)]; + base::scoped_nsobject drag_region_view( + [[DragRegionView alloc] initWithFrame:inspectable_view.bounds]); + [inspectable_view addSubview:drag_region_view]; // Then, on top of that, add "exclusion zones" for (const auto& rect : drag_exclude_rects) { - base::scoped_nsobject controlRegion( - [[ExcludeDragRegionView alloc] initWithFrame:NSZeroRect]); - [controlRegion setFrame:NSMakeRect(rect.x() - webViewX, - webViewHeight - rect.bottom() + webViewY, - rect.width(), - rect.height())]; - [dragRegion addSubview:controlRegion]; - } + const auto window_content_view_exclude_rect = + NSMakeRect(rect.x(), window_content_view_height - rect.bottom(), + rect.width(), rect.height()); + const auto drag_region_view_exclude_rect = + [window_content_view convertRect:window_content_view_exclude_rect + toView:drag_region_view]; - // Add the DragRegion to the WebView - [webView addSubview:dragRegion]; + base::scoped_nsobject exclude_drag_region_view( + [[ExcludeDragRegionView alloc] + initWithFrame:drag_region_view_exclude_rect]); + [drag_region_view addSubview:exclude_drag_region_view]; + } } // static From 42934a10061875146afcbd3d99f00689fb1d6216 Mon Sep 17 00:00:00 2001 From: Birunthan Mohanathas Date: Mon, 19 Mar 2018 20:45:40 +0200 Subject: [PATCH 4/5] Make BrowserView DragRegionViews children of the WebContents view Previously they were children of the `InspectableWebContentsView` view, which caused this assertion to fail: https://github.com/electron/electron/blob/f9938884248627335c59da6b3b0ff0dc7df3b258/brightray/browser/mac/bry_inspectable_web_contents_view.mm#L162 --- atom/browser/native_browser_view.cc | 4 ++++ atom/browser/native_browser_view.h | 2 ++ atom/browser/native_browser_view_mac.mm | 7 ++++--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/atom/browser/native_browser_view.cc b/atom/browser/native_browser_view.cc index 4fd5889597b9..6460f2bfbd22 100644 --- a/atom/browser/native_browser_view.cc +++ b/atom/browser/native_browser_view.cc @@ -22,4 +22,8 @@ NativeBrowserView::GetInspectableWebContentsView() { return inspectable_web_contents_->GetView(); } +content::WebContents* NativeBrowserView::GetWebContents() { + return inspectable_web_contents_->GetWebContents(); +} + } // namespace atom diff --git a/atom/browser/native_browser_view.h b/atom/browser/native_browser_view.h index b58af0fd834b..cab6980fa2cf 100644 --- a/atom/browser/native_browser_view.h +++ b/atom/browser/native_browser_view.h @@ -9,6 +9,7 @@ #include "atom/common/draggable_region.h" #include "base/macros.h" +#include "content/public/browser/web_contents.h" #include "third_party/skia/include/core/SkColor.h" namespace brightray { @@ -39,6 +40,7 @@ class NativeBrowserView { } brightray::InspectableWebContentsView* GetInspectableWebContentsView(); + content::WebContents* GetWebContents(); virtual void SetAutoResizeFlags(uint8_t flags) = 0; virtual void SetBounds(const gfx::Rect& bounds) = 0; diff --git a/atom/browser/native_browser_view_mac.mm b/atom/browser/native_browser_view_mac.mm index 853652cc084d..86df632638ef 100644 --- a/atom/browser/native_browser_view_mac.mm +++ b/atom/browser/native_browser_view_mac.mm @@ -195,13 +195,14 @@ void NativeBrowserViewMac::SetBackgroundColor(SkColor color) { void NativeBrowserViewMac::UpdateDraggableRegions( const std::vector& drag_exclude_rects) { + NSView* web_view = GetWebContents()->GetNativeView(); NSView* inspectable_view = GetInspectableWebContentsView()->GetNativeView(); NSView* window_content_view = inspectable_view.superview; const auto window_content_view_height = NSHeight(window_content_view.bounds); // Remove all DragRegionViews that were added last time. Note that we need // to copy the `subviews` array to avoid mutation during iteration. - base::scoped_nsobject subviews([[inspectable_view subviews] copy]); + base::scoped_nsobject subviews([[web_view subviews] copy]); for (NSView* subview in subviews.get()) { if ([subview isKindOfClass:[DragRegionView class]]) { [subview removeFromSuperview]; @@ -210,8 +211,8 @@ void NativeBrowserViewMac::UpdateDraggableRegions( // Create one giant NSView that is draggable. base::scoped_nsobject drag_region_view( - [[DragRegionView alloc] initWithFrame:inspectable_view.bounds]); - [inspectable_view addSubview:drag_region_view]; + [[DragRegionView alloc] initWithFrame:web_view.bounds]); + [web_view addSubview:drag_region_view]; // Then, on top of that, add "exclusion zones" for (const auto& rect : drag_exclude_rects) { From 20a0508a168accda828929885683fd7f097c57c3 Mon Sep 17 00:00:00 2001 From: Birunthan Mohanathas Date: Mon, 19 Mar 2018 20:45:42 +0200 Subject: [PATCH 5/5] Update draggable regions when changing BrowserView Fixes #12150. --- atom/browser/api/atom_api_browser_window.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atom/browser/api/atom_api_browser_window.cc b/atom/browser/api/atom_api_browser_window.cc index 57c06dccca12..362ca1ac9d89 100644 --- a/atom/browser/api/atom_api_browser_window.cc +++ b/atom/browser/api/atom_api_browser_window.cc @@ -1057,6 +1057,10 @@ void BrowserWindow::SetBrowserView(v8::Local value) { window_->SetBrowserView(browser_view->view()); browser_view->web_contents()->SetOwnerWindow(window_.get()); browser_view_.Reset(isolate(), value); + +#if defined(OS_MACOSX) + UpdateDraggableRegions(nullptr, draggable_regions_); +#endif } }