MacViews: Reattach children of native windows when they are exposed.

ui/views/cocoa/widget_owner_nswindow_adapter.mm

 // Copyright 2015 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/widget_owner_nswindow_adapter.h"
 
 #import <Cocoa/Cocoa.h>
 
 #include "base/logging.h"
 #include "base/mac/sdk_forward_declarations.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/vector2d.h"
 #import "ui/gfx/mac/coordinate_conversion.h"
 #import "ui/views/cocoa/bridged_native_widget.h"
 
 // Bridges an AppKit observer to observe when the (non-views) NSWindow owning a
 // views::Widget will close.

[karandeepb, 2016/10/27 03:21:17]

Modify this comment. It's also observing changes to the occlusion state now.

[tapted, 2016/10/27 04:06:48]

Done.

 @interface WidgetOwnerNSWindowAdapterBridge : NSObject {
  @private
   views::WidgetOwnerNSWindowAdapter* adapter_;  // Weak. Owns us.
 }
 - (instancetype)initWithAdapter:(views::WidgetOwnerNSWindowAdapter*)adapter;
 - (void)windowWillClose:(NSNotification*)notification;
+- (void)windowDidChangeOcclusionState:(NSNotification*)notification;
 @end
 
 @implementation WidgetOwnerNSWindowAdapterBridge
 
 - (instancetype)initWithAdapter:(views::WidgetOwnerNSWindowAdapter*)adapter {
   if ((self = [super init]))
     adapter_ = adapter;
   return self;
 }
 
 - (void)windowWillClose:(NSNotification*)notification {
   adapter_->OnWindowWillClose();
 }
 
+- (void)windowDidChangeOcclusionState:(NSNotification*)notification {
+  adapter_->OnWindowDidChangeOcclusionState();
+}
+
 @end
 
 namespace views {
 
 WidgetOwnerNSWindowAdapter::WidgetOwnerNSWindowAdapter(
     BridgedNativeWidget* child,
     NSView* anchor_view)
     : child_(child),
       anchor_view_([anchor_view retain]),
       observer_bridge_(
           [[WidgetOwnerNSWindowAdapterBridge alloc] initWithAdapter:this]) {
 
   // Although the |anchor_view| must be in an NSWindow when the child dialog is
   // created, it's permitted for the |anchor_view| to be removed from its view
   // hierarchy before the child dialog window is fully removed from screen. When
   // this happens, [anchor_view_ window] will become nil, so retain both.
   anchor_window_.reset([[anchor_view_ window] retain]);
   DCHECK(anchor_window_);
 
   [[NSNotificationCenter defaultCenter]
       addObserver:observer_bridge_
          selector:@selector(windowWillClose:)
              name:NSWindowWillCloseNotification
            object:anchor_window_];
+
+  [[NSNotificationCenter defaultCenter]

[karandeepb, 2016/10/27 03:21:17]

Maybe add a comment explaining why we need to observe changes to the occlusion
state.

[tapted, 2016/10/27 04:06:48]

Done.

+      addObserver:observer_bridge_
+         selector:@selector(windowDidChangeOcclusionState:)
+             name:NSWindowDidChangeOcclusionStateNotification
+           object:anchor_window_];
 }
 
 void WidgetOwnerNSWindowAdapter::OnWindowWillClose() {
   // Retain the child window before closing it. If the last reference to the
   // NSWindow goes away inside -[NSWindow close], then bad stuff can happen.
   // See e.g. http://crbug.com/616701.
   base::scoped_nsobject<NSWindow> child_window(child_->ns_window(),
                                                base::scoped_policy::RETAIN);
 
   // AppKit child window relationships break when the windows are not visible,
   // so if the child is not visible, it won't currently be a child.
   DCHECK(![child_window isVisible] || [child_window parentWindow]);
   DCHECK([child_window delegate]);
 
   [child_window close];
   // Note: |this| will be deleted here.
 
   DCHECK(![child_window parentWindow]);
   DCHECK(![child_window delegate]);
 }
 
+void WidgetOwnerNSWindowAdapter::OnWindowDidChangeOcclusionState() {
+  // The adapter only needs to handle a parent "show", since the only way it
+  // should be hidden is via -[NSApp hide], and all BridgedNativeWidgets
+  // subscribe to NSApplicationDidHideNotification already.
+  if (![anchor_window_ isVisible])

[karandeepb, 2016/10/27 03:21:17]

Just to clarify, the parent-child relationship breaks when the window is hidden.
It does not break when the window is not hidden but only occluded, right?

And what we want to do is to reattach the child when the parent window becomes
"visible" (as opposed to just being non-occluded)? Buy since there is no proper
visibility change notification, we are using occlusion state.

[tapted, 2016/10/27 04:06:48]

Yup. AppKit fails to update zorder/position for child windows after an orderOut,
so we intentionally break the relationship in those cases. But it's fine just
for occlusion.
Yep - exactly. There is a WindowDidExpose notification but it doesn't work.
Visibility changes on Mac are a pain, but occlusions works pretty well. It came
in 10.9, so lucky we don't support further back than that :)

+    return;
+
+  if (child_->window_visible()) {
+    DCHECK(child_->wants_to_be_visible());

[karandeepb, 2016/10/27 03:21:17]

Should we DCHECK([child->ns_window() parentWindow]);

[tapted, 2016/10/27 04:06:48]

Done.

+    return;
+  }
+
+  if (!child_->wants_to_be_visible())
+    return;
+

[karandeepb, 2016/10/27 03:21:17]

So now the parent is visible, and the child is hidden but wants to be visible
(it is not visible because it had a hidden parent). Can we add a DCHECK here
that the occlusion state of the anchor window has changed to
NSWindowOcclusionStateVisible?

[tapted, 2016/10/27 04:06:48]

I don't think this is the same. The parent window can be "visible", but fully
occluded -- we don't hide child widgets in that situation, so we shouldn't block
re-showing them either. And since occlusion is generated by the window server,
there's possible race conditions that could mean a window could be
fully-occluded the first time it becomes "visible".

+  // The parent relationship should have been removed when the child was hidden.
+  DCHECK(![child_->ns_window() parentWindow]);

[karandeepb, 2016/10/27 03:21:17]

Can this DCHECK move before the preceding if condition?

[tapted, 2016/10/27 04:06:48]

Done.

+  [child_->ns_window() orderWindow:NSWindowAbove
+                        relativeTo:[anchor_window_ windowNumber]];
+
+  // Ordering the window should add back the relationship.
+  DCHECK([child_->ns_window() parentWindow]);

[karandeepb, 2016/10/27 03:21:17]

DCHECK(child_->window_visible())?

[tapted, 2016/10/27 04:06:48]

Done.

+}
+
 NSWindow* WidgetOwnerNSWindowAdapter::GetNSWindow() {
   return anchor_window_;
 }
 
 gfx::Vector2d WidgetOwnerNSWindowAdapter::GetChildWindowOffset() const {
   NSRect rect_in_window =
       [anchor_view_ convertRect:[anchor_view_ bounds] toView:nil];
   NSRect rect_in_screen = [anchor_window_ convertRectToScreen:rect_in_window];
   // Ensure we anchor off the top-left of |anchor_view_| (rect_in_screen.origin
   // is the bottom-left of the view).
@@ skipping 10 matching lines @@
   DCHECK_EQ(child, child_);
   [GetNSWindow() removeChildWindow:child->ns_window()];
   delete this;
 }
 
 WidgetOwnerNSWindowAdapter::~WidgetOwnerNSWindowAdapter() {
   [[NSNotificationCenter defaultCenter] removeObserver:observer_bridge_];
 }
 
 }  // namespace views