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

From: Andrii Kolomoiets <[email-masked]>
Date: Fri, 14 Feb 2020 10:23:22 +0200

Andrii Kolomoiets <[email-masked]> writes:

>> On Jul 16, 2019, at 22:28, Alan Third <[email-masked]> wrote:
>> 
>> The patch looks good to me, but lets see if we can find solutions to
>> these other issues.
>
> I made some progress on this issue.
>
> The idea is to wait for fullscreen transition to complete:
> - Set property isFullScreenTransition on windowWillEnterFullScreen and
>   windowWillExitFullScreen events;
> - Clear that property on windowDidEnterFullScreen and
>   windowDidExitFullScreen events;
> - Wait for property to be cleared when we need to.

Updated patch to work with latest master

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))))))
 
 
 ;;;; Key bindings
diff --git a/src/nsterm.h b/src/nsterm.h
index 7c6197f128..243d66be60 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -417,6 +417,7 @@ #define NSTRACE_UNSILENCE()
    int maximized_width, maximized_height;
    NSWindow *nonfs_window;
    BOOL fs_is_native;
+   BOOL is_fullscreen_transition;
 #ifdef NS_IMPL_COCOA
    CGContextRef drawingBuffer;
 #endif
@@ -451,6 +452,8 @@ #define NSTRACE_UNSILENCE()
 - (void) toggleFullScreen: (id) sender;
 - (BOOL) fsIsNative;
 - (BOOL) isFullscreen;
+- (BOOL) isFullScreenTransition;
+- (void) waitFullScreenTransition;
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void) updateCollectionBehavior;
 #endif
@@ -1270,6 +1273,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 2cf6774a1f..b290fd3d95 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1588,9 +1588,11 @@ -(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])
+      if ([view fsIsNative] && [view isFullscreen]) {
+        // maybe it is not necessary to wait
+        [view waitFullScreenTransition];
         return;
+      }
 
       if (f->want_fullscreen != FULLSCREEN_NONE)
         {
@@ -1976,19 +1978,52 @@ 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
@@ -7411,6 +7446,7 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #endif
     fs_is_native = ns_use_native_fullscreen;
 #endif
+  is_fullscreen_transition = NO;
 
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
@@ -7442,7 +7478,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];
@@ -7565,11 +7604,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);
+      //   }
     }
 }
 
@@ -7769,6 +7809,7 @@ - (NSApplicationPresentationOptions)window:(NSWindow *)window
 - (void)windowWillEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillEnterFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillEnterFullScreen];
 }
 - (void)windowWillEnterFullScreen /* provided for direct calls */
@@ -7781,6 +7822,7 @@ - (void)windowDidEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidEnterFullScreen:]");
   [self windowDidEnterFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidEnterFullScreen /* provided for direct calls */
@@ -7819,6 +7861,7 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
 - (void)windowWillExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillExitFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillExitFullScreen];
 }
 
@@ -7838,6 +7881,7 @@ - (void)windowDidExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidExitFullScreen:]");
   [self windowDidExitFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidExitFullScreen /* provided for direct calls */
@@ -7867,6 +7911,21 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     [[self window] performZoom:self];
 }
 
+- (BOOL)isFullScreenTransition
+{
+  return is_fullscreen_transition;
+}
+
+- (void)waitFullScreenTransition
+{
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  while ([self isFullScreenTransition]){
+    NSTRACE ("wait for fullscreen");
+    wait_reading_process_output (0, 300000000, 0, 1, Qnil, NULL, 0);
+  }
+#endif
+}
+
 - (BOOL)fsIsNative
 {
   return fs_is_native;
@@ -7904,10 +7963,17 @@ - (void)updateCollectionBehavior
     {
       NSWindow *win = [self window];
       NSWindowCollectionBehavior b = [win collectionBehavior];
-      if (ns_use_native_fullscreen)
-        b |= NSWindowCollectionBehaviorFullScreenPrimary;
-      else
+      if (ns_use_native_fullscreen) {
+          if ([win parentWindow]) {
+            b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+            b |= NSWindowCollectionBehaviorFullScreenAuxiliary;
+          } else {
+            b |= NSWindowCollectionBehaviorFullScreenPrimary;
+            b &= ~NSWindowCollectionBehaviorFullScreenAuxiliary;
+          }
+      } else {
         b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+      }
 
       [win setCollectionBehavior: b];
 #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;
     }