MacViews: Support -[NSWindow close] on sheets.

ui/views/cocoa/views_nswindow_delegate.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ui/views/cocoa/views_nswindow_delegate.h"
 
 #include "base/logging.h"
+#import "base/mac/bind_objc_block.h"
+#include "base/threading/thread_task_runner_handle.h"
 #import "ui/views/cocoa/bridged_content_view.h"
 #import "ui/views/cocoa/bridged_native_widget.h"
 #include "ui/views/widget/native_widget_mac.h"
 
 @implementation ViewsNSWindowDelegate
 
 - (id)initWithBridgedNativeWidget:(views::BridgedNativeWidget*)parent {
   DCHECK(parent);
   if ((self = [super init])) {
     parent_ = parent;
@@ skipping 21 matching lines @@
   parent_->OnVisibilityChanged();
 }
 
 - (void)onSystemControlTintChanged:(NSNotification*)notification {
   parent_->OnSystemControlTintChanged();
 }
 
 - (void)sheetDidEnd:(NSWindow*)sheet
          returnCode:(NSInteger)returnCode
         contextInfo:(void*)contextInfo {
+  // |parent_| will be null when triggered from the block in -windowWillClose:.
+  if (!parent_)
+    return;
+
   [sheet orderOut:nil];
   parent_->OnWindowWillClose();
 }
 
 // NSWindowDelegate implementation.
 
 - (void)windowDidFailToEnterFullScreen:(NSWindow*)window {
   // Cocoa should already have sent an (unexpected) windowDidExitFullScreen:
   // notification, and the attempt to get back into fullscreen should fail.
   // Nothing to do except verify |parent_| is no longer trying to fullscreen.
@@ skipping 20 matching lines @@
 }
 
 - (void)windowDidBecomeKey:(NSNotification*)notification {
   parent_->OnWindowKeyStatusChangedTo(true);
 }
 
 - (void)windowDidResignKey:(NSNotification*)notification {
   parent_->OnWindowKeyStatusChangedTo(false);
 }
 
+// Does nothing: Used to force |self| to be retained in a block.

[Robert Sesek, 2017/01/13 20:02:47]

Could you instead just put another keepAlive in the block?

[tapted, 2017/01/13 20:52:51]

That would take (and release) an additional reference :). Currently there's just
the reference being held by the ObjC block machinery. i.e. It would work, but it
would effectively be replacing

[self doNothing];

with

[self retain];
[self release];

So I don't know if that's better. I do think `doNothing` is weird, but I didn't
want to use additional retain/release since it might suggest that the block
doesn't already have a ref to begin with that exists for the lifetime of the
block (or that we need to account for some other hidden weirdness..)

But I'm not otherwise attached to `doNothing` - e.g. I thought about calling
some random NSObject property, but thought "doNothing" gave slightly more
meaning.

[Robert Sesek, 2017/01/13 21:46:00]

Hm, I see your point. Maybe rather than doNothing, you end the block with [[self
retain] release]; ?

[tapted, 2017/01/13 22:32:45]

Done!

+- (void)doNothing {}
+
 - (void)windowWillClose:(NSNotification*)notification {
-  DCHECK([parent_->ns_window() isEqual:[notification object]]);
+  // Retain |self|. |parent_| should be cleared. OnWindowWillClose() may delete
+  // |parent_|, but it may also dealloc |self| before returning. However, the
+  // observers it notifies before that need a valid |parent_| on the delegate,
+  // so it can only be cleared after OnWindowWillClose() returns.
+  base::scoped_nsobject<NSObject> keepAlive(self, base::scoped_policy::RETAIN);
+  NSWindow* window = parent_->ns_window();
+  if (NSWindow* sheetParent = [window sheetParent]) {
+    // On no! Something called -[NSWindow close] on a sheet rather than calling
+    // -[NSWindow endSheet:] on its parent. If the modal session is not ended
+    // then the parent will never be able to show another sheet. But calling
+    // -endSheet: here will block the thread with an animation, so post a task.
+    // Use a block: The argument to -endSheet: must be retained, since it's the
+    // window that is closing and -performSelector: won't retain the argument.
+    // The NSWindowDelegate (i.e. |self|) must also be explicitly retained. Even
+    // though the call to OnWindowWillClose() below will remove |self| as the
+    // NSWindow delegate, the call to -[NSApp beginSheet:] also took a weak
+    // reference to the delegate, which will be destroyed when the sheet's
+    // BridgedNativeWidget is destroyed.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::BindBlock(^{
+      [sheetParent endSheet:window];
+      [self doNothing];
+    }));
+  }
+  DCHECK([window isEqual:[notification object]]);
   parent_->OnWindowWillClose();
+  parent_ = nullptr;
 }
 
 - (void)windowDidMiniaturize:(NSNotification*)notification {
   parent_->OnVisibilityChanged();
 }
 
 - (void)windowDidDeminiaturize:(NSNotification*)notification {
   parent_->OnVisibilityChanged();
 }
 
@@ skipping 33 matching lines @@
             usingRect:(NSRect)defaultSheetLocation {
   // As per NSWindowDelegate documentation, the origin indicates the top left
   // point of the host frame in window coordinates. The width changes the
   // animation from vertical to trapezoid if it is smaller than the width of the
   // dialog. The height is ignored but should be set to zero.
   return NSMakeRect(0, [self nativeWidgetMac]->SheetPositionY(),
                     NSWidth(defaultSheetLocation), 0);
 }
 
 @end