Implement NativeWidgetMac::ReorderNativeViews

ui/views/widget/native_widget_mac_unittest.mm

Index: ui/views/widget/native_widget_mac_unittest.mm
diff --git a/ui/views/widget/native_widget_mac_unittest.mm b/ui/views/widget/native_widget_mac_unittest.mm
index e62394eeb9495cae774ef8a765be0f63bfeb9679..01b974787f005e5aa42bbaed2f9f1176ba92fba4 100644
--- a/ui/views/widget/native_widget_mac_unittest.mm
+++ b/ui/views/widget/native_widget_mac_unittest.mm
@@ -27,6 +27,7 @@
 #import "ui/views/cocoa/native_widget_mac_nswindow.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/label.h"
+#include "ui/views/controls/native/native_view_host.h"
 #include "ui/views/native_cursor.h"
 #include "ui/views/test/native_widget_factory.h"
 #include "ui/views/test/test_widget_observer.h"
@@ -211,6 +212,30 @@ class WidgetChangeObserver : public TestWidgetObserver {
   DISALLOW_COPY_AND_ASSIGN(WidgetChangeObserver);
 };
 
+class NativeHostHolder {
+ public:
+  NativeHostHolder()
+      : view_([[NSView alloc] init]), host_(new NativeViewHost()) {
+    host_->set_owned_by_client();

[tapted, 2016/03/17 23:04:15]

oh neat - I forgot you could do this

+  }
+
+  void AttachNativeView() {
+    DCHECK(!host_->native_view());
+    host_->Attach(view_.get());
+  }
+
+  void Detach() { host_->Detach(); }
+
+  gfx::NativeView view() const { return view_.get(); }
+  NativeViewHost* host() const { return host_.get(); }
+
+ private:
+  base::scoped_nsobject<NSView> view_;
+  scoped_ptr<NativeViewHost> host_;
+
+  DISALLOW_COPY_AND_ASSIGN(NativeHostHolder);
+};
+
 // Test visibility states triggered externally.
 TEST_F(NativeWidgetMacTest, HideAndShowExternally) {
   Widget* widget = CreateTopLevelPlatformWidget();
@@ -1336,6 +1361,117 @@ TEST_F(NativeWidgetMacTest, ChangeFocusOnChangeFirstResponder) {
   widget->CloseNow();
 }
 
+class NativeWidgetMacViewsOrderTest : public WidgetTest {
+ public:
+  NativeWidgetMacViewsOrderTest() {}
+
+ protected:
+  void SetUp() override {

[tapted, 2016/03/17 23:04:15]

nit:

// testing::Test:

+    WidgetTest::SetUp();
+
+    widget_ = CreateTopLevelPlatformWidget();
+
+    ASSERT_EQ(1u, [[widget_->GetNativeView() subviews] count]);
+    compositor_view_ = [[widget_->GetNativeView() subviews] firstObject];
+
+    native_host_parent_ = new View();
+    widget_->GetContentsView()->AddChildView(native_host_parent_);
+
+    const int kNativeViewCount = 3;
+    for (int i = 0; i < kNativeViewCount; ++i) {
+      scoped_ptr<NativeHostHolder> holder(new NativeHostHolder());
+      native_host_parent_->AddChildView(holder->host());
+      holder->AttachNativeView();
+      hosts_.push_back(std::move(holder));
+    }
+    EXPECT_EQ(native_host_parent_->child_count(), kNativeViewCount);

[tapted, 2016/03/17 23:04:15]

nit: EXPECT_EQ(kNativeViewCount, native_host_parent_->child_count());

(putting the "more constant" value on the left makes the logging more sensible)

+    EXPECT_TRUE(([[widget_->GetNativeView() subviews] isEqualToArray:@[
+      compositor_view_, hosts_[0]->view(), hosts_[1]->view(), hosts_[2]->view()
+    ]]));
+  }
+
+  void TearDown() override {
+    widget_->CloseNow();
+    WidgetTest::TearDown();
+  }
+
+  NSView* GetContentNativeView() { return widget_->GetNativeView(); }
+  NSView* compositor_view() { return compositor_view_; }
+  View* native_host_parent() { return native_host_parent_; }
+  const std::vector<scoped_ptr<NativeHostHolder>>& hosts() const {
+    return hosts_;
+  }
+
+ private:

[tapted, 2016/03/17 23:04:15]

It's actually fine to have protected members in test harnesses (there's an
exception in the style guide -- just ensure the DISALLOW is still private). This
would allow you do do hosts_[i]->view, which feels a lot neater than
hosts()[i]->view

+  Widget* widget_ = nullptr;
+  View* native_host_parent_ = nullptr;
+  NSView* compositor_view_ = nil;
+  std::vector<scoped_ptr<NativeHostHolder>> hosts_;
+
+  DISALLOW_COPY_AND_ASSIGN(NativeWidgetMacViewsOrderTest);
+};
+
+// Test that NativeViewHost::Attach()/Detach() method saves the NativeView
+// z-order.
+TEST_F(NativeWidgetMacViewsOrderTest, NativeViewAttached) {
+  hosts()[1]->Detach();
+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[0]->view(), hosts()[2]->view()
+  ]]));
+
+  hosts()[1]->AttachNativeView();
+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[0]->view(), hosts()[1]->view(),
+    hosts()[2]->view()
+  ]]));
+}
+
+// Tests that NativeViews order changes according to views::View hierarchy.
+TEST_F(NativeWidgetMacViewsOrderTest, ReorderViews) {
+  native_host_parent()->ReorderChildView(hosts()[2]->host(), 1);
+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[0]->view(), hosts()[2]->view(),
+    hosts()[1]->view()
+  ]]));
+
+  native_host_parent()->RemoveChildView(hosts()[2]->host());
+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[0]->view(), hosts()[1]->view()
+  ]]));
+
+  View* new_parent = new View();
+  native_host_parent()->RemoveChildView(hosts()[1]->host());
+  native_host_parent()->AddChildView(new_parent);
+  new_parent->AddChildView(hosts()[1]->host());
+  new_parent->AddChildView(hosts()[2]->host());
+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[0]->view(), hosts()[1]->view(),
+    hosts()[2]->view()
+  ]]));
+
+  native_host_parent()->ReorderChildView(new_parent, 0);
+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[1]->view(), hosts()[2]->view(),
+    hosts()[0]->view()
+  ]]));
+}
+
+// Test that unassociated native views stay on top after reordering.
+TEST_F(NativeWidgetMacViewsOrderTest, UnassociatedViewsIsAbove) {
+  base::scoped_nsobject<NSView> child_view([[NSView alloc] init]);
+  [GetContentNativeView() addSubview:child_view.get()];

[tapted, 2016/03/17 23:04:15]

nit: no .get()

+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[0]->view(), hosts()[1]->view(),
+    hosts()[2]->view(), child_view.get()
+  ]]));
+
+  native_host_parent()->ReorderChildView(hosts()[2]->host(), 1);
+  EXPECT_TRUE(([[GetContentNativeView() subviews] isEqualToArray:@[
+    compositor_view(), hosts()[0]->view(), hosts()[2]->view(),
+    hosts()[1]->view(), child_view.get()
+  ]]));
+}
+
 }  // namespace test
 }  // namespace views