MacViews: Fix WidgetTest.DesktopNativeWidgetNoPaintAfterCloseTest

But.. first make some tests fail.

DesktopNativeWidgetNoPaintAfterCloseTest currently passes but it
shouldn't. WidgetTests DesktopNativeWidgetNoPaintAfterHideTest and
TestWindowVisibilityAfterHide were also passing, but
NativeWidgetMac::Hide() is a no-op, so that shouldn't be the case.

This CL (first) makes them all fail on Mac. To do this,
BridgedNativeWidget needs to call Widget::OnNativeWidgetPaint() rather
than painting the RootView directly. And TestWindowVisibilityAfterHide
must show the widget.

DesktopNativeWidgetNoPaintAfterCloseTest was then fixed by skipping a
paint when the window is not visible, as toolkit-views expects. However,
Cocoa paints the window before changing -[NSWindow isVisible]. To fix,
intercept -[NSWindow orderWindow:relativeTo:]. We need to do this anyway
to send Widget::OnNativeWidgetVisibilityChanged(), but that's left for a
follow-up.

BUG=378134

Review URL: https://codereview.chromium.org/654393002

Cr-Commit-Position: refs/heads/master@{#300056}

ui/views/cocoa/bridged_content_view.h

@@ -31,10 +31,16 @@ class View;
 
   // A tracking area installed to enable mouseMoved events.
   ui::ScopedCrTrackingArea trackingArea_;
+
+  // Set to ignore window visibility in a subsequent call to drawRect:. Views
+  // does not expect hidden windows to paint. However, when showing a window,
+  // Cocoa first paints before updating visibility.
+  BOOL willShow_;
 }
 
 @property(readonly, nonatomic) views::View* hostedView;
 @property(assign, nonatomic) ui::TextInputClient* textInputClient;
+@property(assign, nonatomic) BOOL willShow;
 
 // Initialize the NSView -> views::View bridge. |viewToHost| must be non-NULL.
 - (id)initWithView:(views::View*)viewToHost;

ui/views/cocoa/bridged_content_view.mm

@@ -30,6 +30,7 @@
 
 @synthesize hostedView = hostedView_;
 @synthesize textInputClient = textInputClient_;
+@synthesize willShow = willShow_;
 
 - (id)initWithView:(views::View*)viewToHost {
   DCHECK(viewToHost);
@@ -89,11 +90,13 @@
 }
 
 - (void)drawRect:(NSRect)dirtyRect {
-  if (!hostedView_)
+  // Note that on a Show, Cocoa calls drawRect: before changing
+  // -[NSWindow isVisible], hence the extra check.
+  if (!hostedView_ || (!willShow_ && ![[self window] isVisible]))
     return;
 
   gfx::CanvasSkiaPaint canvas(dirtyRect, false /* opaque */);
-  hostedView_->Paint(&canvas, views::CullSet());
+  hostedView_->GetWidget()->OnNativeWidgetPaint(&canvas);
 }
 
 // NSResponder implementation.

ui/views/cocoa/bridged_native_widget_unittest.mm

@@ -20,6 +20,7 @@
 #include "ui/views/ime/input_method.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/native_widget_mac.h"
+#include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
 
