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: 1437 bytes --]


> On Jul 16, 2019, at 22:28, Alan Third <alan@idiocy.org> 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.

Removing parent-frame property causes child frame to enter fullscreen
if some of parent frames is in fullscreen.

Setting parent-frame property to fullscreen window causes window to
leave fullscreen.

As a side effect (sleep-for 0.5) in toggle-frame-fullscreen is no
longer needed (bug#28496).

But I need help with some questions:

1. Is it the rigth way to wait for some NS event by calling
   wait_reading_process_output or is there a better way?

2. I commented some code in windowDidMove with FIXME comment.
   After windowDidMove event is triggered, windowDidExitFullScreen is
   not received and fullscreen transition is never "completed".
   With that code commented windowDidExitFullScreen is received.
   Is this code somehow breaks the events queue?

Actually it is the first time I'm working with the C side of Emacs.
And with AppKit. And with C :) Please let me know if my decisions
and conclusions are wrong.

Thanks!

[-- Attachment #2: ns-emacs-spaces.patch --]
[-- Type: application/octet-stream, Size: 9341 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index fd7e872fb6..325b4c07c5 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2629,11 +2629,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 9773eb3e66..9391e31b48 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;
 @public
    struct frame *emacsframe;
    int rows, cols;
@@ -448,6 +449,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
@@ -1274,6 +1277,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 02331826d9..df6a0aab80 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1495,9 +1495,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)
         {
@@ -1882,19 +1884,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
@@ -7336,6 +7371,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;
@@ -7365,7 +7401,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];
@@ -7496,11 +7535,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);
+      //   }
     }
 }
 
@@ -7700,6 +7740,7 @@ - (NSApplicationPresentationOptions)window:(NSWindow *)window
 - (void)windowWillEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillEnterFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillEnterFullScreen];
 }
 - (void)windowWillEnterFullScreen /* provided for direct calls */
@@ -7712,6 +7753,7 @@ - (void)windowDidEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidEnterFullScreen:]");
   [self windowDidEnterFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidEnterFullScreen /* provided for direct calls */
@@ -7750,6 +7792,7 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
 - (void)windowWillExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillExitFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillExitFullScreen];
 }
 
@@ -7769,6 +7812,7 @@ - (void)windowDidExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidExitFullScreen:]");
   [self windowDidExitFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidExitFullScreen /* provided for direct calls */
@@ -7798,6 +7842,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;
@@ -7835,10 +7894,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
@@ -7863,9 +7929,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;
     }