bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Andrii Kolomoiets <andreyk.mad@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

Alan Third <alan@idiocy.org> writes:

> Thanks! This looks much better than what we had before. No crashes for
> one!
>
> A few nitpicks:
>
>> +   BOOL is_fullscreen_transition;
>
> I may be misunderstanding this, but would ‘in_fullscreen_transition’
> be a better name?

OK, renamed. 

> Opening braces go on a new line.

Braces style is fixed to match Emacs code style.

> And again. That ‘} else {’ should also be over three lines.
>
>>  #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
>> @@ -7932,9 +7998,14 @@ - (void)toggleFullScreen: (id)sender
>>      {
>>  #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
>>  #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
>> -      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
>> +      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)]){
>>  #endif
>>          [[self window] toggleFullScreen:sender];
>> +        // wait for fullscreen animation complete (bug#28496)
>> +        [self waitFullScreenTransition];
>> +#endif
>> +#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
>> +      }
>>  #endif
>>        return;
>>      }
>
> Should that final #if #endif not be within the outer #if #endif?

Yes. Fixed.

Thanks!

Please see attached patch. Log message is also provided.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-NS-child-frame-in-native-fullscreen.patch --]
[-- Type: text/x-patch, Size: 10837 bytes --]

From a9705617188c9a5ef77a8f2d5cc796b86dda9fa6 Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Tue, 10 Mar 2020 10:14:59 +0200
Subject: [PATCH] NS child frame in native fullscreen

* lisp/frame.el (toggle-frame-fullscreen): Don't sleep on cocoa.
Fullscreen animation waiting is moved to src/nsterm.m.
* src/nsterm.h (EmacsView): Add in_fullscreen_transition,
inFullScreenTransition, waitFullScreenTransition.
(NSWindowCollectionBehaviorFullScreenAuxiliary): New define.
* src/nsterm.m (ns_make_frame_visible): Wait for fullscreen animation.
(ns_set_parent_frame): Set frame collection behavior; make child frames
non-fullscreen; make non-child frames fullscreen if parent was fullscreen.
([EmacsView initFrameFromEmacs]): Set in_fullscreen_transition; set frame
collection behavior according to parent frame.
([EmacsView windowDidMove]): Remove code by commenting with "fixme".
([EmacsView windowWillEnterFullScreen], [EmacsView windowDidEnterFullScreen])
([EmacsView windowWillExitFullScreen], [EmacsView windowDidExitFullScreen]):
Set in_fullscreen_transition.
([EmacsView inFullScreenTransition], [EmacsView waitFullScreenTransition]):
New methods.
([EmacsView updateCollectionBehavior]): Set collection behavior according to
parent frame.
([EmacsView toggleFullScreen]): Wait for fullscreen animation.
---
 lisp/frame.el |   6 +---
 src/nsterm.h  |   4 +++
 src/nsterm.m  | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 16ee7580f8..7ed3e23f3c 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2676,11 +2676,7 @@ toggle-frame-fullscreen
 	      (set-frame-parameter frame 'fullscreen fullscreen-restore)
 	    (set-frame-parameter frame 'fullscreen nil)))
       (modify-frame-parameters
-       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))
-    ;; Manipulating a frame without waiting for the fullscreen
-    ;; animation to complete can cause a crash, or other unexpected
-    ;; behavior, on macOS (bug#28496).
-    (when (featurep 'cocoa) (sleep-for 0.5))))
+       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))))
 
 \f
 ;;;; Key bindings
diff --git a/src/nsterm.h b/src/nsterm.h
index db966e1581..8396a542f7 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -433,6 +433,7 @@ #define NS_DRAW_TO_BUFFER 1
    int maximized_width, maximized_height;
    NSWindow *nonfs_window;
    BOOL fs_is_native;
+   BOOL in_fullscreen_transition;
 #ifdef NS_DRAW_TO_BUFFER
    CGContextRef drawingBuffer;
 #endif
@@ -467,6 +468,8 @@ #define NS_DRAW_TO_BUFFER 1
 - (void) toggleFullScreen: (id) sender;
 - (BOOL) fsIsNative;
 - (BOOL) isFullscreen;
+- (BOOL) inFullScreenTransition;
+- (void) waitFullScreenTransition;
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void) updateCollectionBehavior;
 #endif
@@ -1286,6 +1289,7 @@ #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 #if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_7)
 #define NSFullScreenWindowMask                      (1 << 14)
 #define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSWindowCollectionBehaviorFullScreenAuxiliary (1 << 8)
 #define NSApplicationPresentationFullScreen         (1 << 10)
 #define NSApplicationPresentationAutoHideToolbar    (1 << 11)
 #define NSAppKitVersionNumber10_7                   1138
diff --git a/src/nsterm.m b/src/nsterm.m
index 851a5617d7..96a7fdc018 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1571,9 +1571,12 @@ -(void)remove
 
       /* Making a new frame from a fullscreen frame will make the new frame
          fullscreen also.  So skip handleFS as this will print an error.  */
