MacViews: Bubbles and dialogs behave more like sheets wrt main status

ui/views/widget/native_widget_mac_interactive_uitest.mm

 // Copyright 2014 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.
 
 #include "ui/views/widget/native_widget_mac.h"
 
 #import <Cocoa/Cocoa.h>
 
+#import "base/mac/mac_util.h"
 #import "base/mac/scoped_nsobject.h"
 #include "base/macros.h"
 #import "ui/base/test/windowed_nsnotification_observer.h"
+#include "ui/views/bubble/bubble_delegate.h"
 #include "ui/views/test/test_widget_observer.h"
 #include "ui/views/test/widget_test.h"
 
 namespace views {
 namespace test {
 
 // Tests for NativeWidgetMac that rely on global window manager state, and can
 // not be parallelized.
 class NativeWidgetMacInteractiveUITest
     : public WidgetTest,
@@ skipping 104 matching lines @@
   // Activating the inactive widget should make it key, asynchronously.
   widget->Activate();
   [waiter wait];
   EXPECT_EQ(1, [waiter notificationCount]);
   EXPECT_TRUE(widget->IsActive());
   EXPECT_TRUE([widget->GetNativeWindow() isKeyWindow]);
 
   widget->CloseNow();
 }
 
+namespace {
+
+// Show |widget| and wait for it to become the key window.
+void ShowKeyWindow(Widget* widget) {
+  base::scoped_nsobject<WindowedNSNotificationObserver> waiter(
+      [[WindowedNSNotificationObserver alloc]
+          initForNotification:NSWindowDidBecomeKeyNotification
+                       object:widget->GetNativeWindow()]);
+  widget->Show();
+  EXPECT_TRUE([waiter wait]);
+  EXPECT_TRUE([widget->GetNativeWindow() isKeyWindow]);
+}
+
+NSData* ViewAsTIFF(NSView* view) {
+  NSBitmapImageRep* bitmap =
+      [view bitmapImageRepForCachingDisplayInRect:[view bounds]];
+  [view cacheDisplayInRect:[view bounds] toBitmapImageRep:bitmap];
+  return [bitmap TIFFRepresentation];
+}
+}

[Robert Sesek, 2016/01/21 23:41:35]

"  // namespace" and a line before

[tapted, 2016/01/22 00:28:06]

Done.

+
+// Test that parent windows keep their traffic lights enabled when showing
+// dialogs.
+TEST_F(NativeWidgetMacInteractiveUITest, ParentWindowTrafficLights) {
+  // Snow leopard doesn't have -[NSWindow _sharesParentKeyState].
+  if (base::mac::IsOSSnowLeopard())
+    return;
+
+  Widget* parent_widget = CreateTopLevelPlatformWidget();
+  parent_widget->SetBounds(gfx::Rect(100, 100, 100, 100));
+  ShowKeyWindow(parent_widget);
+
+  NSWindow* parent = parent_widget->GetNativeWindow();
+  EXPECT_TRUE([parent isMainWindow]);
+
+  NSButton* button = [parent standardWindowButton:NSWindowCloseButton];
+  EXPECT_TRUE(button);
+  NSData* active_button_image = ViewAsTIFF(button);
+  EXPECT_TRUE(active_button_image);
+
+  // Create an activatable frameless child. Frameless so that it doesn't have
+  // traffic lights of its own, and activatable so that it can take key status.
+  Widget* child_widget = new Widget;
+  Widget::InitParams params(Widget::InitParams::TYPE_WINDOW_FRAMELESS);
+  params.native_widget = new NativeWidgetMac(child_widget);
+  params.bounds = gfx::Rect(130, 130, 100, 100);
+  params.parent = parent_widget->GetNativeView();
+  child_widget->Init(params);
+  ShowKeyWindow(child_widget);
+
+  // Ensure the button instance is still valid.
+  EXPECT_EQ(button, [parent standardWindowButton:NSWindowCloseButton]);
+
+  // Parent window should still be main, and have its traffic lights active.
+  EXPECT_TRUE([parent isMainWindow]);

[tapted, 2016/01/22 00:28:06]

yegads AppKit. So with the bridge()->parent() => [self parentWindow] change,
this line now fails. But if you spin a run loop just after this, `parent`
clearly is the main window.

So, while the parentWindow approach works fine in practice, there may be a trap
lurking. And I'm still scarred by the hangouts multiple-key-status bug that this
makes me worry a little.

I think the problem is that AppKit's parent/child relationships can only be
relied upon while the window is visible (calling addChildWindow on a hidden
window has always led to strife). So, the first time a window becomes visible,
it temporarily reports `yes` in canBecomeMain, then when the window server gets
around to actually activating it, it reports `no`, and doesn't actually become
main. Yet, this isn't properly reflected `isMainWindow`.

Need to think some more about the right way to fix this.

+  EXPECT_FALSE([parent isKeyWindow]);
+
+  // Enabled status doesn't actually change, but check anyway.
+  EXPECT_TRUE([button isEnabled]);
+  NSData* button_image_with_child = ViewAsTIFF(button);
+  EXPECT_TRUE([active_button_image isEqualToData:button_image_with_child]);

[tapted, 2016/01/21 23:02:27]

oops#2 - I meant to comment on this too that this all seems a bit nuts, but I
couldn't figure out any NSButton[Cell] property I could query that gave the same
information -- it all seems to be internal to the particular NSButtonCell used
for the traffic lights. But maybe you have an idea?

I think test coverage of the private API usage is important, and this isn't any
scarier that what we do for checking the `showsWindowTitle` private API
overrides we use elsewhere.

+
+  // Verify that activating some other random window does change the button.
+  Widget* other_widget = CreateTopLevelPlatformWidget();
+  other_widget->SetBounds(gfx::Rect(200, 200, 100, 100));
+  ShowKeyWindow(other_widget);
+  EXPECT_FALSE([parent isMainWindow]);
+  EXPECT_FALSE([parent isKeyWindow]);
+  EXPECT_TRUE([button isEnabled]);
+  NSData* inactive_button_image = ViewAsTIFF(button);
+  EXPECT_FALSE([active_button_image isEqualToData:inactive_button_image]);
+
+  other_widget->CloseNow();
+  parent_widget->CloseNow();
+}
+
 INSTANTIATE_TEST_CASE_P(NativeWidgetMacInteractiveUITestInstance,
                         NativeWidgetMacInteractiveUITest,
                         ::testing::Bool());
 
 }  // namespace test
 }  // namespace views