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

Refactor NativeWindow (Part 9): Use views::Widget on macOS #12696

Merged
merged 4 commits into from Apr 26, 2018

Conversation

Projects
None yet
2 participants
@zcbenz
Contributor

zcbenz commented Apr 24, 2018

This PR uses views::Widget to replace current manual creation of NSWindow, it would allow us to put arbitrary views::View inside TopLevelWindow on macOS.

To make the migration graceful, I'm still using our own logics for managing window's styles and states, so this change should have minimal affects on window's behaviors. But it can still cause quirks and regressions since Chromium is adding a lot of custom logics to NSWindow in views::Widget, please let me know if you noticed any quirk after this change.

@zcbenz zcbenz requested a review from electron/reviewers as a code owner Apr 24, 2018

options.Get(options::kSimpleFullScreen, &always_simple_fullscreen_);
if (!has_frame() || use_content_size)
SetContentSize(gfx::Size(width, height));

This comment has been minimized.

@nornagon

nornagon Apr 26, 2018

Contributor

does this change the semantic meaning of width and height to refer to the contents rather than the whole window (including the border)?

@nornagon

nornagon Apr 26, 2018

Contributor

does this change the semantic meaning of width and height to refer to the contents rather than the whole window (including the border)?

This comment has been minimized.

@zcbenz

zcbenz Apr 26, 2018

Contributor

The views::Widget forces setting window size with window borders, which is opposite to when we manually created NSWindow. So I need to replace SetSize with SetContentSize here, for frameless window both calls are the same.

@zcbenz

zcbenz Apr 26, 2018

Contributor

The views::Widget forces setting window size with window borders, which is opposite to when we manually created NSWindow. So I need to replace SetSize with SetContentSize here, for frameless window both calls are the same.

base::scoped_nsobject<CALayer> background_layer([[CALayer alloc] init]);
[background_layer
setAutoresizingMask:kCALayerWidthSizable | kCALayerHeightSizable];
[[window_ contentView] setLayer:background_layer];

This comment has been minimized.

@nornagon

nornagon Apr 26, 2018

Contributor

i'm curious, what happened without the CALayer set?

@nornagon

nornagon Apr 26, 2018

Contributor

i'm curious, what happened without the CALayer set?

This comment has been minimized.

@zcbenz

zcbenz Apr 26, 2018

Contributor

Without setting a layer we would have a black flash when showing window. Cocoa seems to behave differently when we have more than one views with layers.

@zcbenz

zcbenz Apr 26, 2018

Contributor

Without setting a layer we would have a black flash when showing window. Cocoa seems to behave differently when we have more than one views with layers.

Show outdated Hide outdated atom/browser/native_window_mac.mm
Show outdated Hide outdated atom/browser/ui/cocoa/atom_ns_window_delegate.mm

@zcbenz zcbenz merged commit 152422a into master Apr 26, 2018

10 checks passed

WIP ready for review
Details
ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
ci/circleci: electron-mas-x64 Your tests passed on CircleCI!
Details
ci/circleci: electron-osx-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@zcbenz zcbenz deleted the mac-use-widget branch Apr 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment