Add child for parent_mus when params.parent_mus is not initialized.

ui/views/mus/native_widget_mus_unittest.cc

Index: ui/views/mus/native_widget_mus_unittest.cc
diff --git a/ui/views/mus/native_widget_mus_unittest.cc b/ui/views/mus/native_widget_mus_unittest.cc
index 3faa57a2ae4a2346ee7604c6711886f3645d8c16..bcf9be950d99394f437c39ee6a34178f715a7c5e 100644
--- a/ui/views/mus/native_widget_mus_unittest.cc
+++ b/ui/views/mus/native_widget_mus_unittest.cc
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ui/views/mus/native_widget_mus.h"

[mfomitchev, 2016/11/09 17:44:20]

The unit test is for NativeWidgetMus, so this include should stay at the top.

 
 #include "base/callback.h"
 #include "base/macros.h"
@@ -17,6 +16,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/aura/client/aura_constants.h"

[mfomitchev, 2016/11/09 17:44:20]

Is this used?

[thanhph, 2016/11/09 19:11:37]

I removed this include with the function CreateTestWindow below. Thanks!

 #include "ui/aura/window.h"
 #include "ui/events/event.h"
 #include "ui/events/test/test_event_handler.h"
@@ -25,6 +25,7 @@
 #include "ui/gfx/path.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/views/controls/native/native_view_host.h"
+#include "ui/views/mus/native_widget_mus.h"
 #include "ui/views/mus/window_manager_connection.h"
 #include "ui/views/test/focus_manager_test.h"
 #include "ui/views/test/views_test_base.h"
@@ -161,6 +162,14 @@ class NativeWidgetMusTest : public ViewsTestBase {
     return widget;
   }
 
+  aura::Window* CreateTestWindow() {

[mfomitchev, 2016/11/09 17:44:20]

This isn't actually used it seems.

[thanhph, 2016/11/09 19:11:37]

Acknowledged.

+    aura::Window* window = new aura::Window(nullptr);
+    window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
+    window->SetType(ui::wm::WINDOW_TYPE_NORMAL);
+    window->Init(ui::LAYER_TEXTURED);
+    return window;
+  }
+
   int ack_callback_count() { return ack_callback_count_; }
 
   void AckCallback(ui::mojom::EventResult result) {
@@ -609,4 +618,54 @@ TEST_F(NativeWidgetMusTest, IsMaximized) {
   EXPECT_TRUE(widget->IsMaximized());
 }
 
+// This test is to ensure corresponding parentship relation between widget and

[mfomitchev, 2016/11/09 17:44:20]

This isn't quite accurate. Strictly speaking, widgets themselves don't have
parent/child relationship. There's parent/child relationship between
aura::Windows and ui::Windows, both of which are associated with Widgets. The
parentship relationship that we are ensuring here is not between Widgets, but
between ui::Windows corresponding to the Widgets. It is established based on the
parent/child relationship between aura::Windows, also associated with these
Widgets.

A description like this would be more accurate:

Ensure that when initializing a Widget with InitParams.parent set to another
widget's aura::Window, the ui::Window of the former Widget is added as a child
to the ui::Window of the latter Widget.

[thanhph, 2016/11/09 19:11:37]

Acknowledged. Thanks for the clarification!

+// window. If widget A is child of Widget B, then window that contains widget A
+// is also a child of window that contains widget B.
+TEST_F(NativeWidgetMusTest, WindowOfChildWidgetIsChildOfParentWindowWidget) {

[mfomitchev, 2016/11/09 17:44:20]

Maybe just call the test something like InitNativeWidgetParentsUiWindow to make
it more parsable?

[thanhph, 2016/11/09 19:11:37]

Acknowledged.

+  ASSERT_TRUE(WindowManagerConnection::Exists());
+
+  ui::Window* parent_window = WindowManagerConnection::Get()->NewTopLevelWindow(
+      std::map<std::string, std::vector<uint8_t>>());
+  std::unique_ptr<Widget> parent_widget(new Widget());
+  Widget::InitParams parent_params =
+      CreateParams(Widget::InitParams::TYPE_WINDOW);
+  parent_params.name = "Parent Widget";
+  parent_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  parent_params.shadow_type = Widget::InitParams::SHADOW_TYPE_NONE;
+  parent_params.opacity = Widget::InitParams::OPAQUE_WINDOW;
+  parent_params.parent = nullptr;
+  parent_params.bounds = initial_bounds();
+  parent_params.native_widget =
+      new NativeWidgetMus(parent_widget.get(), parent_window,
+                          ui::mojom::CompositorFrameSinkType::DEFAULT);
+  parent_widget->Init(parent_params);
+
+  std::unique_ptr<Widget> child_widget(new Widget());
+  ui::Window* child_window = parent_window->window_tree()->NewWindow();
+  Widget::InitParams child_params = CreateParams(Widget::InitParams::TYPE_MENU);
+  child_params.parent = parent_widget->GetNativeView();
+  child_params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  child_params.name = "Child Widget";
+  child_params.native_widget =
+      new NativeWidgetMus(child_widget.get(), child_window,
+                          ui::mojom::CompositorFrameSinkType::DEFAULT);
+  child_widget->Init(child_params);
+
+  EXPECT_EQ(child_window->parent(), parent_window);
+
+  std::unique_ptr<Widget> not_child_widget(new Widget());
+  ui::Window* not_child_window = parent_window->window_tree()->NewWindow();
+  Widget::InitParams not_child_params =
+      CreateParams(Widget::InitParams::TYPE_MENU);
+  not_child_params.ownership =
+      views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  not_child_params.name = "Not Child Widget";
+  not_child_params.native_widget =
+      new NativeWidgetMus(not_child_widget.get(), not_child_window,
+                          ui::mojom::CompositorFrameSinkType::DEFAULT);
+  not_child_widget->Init(not_child_params);
+
+  EXPECT_NE(not_child_window->parent(), parent_window);
+}
+
 }  // namespace views