exo: Fix dragging edge cases

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 <algorithm>
 #include <utility>
 
 #include "ash/common/frame/custom_frame_view_ash.h"
@@ skipping 355 matching lines @@
     : widget_(nullptr),
       surface_(surface),
       parent_(parent ? parent->GetWidget()->GetNativeWindow() : nullptr),
       initial_bounds_(initial_bounds),
       transform_to_screen_(std::move(transform_to_screen)),
       transform_from_screen_(std::move(transform_from_screen)),
       activatable_(activatable),
       can_minimize_(can_minimize),
       container_(container) {
   WMHelper::GetInstance()->AddActivationObserver(this);
+  WMHelper::GetInstance()->AddShellObserver(this);
   surface_->SetSurfaceDelegate(this);
   surface_->AddSurfaceObserver(this);
   surface_->window()->Show();
   set_owned_by_client();
   if (parent_)
     parent_->AddObserver(this);
 }
 
 ShellSurface::ShellSurface(Surface* surface)
     : ShellSurface(surface,
@@ skipping 12 matching lines @@
     ash::wm::GetWindowState(widget_->GetNativeWindow())->RemoveObserver(this);
     widget_->GetNativeWindow()->RemoveObserver(this);
     // Remove transient children so they are not automatically destroyed.
     for (auto child : wm::GetTransientChildren(widget_->GetNativeWindow()))
       wm::RemoveTransientChild(widget_->GetNativeWindow(), child);
     if (widget_->IsVisible())
       widget_->Hide();
     widget_->CloseNow();
   }
   WMHelper::GetInstance()->RemoveActivationObserver(this);
+  WMHelper::GetInstance()->RemoveShellObserver(this);
   if (parent_)
     parent_->RemoveObserver(this);
   if (surface_) {
     if (scale_ != 1.0)
       surface_->window()->SetTransform(gfx::Transform());
     surface_->SetSurfaceDelegate(nullptr);
     surface_->RemoveSurfaceObserver(this);
   }
   WMHelper::GetInstance()->RemoveAccessibilityObserver(this);
 }
@@ skipping 509 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // ash::wm::WindowStateObserver overrides:
 
 void ShellSurface::OnPreWindowStateTypeChange(
     ash::wm::WindowState* window_state,
     ash::wm::WindowStateType old_type) {
   ash::wm::WindowStateType new_type = window_state->GetStateType();
   if (ash::wm::IsMaximizedOrFullscreenOrPinnedWindowStateType(old_type) ||
       ash::wm::IsMaximizedOrFullscreenOrPinnedWindowStateType(new_type)) {
+    EndDragOrMove(false /* revert */);

[reveman, 2017/01/23 20:23:48]

how is other chrome windows handling this?

[Dominik Laskowski, 2017/01/23 23:01:36]

It's not handled. If you enter full screen after dragging (but not dropping) a
Chrome window to another display, its content disappears and the full screen key
stops working.

[reveman, 2017/01/26 06:50:21]

That sounds like a bug. Is there a more general way to fix this than just making
exo/ handle it correctly?

+
     // When transitioning in/out of maximized or fullscreen mode we need to
     // make sure we have a configure callback before we allow the default
     // cross-fade animations. The configure callback provides a mechanism for
     // the client to inform us that a frame has taken the state change into
     // account and without this cross-fade animations are unreliable.
     if (configure_callback_.is_null())
       scoped_animations_disabled_.reset(new ScopedAnimationsDisabled(this));
   }
 }
 
@@ skipping 74 matching lines @@
 
   if (gained_active == widget_->GetNativeWindow() ||
       lost_active == widget_->GetNativeWindow()) {
     DCHECK(activatable_);
     Configure();
     UpdateShadow();
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// WMHelper::ShellObserver overrides:
+
+void ShellSurface::OnOverviewModeStarted() {
+  EndDragOrMove(false /* revert */);

[reveman, 2017/01/23 20:23:48]

is this also a problem for non-exo window dragging? or are they already doing
this?

[Dominik Laskowski, 2017/01/23 23:01:36]

Yes, entering/exiting overview mode while dragging is broken for Chrome windows
too. You can keep dragging in overview mode, leading to inconsistent state. I
have another small patch that does the same for Ash.

[reveman, 2017/01/26 06:50:21]

Ok, let's do Ash first and once that lands we'll make a consistent change here.

+  ignore_widget_bounds_changes_ = true;

[reveman, 2017/01/23 20:23:48]

why is this needed? seems fragile to rely on us detecting when we enter this
specific mode..

[Dominik Laskowski, 2017/01/23 23:01:36]

When entering overview mode, a lost focus event causes Android to abort the
drag, but it's possible for geometry updates to be in flight at that time. This
flag is needed to discard those updates.

[reveman, 2017/01/26 06:50:21]

Why do they need to be discarded? Doesn't this leave us in a state where chrome
is not in sync with the android state? 

+}
+
+void ShellSurface::OnOverviewModeEnded() {
+  ignore_widget_bounds_changes_ = false;
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // WMHelper::AccessibilityObserver overrides:
 
 void ShellSurface::OnAccessibilityModeChanged() {
   UpdateShadow();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ui::EventHandler overrides:
 
 void ShellSurface::OnKeyEvent(ui::KeyEvent* event) {
@@ skipping 362 matching lines @@
   // Return early if the shell is currently managing the bounds of the widget.
   // 1) When a window is either maximized/fullscreen/pinned, and the bounds
   // isn't controlled by a client.
   ash::wm::WindowState* window_state =
       ash::wm::GetWindowState(widget_->GetNativeWindow());
   if (window_state->IsMaximizedOrFullscreenOrPinned() &&
       !window_state->allow_set_bounds_in_maximized()) {
     return;
   }
 
-  // 2) When a window is being dragged.
-  if (IsResizing())
+  // 2) When a window is being dragged, or is being moved by the shell.
+  if (IsResizing() || ignore_widget_bounds_changes_)
     return;
 
   // Return early if there is pending configure requests.
   if (!pending_configs_.empty() || scoped_configure_)
     return;
 
   gfx::Rect visible_bounds = GetVisibleBounds();
   gfx::Rect new_widget_bounds =
       widget_->non_client_view()->GetWindowBoundsForClientBounds(
           visible_bounds);
@@ skipping 161 matching lines @@
 gfx::Point ShellSurface::GetMouseLocation() const {
   aura::Window* const root_window = widget_->GetNativeWindow()->GetRootWindow();
   gfx::Point location =
       root_window->GetHost()->dispatcher()->GetLastMouseLocationInRoot();
   aura::Window::ConvertPointToTarget(
       root_window, widget_->GetNativeWindow()->parent(), &location);
   return location;
 }
 
 }  // namespace exo