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

ui/views/mus/native_widget_mus_unittest.cc

 // Copyright 2016 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/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"
 #include "services/ui/public/cpp/property_type_converters.h"
 #include "services/ui/public/cpp/tests/window_tree_client_private.h"
 #include "services/ui/public/cpp/window.h"
 #include "services/ui/public/cpp/window_observer.h"
 #include "services/ui/public/cpp/window_property.h"
 #include "services/ui/public/cpp/window_tree_client.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "services/ui/public/interfaces/window_tree.mojom.h"
 #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"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/image_skia.h"
 #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"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/widget/widget_observer.h"
 #include "ui/wm/public/activation_client.h"
 
 using ui::mojom::EventResult;
 
@@ skipping 116 matching lines @@
   std::unique_ptr<Widget> CreateWidget(WidgetDelegate* delegate) {
     std::unique_ptr<Widget> widget(new Widget());
     Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
     params.delegate = delegate;
     params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
     params.bounds = initial_bounds();
     widget->Init(params);
     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) {
     ack_callback_count_++;
     EXPECT_EQ(ui::mojom::EventResult::HANDLED, result);
   }
 
   // Returns a mouse pressed event inside the widget. Tests that place views
   // within the widget that respond to the event must be constructed within the
   // widget coordinate space such that they respond correctly.
@@ skipping 428 matching lines @@
   params.bounds = initial_bounds();
   params.native_widget = new NativeWidgetMus(
       widget.get(), window, ui::mojom::CompositorFrameSinkType::DEFAULT);
   widget->Init(params);
   window->SetSharedProperty<int32_t>(
       ui::mojom::WindowManager::kShowState_Property,
       static_cast<uint32_t>(ui::mojom::ShowState::MAXIMIZED));
   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