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

Add -webkit-app-region support to BrowserView #10232

Merged
merged 16 commits into from
Aug 24, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions atom/browser/native_browser_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,8 @@ NativeBrowserView::NativeBrowserView(

NativeBrowserView::~NativeBrowserView() {}

void NativeBrowserView::UpdateDraggableRegions(
std::vector<gfx::Rect> system_drag_exclude_areas) {
}

} // namespace atom
6 changes: 6 additions & 0 deletions atom/browser/native_browser_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include "base/macros.h"
#include "third_party/skia/include/core/SkColor.h"
#include "atom/common/draggable_region.h"
#include <vector>

namespace brightray {
class InspectableWebContentsView;
Expand Down Expand Up @@ -38,6 +40,10 @@ class NativeBrowserView {
virtual void SetBounds(const gfx::Rect& bounds) = 0;
virtual void SetBackgroundColor(SkColor color) = 0;

// Called when the window needs to update its draggable region.
virtual void UpdateDraggableRegions(
std::vector<gfx::Rect> system_drag_exclude_areas);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use const std::vector<gfx::Rect& to avoid copying.

Could you also use {} and get rid of the change to native_browser_view.cc.


protected:
explicit NativeBrowserView(
brightray::InspectableWebContentsView* web_contents_view);
Expand Down
3 changes: 3 additions & 0 deletions atom/browser/native_browser_view_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#import <Cocoa/Cocoa.h>

#include "atom/common/draggable_region.h"
#include "base/mac/scoped_nsobject.h"
#include "atom/browser/native_browser_view.h"

namespace atom {
Expand All @@ -20,6 +22,7 @@ class NativeBrowserViewMac : public NativeBrowserView {
void SetAutoResizeFlags(uint8_t flags) override;
void SetBounds(const gfx::Rect& bounds) override;
void SetBackgroundColor(SkColor color) override;
void UpdateDraggableRegions(std::vector<gfx::Rect> system_drag_exclude_areas) override;

private:
DISALLOW_COPY_AND_ASSIGN(NativeBrowserViewMac);
Expand Down
131 changes: 131 additions & 0 deletions atom/browser/native_browser_view_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,100 @@
#include "skia/ext/skia_utils_mac.h"
#include "ui/gfx/geometry/rect.h"

using base::scoped_nsobject;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?


// Match view::Views behavior where the view sticks to the top-left origin.
const NSAutoresizingMaskOptions kDefaultAutoResizingMask =
NSViewMaxXMargin | NSViewMinYMargin;

@interface DragRegionView : NSView

@property (assign) NSPoint initialLocation;

@end

@implementation DragRegionView

- (BOOL)mouseDownCanMoveWindow
{
return NO;
}

- (NSView *)hitTest:(NSPoint)aPoint
{
// Pass-through events that don't hit one of the exclusion zones
for (NSView *exlusion_zones in [self subviews]) {
if ([exlusion_zones hitTest:aPoint])
return nil;
}

return self;
}

- (void)mouseDown:(NSEvent *)event
{
if ([self.window respondsToSelector:@selector(performWindowDragWithEvent:)]) {
[self.window performWindowDragWithEvent:event];
return;
}

self.initialLocation = [event locationInWindow];
}

- (void)mouseDragged:(NSEvent *)theEvent
{
if ([self.window respondsToSelector:@selector(performWindowDragWithEvent:)]) {
return;
}

NSPoint currentLocation;
NSPoint newOrigin;

NSRect screenFrame = [[NSScreen mainScreen] frame];
NSRect windowFrame = [self.window frame];

currentLocation = [NSEvent mouseLocation];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could move these up so that we don't have to leave currentLocation and newOrigin uninitailized. You could also you NSMakeRect to initialize newOrigin in one line.

newOrigin.x = currentLocation.x - self.initialLocation.x;
newOrigin.y = currentLocation.y - self.initialLocation.y;

// Don't let window get dragged up under the menu bar
if( (newOrigin.y+windowFrame.size.height) > (screenFrame.origin.y+screenFrame.size.height) ){
newOrigin.y=screenFrame.origin.y + (screenFrame.size.height-windowFrame.size.height);
}

// Move the window to the new location
[self.window setFrameOrigin:newOrigin];
}

// Debugging tips:
// Uncomment the following four lines to color DragRegionView bright red
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be in a:

#ifdef DEBUG_DRAG_REGIONS
...
#endif

with a comment at the top:

// Uncomment to...
//#define DEBUG_DRAG_REGIONS

// - (void)drawRect:(NSRect)aRect
// {
// [[NSColor redColor] set];
// NSRectFill([self bounds]);
// }

@end

@interface ExcludeDragRegionView : NSView
@end

@implementation ExcludeDragRegionView

- (BOOL)mouseDownCanMoveWindow {
return NO;
}

// Debugging tips:
// Uncomment the following four lines to color ExcludeDragRegionView bright red
// - (void)drawRect:(NSRect)aRect
// {
// [[NSColor greenColor] set];
// NSRectFill([self bounds]);
// }

@end

namespace atom {

NativeBrowserViewMac::NativeBrowserViewMac(
Expand Down Expand Up @@ -51,6 +141,47 @@
view.layer.backgroundColor = skia::CGColorCreateFromSkColor(color);
}

void NativeBrowserViewMac::UpdateDraggableRegions(
std::vector<gfx::Rect> system_drag_exclude_areas) {
NSView* webView = GetInspectableWebContentsView()->GetNativeView();
NSInteger webViewHeight = NSHeight([webView bounds]);
NSInteger webViewWidth = NSWidth([webView bounds]);

// 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<NSArray> 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<NSView> dragRegion(
[[DragRegionView alloc] initWithFrame:NSZeroRect]);
[dragRegion setFrame:NSMakeRect(0,
0,
webViewWidth,
webViewHeight)];

// Then, on top of that, add "exclusion zones"
for (std::vector<gfx::Rect>::const_iterator iter =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use auto instead of std::vector<gfx::Rect>::const_iterator.

system_drag_exclude_areas.begin();
iter != system_drag_exclude_areas.end();
++iter) {
base::scoped_nsobject<NSView> controlRegion(
[[ExcludeDragRegionView alloc] initWithFrame:NSZeroRect]);
[controlRegion setFrame:NSMakeRect(iter->x(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a good chance I'm missing something, but just in case: is this assuming the browser view bounds are 100% of the containing window? Should use view.frame and view.superview.frame to handle the case where the browserview.SetBounds() has been used to position it in the bottom right corner?

webViewHeight - iter->bottom(),
iter->width(),
iter->height())];
[dragRegion addSubview:controlRegion];
}

// Add the DragRegion to the WebView
[webView addSubview:dragRegion];
}

// static
NativeBrowserView* NativeBrowserView::Create(
brightray::InspectableWebContentsView* web_contents_view) {
Expand Down
2 changes: 1 addition & 1 deletion atom/browser/native_window_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class NativeWindowMac : public NativeWindow,
void UninstallView();

// Install the drag view, which will cover the whole window and decides
// whehter we can drag.
// whether we can drag.
void UpdateDraggableRegionViews(const std::vector<DraggableRegion>& regions);

void RegisterInputEventObserver(content::RenderViewHost* host);
Expand Down
6 changes: 6 additions & 0 deletions atom/browser/native_window_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1767,6 +1767,12 @@ static bool FromV8(v8::Isolate* isolate, v8::Handle<v8::Value> val,
std::vector<gfx::Rect> system_drag_exclude_areas =
CalculateNonDraggableRegions(regions, webViewWidth, webViewHeight);

// Call down to a BrowserView, if it exists, and inform it about the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment is not needed, the code is sufficiently clear.

// draggable areas
if (browser_view_) {
browser_view_->UpdateDraggableRegions(system_drag_exclude_areas);
}

// Create and add a ControlRegionView for each region that needs to be
// excluded from the dragging.
for (std::vector<gfx::Rect>::const_iterator iter =
Expand Down