Use ShelfBubbleContainer for x11 override redirect window

* I uesd ShelfBubbleContainer because it has to be above most of
  windows, but below lock screen.
* Refactor ShellSurfaceBase::CreateShellSurfaceWidget
  - use parent_ only if the container isn't specified.
* Removed SetContainer(menu_container) in zxdg_shell.cc because
  it's unnecessary. (It was using transient parent, and container_ was ignored)

We may need a separate container for override redirect but
we probably should implement the xgrabpointer correctly first.

Bug: 938130

Change-Id: I2e24e3f117764ff2e420fb9f7bd6390673d25c83
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1602659Reviewed-by: Nic Hollingum <hollingum@google.com>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659385}

components/exo/shell_surface.cc

@@ -309,13 +309,9 @@ void ShellSurface::OnSetParent(Surface* parent, const gfx::Point& position) {
 
 void ShellSurface::InitializeWindowState(ash::wm::WindowState* window_state) {
   window_state->AddObserver(this);
-  // Sommelier sets the null application id for override redirect windows,
-  // which controls its bounds by itself.
-  bool emulate_x11_override_redirect =
-      (GetShellApplicationId(window_state->window()) == nullptr) && !!parent_;
-  window_state->set_allow_set_bounds_direct(emulate_x11_override_redirect);
-  widget_->set_movement_disabled(movement_disabled_);
+  window_state->set_allow_set_bounds_direct(movement_disabled_);
   window_state->set_ignore_keyboard_bounds_change(movement_disabled_);
+  widget_->set_movement_disabled(movement_disabled_);
 
   // If this window is a child of some window, it should be made transient.
   MaybeMakeTransient();
@@ -572,22 +568,18 @@ void ShellSurface::Configure(bool ends_drag) {
   gfx::Vector2d origin_offset = pending_origin_offset_accumulator_;
   pending_origin_offset_accumulator_ = gfx::Vector2d();
 
+  auto* window_state =
+      widget_ ? ash::wm::GetWindowState(widget_->GetNativeWindow()) : nullptr;
   int resize_component = HTCAPTION;
-  if (widget_) {
-    ash::wm::WindowState* window_state =
-        ash::wm::GetWindowState(widget_->GetNativeWindow());
-
   // If surface is being resized, save the resize direction.
-    if (window_state->is_dragged() && !ends_drag)
+  if (window_state && window_state->is_dragged() && !ends_drag)
     resize_component = window_state->drag_details()->window_component;
-  }
 
   uint32_t serial = 0;
   if (!configure_callback_.is_null()) {
-    if (widget_) {
+    if (window_state) {
       serial = configure_callback_.Run(
-          GetClientViewBounds().size(),
-          ash::wm::GetWindowState(widget_->GetNativeWindow())->GetStateType(),
+          GetClientViewBounds().size(), window_state->GetStateType(),
           IsResizing(), widget_->IsActive(), origin_offset);
     } else {
       serial =

components/exo/shell_surface_base.cc

@@ -249,7 +249,6 @@ class CustomWindowTargeter : public aura::WindowTargeter {
         !widget_->widget_delegate()->CanResize()) {
       return false;
     }
-
     // Use ash's resize handle detection logic if
     // a) ClientControlledShellSurface uses server side resize or
     // b) xdg shell is using the server side decoration.
@@ -861,22 +860,45 @@ void ShellSurfaceBase::CreateShellSurfaceWidget(
   DCHECK(enabled());
   DCHECK(!widget_);
 
+  // Sommelier sets the null application id for override redirect windows,
+  // which controls its bounds by itself.
+  bool emulate_x11_override_redirect =
+      !is_popup_ && parent_ && ash::desks_util::IsDeskContainerId(container_) &&
+      !application_id_.has_value();
+
+  if (emulate_x11_override_redirect) {
+    // override redirect is used for menu, tooltips etc, which should be placed
+    // above normal windows, but below lock screen. Specify the container here
+    // to avoid using parent_ in params.parent.
+    container_ = ash::kShellWindowId_ShelfBubbleContainer;
+    // X11 override redirect should not be activatable.
+    activatable_ = false;
+    DisableMovement();
+  }
+
   views::Widget::InitParams params;
-  params.type = is_popup_ ? views::Widget::InitParams::TYPE_POPUP
-                          : views::Widget::InitParams::TYPE_WINDOW;
+  params.type = emulate_x11_override_redirect
+                    ? views::Widget::InitParams::TYPE_MENU
+                    : (is_popup_ ? views::Widget::InitParams::TYPE_POPUP
+                                 : views::Widget::InitParams::TYPE_WINDOW);
   params.ownership = views::Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET;
   params.delegate = this;
   params.shadow_type = views::Widget::InitParams::SHADOW_TYPE_NONE;
   params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
   params.show_state = show_state;
-  // Make shell surface a transient child if |parent_| has been set.
-  params.parent =
-      parent_ ? parent_
-              : ash::Shell::GetContainer(
-                    WMHelper::GetInstance()->GetRootWindowForNewWindows(),
-                    container_);
+  // Make shell surface a transient child if |parent_| has been set and
+  // container_ isn't specified.
+  if (ash::desks_util::IsDeskContainerId(container_) && parent_) {
+    params.parent = parent_;
+  } else {
+    params.parent = ash::Shell::GetContainer(
+        WMHelper::GetInstance()->GetRootWindowForNewWindows(), container_);
+  }
   params.bounds = gfx::Rect(origin_, gfx::Size());
   bool activatable = activatable_;
+  if (container_ == ash::kShellWindowId_SystemModalContainer)
+    activatable &= HasHitTestRegion();
+
   // ShellSurfaces in system modal container are only activatable if input
   // region is non-empty. See OnCommitSurface() for more details.
   if (container_ == ash::kShellWindowId_SystemModalContainer)
@@ -907,6 +929,7 @@ void ShellSurfaceBase::CreateShellSurfaceWidget(
   InitializeWindowState(window_state);
 
   // AutoHide shelf in fullscreen state.
+  if (window_state)
     window_state->SetHideShelfWhenFullscreen(false);
 
   // Fade visibility animations for non-activatable windows.

components/exo/shell_surface_unittest.cc

@@ -318,7 +318,12 @@ TEST_F(ShellSurfaceTest, EmulateOverrideRedirect) {
   child_surface->Commit();
   aura::Window* child_window =
       child_shell_surface->GetWidget()->GetNativeWindow();
+
+  // The window will not have a window state, thus will no be managed by window
+  // manager.
   EXPECT_TRUE(ash::wm::GetWindowState(child_window)->allow_set_bounds_direct());
+  EXPECT_EQ(ash::kShellWindowId_ShelfBubbleContainer,
+            child_window->parent()->id());
 }
 
 TEST_F(ShellSurfaceTest, SetStartupId) {