Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(405)

Unified Diff: ui/views/cocoa/bridged_native_widget.mm

Issue 2393843002: MacViews: Implement CloseNow() as just -[NSWindow close]. (Closed)
Patch Set: Respond to comments Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | ui/views/cocoa/bridged_native_widget_unittest.mm » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/views/cocoa/bridged_native_widget.mm
diff --git a/ui/views/cocoa/bridged_native_widget.mm b/ui/views/cocoa/bridged_native_widget.mm
index 8e47513350dc150013b5a19023ea9e969f42d8e1..c468ce88044fe01bb7e77e61f44678e734073375 100644
--- a/ui/views/cocoa/bridged_native_widget.mm
+++ b/ui/views/cocoa/bridged_native_widget.mm
@@ -353,42 +353,16 @@ BridgedNativeWidget::BridgedNativeWidget(NativeWidgetMac* parent)
}
BridgedNativeWidget::~BridgedNativeWidget() {
- bool close_window = false;
- if ([window_ delegate]) {
- // If the delegate is still set on a modal dialog, it means it was not
- // closed via [NSApplication endSheet:]. This is probably OK if the widget
- // was never shown. But Cocoa ignores close() calls on open sheets. Calling
- // endSheet: here would work, but it messes up assumptions elsewhere. E.g.
- // DialogClientView assumes its delegate is alive when closing, which isn't
- // true after endSheet: synchronously calls OnNativeWidgetDestroyed().
- // So ban it. Modal dialogs should be closed via Widget::Close().
- DCHECK(!native_widget_mac_->IsWindowModalSheet());
-
- // If the delegate is still set, it means OnWindowWillClose() has not been
- // called and the window is still open. Usually, -[NSWindow close] would
- // synchronously call OnWindowWillClose() which removes the delegate and
- // notifies NativeWidgetMac, which then calls this with a nil delegate.
- // For other teardown flows (e.g. Widget::WIDGET_OWNS_NATIVE_WIDGET or
- // Widget::CloseNow()) the delegate must first be cleared to avoid AppKit
- // calling back into the bridge. This means OnWindowWillClose() needs to be
- // invoked manually, which is done below.
- // Note that if the window has children it can't be closed until the
- // children are gone, but removing child windows calls into AppKit for the
- // parent window, so the delegate must be cleared first.
- [window_ setDelegate:nil];
- close_window = true;
- }
+ // The delegate should be cleared already. Note this enforces the precondition
+ // that -[NSWindow close] is invoked on the hosted window before the
+ // destructor is called.
+ DCHECK(![window_ delegate]);
RemoveOrDestroyChildren();
DCHECK(child_windows_.empty());
SetFocusManager(nullptr);
SetRootView(nullptr);
DestroyCompositor();
-
- if (close_window) {
- OnWindowWillClose();
- [window_ close];
- }
}
void BridgedNativeWidget::Init(base::scoped_nsobject<NSWindow> window,
@@ -706,6 +680,8 @@ void BridgedNativeWidget::SetCursor(NSCursor* cursor) {
}
void BridgedNativeWidget::OnWindowWillClose() {
+ native_widget_mac_->GetWidget()->OnNativeWidgetDestroying();
+
// Ensure BridgedNativeWidget does not have capture, otherwise
// OnMouseCaptureLost() may reference a deleted |native_widget_mac_| when
// called via ~CocoaMouseCapture() upon the destruction of |mouse_capture_|.
@@ -719,10 +695,9 @@ void BridgedNativeWidget::OnWindowWillClose() {
parent_ = nullptr;
}
[[NSNotificationCenter defaultCenter] removeObserver:window_delegate_];
- // Note this also clears the NSWindow delegate, after informing Widget
- // delegates about the closure. NativeWidgetMac then deletes |this| before
- // returning.
- native_widget_mac_->OnWindowWillClose();
+ [window_ setDelegate:nil];
+ native_widget_mac_->OnWindowDestroyed();
+ // Note: |this| is deleted here.
}
void BridgedNativeWidget::OnFullscreenTransitionStart(
« no previous file with comments | « no previous file | ui/views/cocoa/bridged_native_widget_unittest.mm » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698