-      if ([view fsIsNative] && f->want_fullscreen == FULLSCREEN_BOTH
-          && [view isFullscreen])
-        return;
+      if ([view fsIsNative] && [view isFullscreen])
+        {
+          // maybe it is not necessary to wait
+          [view waitFullScreenTransition];
+          return;
+        }
 
       if (f->want_fullscreen != FULLSCREEN_NONE)
         {
@@ -1959,19 +1962,55 @@ so some key presses (TAB) are swallowed by the system.  */
       block_input ();
       child = [FRAME_NS_VIEW (f) window];
 
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+      EmacsView *view = (EmacsView *)FRAME_NS_VIEW (f);
+#endif
+
       if ([child parentWindow] != nil)
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          parent = [child parentWindow];
+#endif
+
           [[child parentWindow] removeChildWindow:child];
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 101000
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101000
           if ([child respondsToSelector:@selector(setAccessibilitySubrole:)])
 #endif
               [child setAccessibilitySubrole:NSAccessibilityStandardWindowSubrole];
+#endif
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          if (NILP (new_value))
+            {
+              NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary");
+              [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+              // if current parent in fullscreen and no new parent make child fullscreen
+              while (parent) {
+                if (([parent styleMask] & NSWindowStyleMaskFullScreen) != 0)
+                  {
+                    [view toggleFullScreen:child];
+                    break;
+                  }
+                // check all parents
+                parent = [parent parentWindow];
+              }
+            }
 #endif
         }
 
       if (!NILP (new_value))
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          // child frame must not be in fullscreen
+          if ([view fsIsNative] && [view isFullscreen])
+            {
+              // in case child is going fullscreen
+              [view waitFullScreenTransition];
+              [view toggleFullScreen:child];
+            }
+          NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary");
+          [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+#endif
           parent = [FRAME_NS_VIEW (p) window];
 
           [parent addChildWindow: child
@@ -7398,6 +7437,7 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #endif
     fs_is_native = ns_use_native_fullscreen;
 #endif
+  in_fullscreen_transition = NO;
 
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
@@ -7431,7 +7471,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
-    [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+    if (FRAME_PARENT_FRAME (f))
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+    else
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
   wr = [win frame];
@@ -7554,11 +7597,12 @@ - (void)windowDidMove: sender
       emacsframe->top_pos =
         NS_PARENT_WINDOW_TOP_POS (emacsframe) - (r.origin.y + r.size.height);
 
-      if (emacs_event)
-        {
-          emacs_event->kind = MOVE_FRAME_EVENT;
-          EV_TRAILER ((id)nil);
-        }
+      // FIXME: after event part below didExitFullScreen is not received
+      // if (emacs_event)
+      //   {
+      //     emacs_event->kind = MOVE_FRAME_EVENT;
+      //     EV_TRAILER ((id)nil);
+      //   }
     }
 }
 
@@ -7758,6 +7802,7 @@ - (NSApplicationPresentationOptions)window:(NSWindow *)window
 - (void)windowWillEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillEnterFullScreen:]");
+  in_fullscreen_transition = YES;
   [self windowWillEnterFullScreen];
 }
 - (void)windowWillEnterFullScreen /* provided for direct calls */
@@ -7770,6 +7815,7 @@ - (void)windowDidEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidEnterFullScreen:]");
   [self windowDidEnterFullScreen];
+  in_fullscreen_transition = NO;
 }
 
 - (void)windowDidEnterFullScreen /* provided for direct calls */
@@ -7808,6 +7854,7 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
 - (void)windowWillExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillExitFullScreen:]");
+  in_fullscreen_transition = YES;
   [self windowWillExitFullScreen];
 }
 
@@ -7827,6 +7874,7 @@ - (void)windowDidExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidExitFullScreen:]");
   [self windowDidExitFullScreen];
+  in_fullscreen_transition = NO;
 }
 
 - (void)windowDidExitFullScreen /* provided for direct calls */
@@ -7856,6 +7904,22 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     [[self window] performZoom:self];
 }
 
+- (BOOL)inFullScreenTransition
+{
+  return in_fullscreen_transition;
+}
+
+- (void)waitFullScreenTransition
+{
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  while ([self inFullScreenTransition])
+    {
+      NSTRACE ("wait for fullscreen");
+      wait_reading_process_output (0, 300000000, 0, 1, Qnil, NULL, 0);
+    }
+#endif
+}
+
 - (BOOL)fsIsNative
 {
   return fs_is_native;
@@ -7894,9 +7958,22 @@ - (void)updateCollectionBehavior
       NSWindow *win = [self window];
       NSWindowCollectionBehavior b = [win collectionBehavior];
       if (ns_use_native_fullscreen)
-        b |= NSWindowCollectionBehaviorFullScreenPrimary;
+        {
+          if ([win parentWindow])
+            {
+              b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+              b |= NSWindowCollectionBehaviorFullScreenAuxiliary;
+            }
+          else
+            {
+              b |= NSWindowCollectionBehaviorFullScreenPrimary;
+              b &= ~NSWindowCollectionBehaviorFullScreenAuxiliary;
+            }
+        }
       else
-        b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+        {
+          b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+        }
 
       [win setCollectionBehavior: b];
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
@@ -7922,8 +7999,14 @@ - (void)toggleFullScreen: (id)sender
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
       if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
+        {
+#endif
+          [[self window] toggleFullScreen:sender];
+          // wait for fullscreen animation complete (bug#28496)
+          [self waitFullScreenTransition];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+        }
 #endif
-        [[self window] toggleFullScreen:sender];
 #endif
       return;
     }
-- 
2.15.1