MacViews: Protect against orderOut: on an NSWindow with an attached sheet.

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"
@@ skipping 84 matching lines @@
 }
 
 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])
     return;
 
   if (child_->window_visible()) {
-    DCHECK([child_->ns_window() parentWindow]);
+    // A sheet should never have a parentWindow, otherwise dismissing the sheet
+    // causes graphical glitches (http://crbug.com/605098). Non-sheets should
+    // always have a parentWindow.
+    DCHECK([child_->ns_window() isSheet] !=

[karandeepb, 2016/11/28 05:19:32]

So this was/is slightly confusing. Is this correct:

For sheet BNW, the BNW->parent()->ns_window() refers to the NSWindow the sheet
is attached to. But the actual parentWindow property ([[BNW->ns_window()]
parentWindow]) for a sheet BNW should be nil?

For non-sheet windows, [BNW->ns_window() parentWindow]  ==
BNW->parent()->ns_window().

Using the same parent_ terminology for both cases is slightly confusing. Maybe
this can be clarified somewhere in bridged_native_widget.h.

[tapted, 2016/11/28 06:15:41]

yes. ([NSWindow parentWindow] should be nil -- but sheets also have [NSWindow
sheetParent])
yes (so long as the windows are visible)
I think the complexity is Cocoa's fault. The nicest thing we can do is probably
isolate the BridgedNativeWidget API from anything to do with NSWindow
parentWindow/sheetParent, and just manage write defensive code under the hood.

(also it's possible we'll drop sheets completely with the modality proposal, and
just use tab-modals that don't use sheets at all, but we need to keep this for a
bit longer so we aren't changing too much UX at once).

[karandeepb, 2016/11/28 23:37:57]

Acknowledged.

+           !![child_->ns_window() parentWindow]);
     DCHECK(child_->wants_to_be_visible());
     return;
   }
 
   // The parent relationship should have been removed when the child was hidden.
   DCHECK(![child_->ns_window() parentWindow]);
   if (!child_->wants_to_be_visible())
     return;
 
   [child_->ns_window() orderWindow:NSWindowAbove
                         relativeTo:[anchor_window_ windowNumber]];
 
-  // Ordering the window should add back the relationship.
-  DCHECK([child_->ns_window() parentWindow]);
+  // Ordering the window should add back the relationship (unless it's a sheet).
+  DCHECK([child_->ns_window() isSheet] != !![child_->ns_window() parentWindow]);
   DCHECK(child_->window_visible());
 }
 
 NSWindow* WidgetOwnerNSWindowAdapter::GetNSWindow() {
   return anchor_window_;
 }
 
 gfx::Vector2d WidgetOwnerNSWindowAdapter::GetChildWindowOffset() const {
   NSRect rect_in_window =
       [anchor_view_ convertRect:[anchor_view_ bounds] toView:nil];
@@ skipping 13 matching lines @@
   DCHECK_EQ(child, child_);
   [GetNSWindow() removeChildWindow:child->ns_window()];
   delete this;
 }
 
 WidgetOwnerNSWindowAdapter::~WidgetOwnerNSWindowAdapter() {
   [[NSNotificationCenter defaultCenter] removeObserver:observer_bridge_];
 }
 
 }  // namespace views