@@ -223,7 +224,6 @@ class BridgedNativeWidgetTest : public BridgedNativeWidgetTestBase {
   virtual void TearDown() override;
 
  protected:
-  // TODO(tapted): Make this a EventCountView from widget_unittest.cc.
   scoped_ptr<views::View> view_;
   scoped_ptr<BridgedNativeWidget> bridge_;
   BridgedContentView* ns_view_;  // Weak. Owned by bridge_.
@@ -255,7 +255,7 @@ std::string BridgedNativeWidgetTest::GetText() {
 void BridgedNativeWidgetTest::SetUp() {
   BridgedNativeWidgetTestBase::SetUp();
 
-  view_.reset(new views::View);
+  view_.reset(new views::internal::RootView(widget_.get()));
   base::scoped_nsobject<NSWindow> window([test_window() retain]);
 
   // BridgedNativeWidget expects to be initialized with a hidden (deferred)

ui/views/cocoa/views_nswindow_delegate.h

@@ -26,6 +26,16 @@ class BridgedNativeWidget;
 // Initialize with the given |parent|.
 - (id)initWithBridgedNativeWidget:(views::BridgedNativeWidget*)parent;
 
+// Notify that the window is about to be reordered on screen. This ensures a
+// paint will occur, even if Cocoa has not yet updated the window visibility.
+- (void)onWindowOrderWillChange:(NSWindowOrderingMode)orderingMode;
+
+// Notify that the window has been reordered in (or removed from) the window
+// server's screen list. This is a substitute for -[NSWindowDelegate
+// windowDidExpose:], which is only sent for nonretained windows (those without
+// a backing store).
+- (void)onWindowOrderChanged;
+
 @end
 
 #endif  // UI_VIEWS_COCOA_VIEWS_NSWINDOW_DELEGATE_H_

ui/views/cocoa/views_nswindow_delegate.mm

@@ -5,6 +5,7 @@
 #import "ui/views/cocoa/views_nswindow_delegate.h"
 
 #include "base/logging.h"
+#import "ui/views/cocoa/bridged_content_view.h"
 #import "ui/views/cocoa/bridged_native_widget.h"
 #include "ui/views/widget/native_widget_mac.h"
 
@@ -22,6 +23,15 @@
   return parent_->native_widget_mac();
 }
 
+- (void)onWindowOrderWillChange:(NSWindowOrderingMode)orderingMode {
+  if (orderingMode != NSWindowOut)
+    [parent_->ns_view() setWillShow:YES];
+}
+
+- (void)onWindowOrderChanged {
+  [parent_->ns_view() setWillShow:NO];
+}
+
 // NSWindowDelegate implementation.
 
 - (void)windowDidFailToEnterFullScreen:(NSWindow*)window {

ui/views/focus/focus_manager.h

@@ -82,7 +82,6 @@ namespace views {
 
 class FocusManagerDelegate;
 class FocusSearch;
-class RootView;
 class View;
 class Widget;
 

ui/views/widget/native_widget_mac.mm

@@ -18,10 +18,15 @@
 #include "ui/views/window/native_frame_view.h"
 
 @interface NativeWidgetMacNSWindow : NSWindow
+- (ViewsNSWindowDelegate*)viewsNSWindowDelegate;
 @end
 
 @implementation NativeWidgetMacNSWindow
 
+- (ViewsNSWindowDelegate*)viewsNSWindowDelegate {
+  return base::mac::ObjCCastStrict<ViewsNSWindowDelegate>([self delegate]);
+}
+
 // Override canBecome{Key,Main}Window to always return YES, otherwise Windows
 // with a styleMask of NSBorderlessWindowMask default to NO.
 - (BOOL)canBecomeKeyWindow {
@@ -32,6 +37,15 @@
   return YES;
 }
 
+// Override orderWindow to intercept visibility changes, since there is no way
+// to observe these changes via NSWindowDelegate.
+- (void)orderWindow:(NSWindowOrderingMode)orderingMode
+         relativeTo:(NSInteger)otherWindowNumber {
+  [[self viewsNSWindowDelegate] onWindowOrderWillChange:orderingMode];
+  [super orderWindow:orderingMode relativeTo:otherWindowNumber];
+  [[self viewsNSWindowDelegate] onWindowOrderChanged];
+}
+
 @end
 
 namespace views {
@@ -301,6 +315,12 @@ void NativeWidgetMac::SetShape(gfx::NativeRegion shape) {
 }
 
 void NativeWidgetMac::Close() {
+  if (!bridge_)
+    return;
+
+  // Clear the view early to suppress repaints.
+  bridge_->SetRootView(NULL);
+
   NSWindow* window = GetNativeWindow();
   // Calling performClose: will momentarily highlight the close button, but
   // AppKit will reject it if there is no close button.

ui/views/widget/widget_unittest.cc

@@ -1125,12 +1125,13 @@ TEST_F(WidgetTest, TestViewWidthAfterMinimizingWidget) {
 class DesktopAuraTestValidPaintWidget : public views::Widget {
  public:
   DesktopAuraTestValidPaintWidget()
-    : expect_paint_(true),
-      received_paint_while_hidden_(false) {
-  }
+    : received_paint_(false),
+      expect_paint_(true),
+      received_paint_while_hidden_(false) {}
 
-  virtual ~DesktopAuraTestValidPaintWidget() {
-  }
+  virtual ~DesktopAuraTestValidPaintWidget() {}
+
+  void InitForTest(Widget::InitParams create_params);
 
   virtual void Show() override {
     expect_paint_ = true;
@@ -1148,58 +1149,66 @@ class DesktopAuraTestValidPaintWidget : public views::Widget {
   }
 
   virtual void OnNativeWidgetPaint(gfx::Canvas* canvas) override {
+    received_paint_ = true;
     EXPECT_TRUE(expect_paint_);
     if (!expect_paint_)
       received_paint_while_hidden_ = true;
     views::Widget::OnNativeWidgetPaint(canvas);
   }
 
+  bool ReadReceivedPaintAndReset() {
+    bool result = received_paint_;
+    received_paint_ = false;
+    return result;
+  }
+
   bool received_paint_while_hidden() const {
     return received_paint_while_hidden_;
   }
 
  private:
+  bool received_paint_;
   bool expect_paint_;
   bool received_paint_while_hidden_;
+
+  DISALLOW_COPY_AND_ASSIGN(DesktopAuraTestValidPaintWidget);
 };
 
-TEST_F(WidgetTest, DesktopNativeWidgetNoPaintAfterCloseTest) {
+void DesktopAuraTestValidPaintWidget::InitForTest(InitParams init_params) {
+  init_params.bounds = gfx::Rect(0, 0, 200, 200);
+  init_params.ownership = InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  init_params.native_widget = new PlatformDesktopNativeWidget(this);
+  Init(init_params);
+
   View* contents_view = new View;
   contents_view->SetFocusable(true);
+  SetContentsView(contents_view);
+
+  Show();
+  Activate();
+}
+
+TEST_F(WidgetTest, DesktopNativeWidgetNoPaintAfterCloseTest) {
   DesktopAuraTestValidPaintWidget widget;
-  Widget::InitParams init_params =
-      CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS);
-  init_params.bounds = gfx::Rect(0, 0, 200, 200);
-  init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
-  init_params.native_widget = new PlatformDesktopNativeWidget(&widget);
-  widget.Init(init_params);
-  widget.SetContentsView(contents_view);
-  widget.Show();
-  widget.Activate();
+  widget.InitForTest(CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS));
   RunPendingMessages();
-  widget.SchedulePaintInRect(init_params.bounds);
+  EXPECT_TRUE(widget.ReadReceivedPaintAndReset());
+  widget.SchedulePaintInRect(widget.GetRestoredBounds());
   widget.Close();
   RunPendingMessages();
+  EXPECT_FALSE(widget.ReadReceivedPaintAndReset());
   EXPECT_FALSE(widget.received_paint_while_hidden());
 }
 
 TEST_F(WidgetTest, DesktopNativeWidgetNoPaintAfterHideTest) {
-  View* contents_view = new View;
-  contents_view->SetFocusable(true);
   DesktopAuraTestValidPaintWidget widget;
-  Widget::InitParams init_params =
-      CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS);
-  init_params.bounds = gfx::Rect(0, 0, 200, 200);
-  init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
-  init_params.native_widget = new PlatformDesktopNativeWidget(&widget);
-  widget.Init(init_params);
-  widget.SetContentsView(contents_view);
-  widget.Show();
-  widget.Activate();
+  widget.InitForTest(CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS));
   RunPendingMessages();
-  widget.SchedulePaintInRect(init_params.bounds);
+  EXPECT_TRUE(widget.ReadReceivedPaintAndReset());
+  widget.SchedulePaintInRect(widget.GetRestoredBounds());
   widget.Hide();
   RunPendingMessages();
+  EXPECT_FALSE(widget.ReadReceivedPaintAndReset());
   EXPECT_FALSE(widget.received_paint_while_hidden());
   widget.Close();
 }
@@ -1221,6 +1230,8 @@ TEST_F(WidgetTest, TestWindowVisibilityAfterHide) {
   NonClientFrameView* frame_view = new MinimumSizeFrameView(&widget);
   non_client_view->SetFrameView(frame_view);
 
+  widget.Show();
+  EXPECT_TRUE(IsNativeWindowVisible(widget.GetNativeWindow()));
   widget.Hide();
   EXPECT_FALSE(IsNativeWindowVisible(widget.GetNativeWindow()));
   widget.Show();