exo: Set widget parent to target root window

components/exo/shell_surface.cc

 // Copyright 2015 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 "components/exo/shell_surface.h"
 
 #include "ash/aura/wm_window_aura.h"
 #include "ash/common/shell_window_ids.h"
 #include "ash/common/wm/window_resizer.h"
 #include "ash/common/wm/window_state.h"
@@ skipping 869 matching lines @@
   DCHECK(enabled());
   DCHECK(!widget_);
 
   views::Widget::InitParams params;
   params.type = 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;
-  params.parent =
-      ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(), container_);
+  // Make shell surface a transient child if |parent_| has been set.
+  params.parent = parent_ ? parent_ :

[reveman, 2016/07/29 18:27:07]

Pretty sure this will break xdg shell modal dialogs. Parent will be set in this
case but the widget should not be a child of the parent widget as that would
result in the dialog being moved if the parent was moved. that's not the
expected behavior. Can this be something like "parent_ ?
parent_->GetRootWindow()" instead and the transient child code below is kept as
it is?

fyi, gtk3-demo is useful to test xdg shell implementation.

[oshima, 2016/07/29 20:35:58]

That's still not right. Let's discuss offline.

[Dominik Laskowski, 2016/07/29 20:37:09]

Unless |params.child| is true, Widget::Init makes the widget a transient child
of |params.parent|, and then uses |params.parent| as a context to find a parent
root window. This is equivalent to the old code, except the root window is the
same as that of the parent.

I've verified that modal dialogs in gtk3-demo behave as expected, modulo an
unrelated bug where windows underneath can be focused.

+      ash::Shell::GetContainer(ash::Shell::GetTargetRootWindow(), container_);
   params.bounds = initial_bounds_;
   bool activatable = activatable_;
   // ShellSurfaces in system modal container are only activatable if input
   // region is non-empty. See OnCommitSurface() for more details.
   if (container_ == ash::kShellWindowId_SystemModalContainer)
     activatable &= !surface_->GetHitTestBounds().IsEmpty();
   params.activatable = activatable ? views::Widget::InitParams::ACTIVATABLE_YES
                                    : views::Widget::InitParams::ACTIVATABLE_NO;
 
   // Note: NativeWidget owns this widget.
@@ skipping 15 matching lines @@
   if (window_state->GetStateType() != ash::wm::WINDOW_STATE_TYPE_NORMAL &&
       !state_changed_callback_.is_null()) {
     state_changed_callback_.Run(ash::wm::WINDOW_STATE_TYPE_NORMAL,
                                 window_state->GetStateType());
   }
 
   // Disable movement if initial bounds were specified.
   widget_->set_movement_disabled(!initial_bounds_.IsEmpty());
   window_state->set_ignore_keyboard_bounds_change(!initial_bounds_.IsEmpty());
 
-  // Make shell surface a transient child if |parent_| has been set.
-  if (parent_)
-    wm::AddTransientChild(parent_, window);
-
   // Allow Ash to manage the position of a top-level shell surfaces if show
   // state is one that allows auto positioning and |initial_bounds_| has
   // not been set.
   window_state->set_window_position_managed(
       ash::wm::ToWindowShowState(ash::wm::WINDOW_STATE_TYPE_AUTO_POSITIONED) ==
           show_state &&
       initial_bounds_.IsEmpty());
 
   // Register close window accelerators.
   views::FocusManager* focus_manager = widget_->GetFocusManager();
@@ skipping 282 matching lines @@
     shadow_origin -= origin.OffsetFromOrigin();
     gfx::Rect shadow_bounds(shadow_origin, shadow_content_bounds_.size());
 
     shadow_overlay_->SetBounds(shadow_bounds);
     shadow_underlay_->SetBounds(shadow_bounds);
     shadow->SetContentBounds(gfx::Rect(shadow_bounds.size()));
   }
 }
 
 }  // namespace exo