exo: Initial support for multiple displays in ARC

components/exo/shell_surface.cc

Index: components/exo/shell_surface.cc
diff --git a/components/exo/shell_surface.cc b/components/exo/shell_surface.cc
index e5322a675378dc72252b64a771441b1f5d9ba7ce..dee88311d4a3797b95a42991cdf8292bf81293a1 100644
--- a/components/exo/shell_surface.cc
+++ b/components/exo/shell_surface.cc
@@ -10,8 +10,10 @@
 #include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/wm/window_resizer.h"
 #include "ash/common/wm/window_state.h"
+#include "ash/common/wm_shell.h"
 #include "ash/common/wm_window.h"
 #include "ash/public/cpp/shell_window_ids.h"
+#include "ash/wm/drag_window_resizer.h"
 #include "ash/wm/window_state_aura.h"
 #include "ash/wm/window_util.h"
 #include "base/logging.h"
@@ -143,6 +145,26 @@ class CustomWindowTargeter : public aura::WindowTargeter {
   DISALLOW_COPY_AND_ASSIGN(CustomWindowTargeter);
 };
 
+class CustomWindowResizer : public ash::WindowResizer {
+ public:
+  explicit CustomWindowResizer(ash::wm::WindowState* window_state)
+      : WindowResizer(window_state), shell_(GetTarget()->GetShell()) {
+    shell_->LockCursor();
+  }
+

[reveman, 2017/03/02 02:00:38]

nit: remove this blank line

[Dominik Laskowski, 2017/03/03 13:50:40]

Done.

+  ~CustomWindowResizer() override { shell_->UnlockCursor(); }
+
+  // ash::WindowResizer:

[reveman, 2017/03/02 02:00:38]

nit: // Overridden from ash::WindowResizer:

[Dominik Laskowski, 2017/03/03 13:50:40]

Done.

+  void Drag(const gfx::Point& location, int event_flags) override {}
+  void CompleteDrag() override {}
+  void RevertDrag() override {}
+
+ private:
+  ash::WmShell* const shell_;
+
+  DISALLOW_COPY_AND_ASSIGN(CustomWindowResizer);
+};
+
 class ShellSurfaceWidget : public views::Widget {
  public:
   explicit ShellSurfaceWidget(ShellSurface* shell_surface)
@@ -304,6 +326,7 @@ ShellSurface::ShellSurface(Surface* surface,
       can_minimize_(can_minimize),
       container_(container) {
   WMHelper::GetInstance()->AddActivationObserver(this);
+  WMHelper::GetInstance()->AddDisplayConfigurationObserver(this);
   surface_->SetSurfaceDelegate(this);
   surface_->AddSurfaceObserver(this);
   surface_->window()->Show();
@@ -336,6 +359,7 @@ ShellSurface::~ShellSurface() {
     widget_->CloseNow();
   }
   WMHelper::GetInstance()->RemoveActivationObserver(this);
+  WMHelper::GetInstance()->RemoveDisplayConfigurationObserver(this);
   if (parent_)
     parent_->RemoveObserver(this);
   if (surface_) {
@@ -524,7 +548,7 @@ void ShellSurface::SetApplicationId(const std::string& application_id) {
 void ShellSurface::Move() {
   TRACE_EVENT0("exo", "ShellSurface::Move");
 
-  if (!widget_)
+  if (!widget_ || resizer_)

[reveman, 2017/03/02 02:00:38]

why is this needed now?

[Dominik Laskowski, 2017/03/03 13:50:40]

This check was pulled out from AttemptToStartDrag to be shared with
AttemptToStartClientDrag, but I've merged those in the latest patch.

     return;
 
   switch (bounds_mode_) {
@@ -532,6 +556,8 @@ void ShellSurface::Move() {
       AttemptToStartDrag(HTCAPTION);
       return;
     case BoundsMode::CLIENT:
+      AttemptToStartClientDrag();

[reveman, 2017/03/02 02:00:38]

nit: please move switch statement into AttemptToStartDrag instead so
AttemptToStartClientDrag is not needed. I'm not a fan of the "ClientDrag" name
in general

[Dominik Laskowski, 2017/03/03 13:50:40]

Merged AttemptToStartClientDrag into AttemptToStartDrag.

+      return;
     case BoundsMode::FIXED:
       return;
   }
@@ -542,7 +568,7 @@ void ShellSurface::Move() {
 void ShellSurface::Resize(int component) {
   TRACE_EVENT1("exo", "ShellSurface::Resize", "component", component);
 
-  if (!widget_)
+  if (!widget_ || resizer_)

[reveman, 2017/03/02 02:00:38]

why is this needed now?

[Dominik Laskowski, 2017/03/03 13:50:40]

See above.

     return;
 
   switch (bounds_mode_) {
@@ -998,6 +1024,14 @@ void ShellSurface::OnAccessibilityModeChanged() {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// WMHelper::DisplayConfigurationObserver overrides:
+
+void ShellSurface::OnDisplayConfigurationChanged() {
+  if (!display_config_changed_callback_.is_null())
+    display_config_changed_callback_.Run();

[reveman, 2017/03/02 02:00:38]

why not send a configure request and adjust origin here directly for
BoundsMode::CLIENT? I thought the purpose of bounds_mode_ was so we could do
that.

[Dominik Laskowski, 2017/03/03 13:50:40]

Given that the origin is part of the public ShellSurface API (constructor and
SetOrigin), I think it's cleaner for the origin to be computed by the user of
ShellSurface, i.e. WaylandRemoteShell.

[reveman, 2017/03/06 19:02:13]

As other changes to origin happen internally and result in configure events I
much prefer if that was also the case for display configuration changes. If this
means we can remove SetOrigin API then that would be great but maybe we still
need that..

[Dominik Laskowski, 2017/03/08 23:13:22]

But then ShellSurface would contain logic specific to Android. With this
injection, the virtual display concept is at least contained in
WaylandRemoteShell.

+}
+
+////////////////////////////////////////////////////////////////////////////////
 // ui::EventHandler overrides:
 
 void ShellSurface::OnKeyEvent(ui::KeyEvent* event) {
@@ -1006,7 +1040,8 @@ void ShellSurface::OnKeyEvent(ui::KeyEvent* event) {
     return;
   }
 
-  if (event->type() == ui::ET_KEY_PRESSED &&
+  if (bounds_mode_ == BoundsMode::SHELL &&

[reveman, 2017/03/02 02:00:38]

We should be allowed to cancel a client drag too. Why add this special case?

[Dominik Laskowski, 2017/03/03 13:50:40]

We need to sync window states to revert dragging properly in the CLIENT case. I
figured that we should disable this until then (even though it's not an issue
because the client currently destroys the window on VKEY_ESCAPE).

[reveman, 2017/03/06 19:02:13]

Please leave this the way it's supposed to work long term of that's not causing
problems. Temporary code that is doing something else on the Chrome side should
at a minimal have a TODO statement with description and a bug number to make it
clear that the code is should updated when possible.

[Dominik Laskowski, 2017/03/08 23:13:22]

Reverted and added TODO.

+      event->type() == ui::ET_KEY_PRESSED &&
       event->key_code() == ui::VKEY_ESCAPE) {
     EndDrag(true /* revert */);
   }
@@ -1035,6 +1070,9 @@ void ShellSurface::OnMouseEvent(ui::MouseEvent* event) {
 
   switch (event->type()) {
     case ui::ET_MOUSE_DRAGGED: {
+      if (bounds_mode_ == BoundsMode::CLIENT)
+        break;
+
       gfx::Point location(event->location());
       aura::Window::ConvertPointToTarget(widget_->GetNativeWindow(),
                                          widget_->GetNativeWindow()->parent(),
@@ -1213,23 +1251,13 @@ void ShellSurface::Configure() {
 void ShellSurface::AttemptToStartDrag(int component) {
   DCHECK(widget_);
 
-  // Cannot start another drag if one is already taking place.

[reveman, 2017/03/02 02:00:38]

why remove this?

[Dominik Laskowski, 2017/03/03 13:50:40]

See above.

-  if (resizer_)
-    return;
-
   if (widget_->GetNativeWindow()->HasCapture())
     return;
 
-  aura::Window* root_window = widget_->GetNativeWindow()->GetRootWindow();
-  gfx::Point drag_location =
-      root_window->GetHost()->dispatcher()->GetLastMouseLocationInRoot();
-  aura::Window::ConvertPointToTarget(
-      root_window, widget_->GetNativeWindow()->parent(), &drag_location);
-
   // Set the cursor before calling CreateWindowResizer(), as that will
   // eventually call LockCursor() and prevent the cursor from changing.
-  aura::client::CursorClient* cursor_client =
-      aura::client::GetCursorClient(root_window);
+  aura::client::CursorClient* cursor_client = aura::client::GetCursorClient(
+      widget_->GetNativeWindow()->GetRootWindow());
   DCHECK(cursor_client);
 
   switch (component) {
@@ -1266,8 +1294,8 @@ void ShellSurface::AttemptToStartDrag(int component) {
   }
 
   resizer_ = ash::CreateWindowResizer(
-      ash::WmWindow::Get(widget_->GetNativeWindow()), drag_location, component,
-      aura::client::WINDOW_MOVE_SOURCE_MOUSE);
+      ash::WmWindow::Get(widget_->GetNativeWindow()), GetMouseLocation(),
+      component, aura::client::WINDOW_MOVE_SOURCE_MOUSE);
   if (!resizer_)
     return;
 
@@ -1286,26 +1314,63 @@ void ShellSurface::AttemptToStartDrag(int component) {
     Configure();
 }
 
+void ShellSurface::AttemptToStartClientDrag() {
+  DCHECK(widget_);
+
+  if (surface_->window()->HasCapture())
+    return;
+
+  ash::wm::WindowState* const window_state =
+      ash::wm::GetWindowState(widget_->GetNativeWindow());
+
+  DCHECK(!window_state->drag_details());
+  window_state->CreateDragDetails(GetMouseLocation(), HTCAPTION,
+                                  aura::client::WINDOW_MOVE_SOURCE_MOUSE);
+
+  // The resizer renders phantom windows, but does not control window movement.
+  resizer_.reset(ash::DragWindowResizer::Create(
+      new CustomWindowResizer(window_state), window_state));
+
+  WMHelper::GetInstance()->AddPreTargetHandler(this);
+  surface_->window()->SetCapture();
+}
+
 void ShellSurface::EndDrag(bool revert) {
   DCHECK(widget_);
   DCHECK(resizer_);
 
-  bool was_resizing = IsResizing();
+  switch (bounds_mode_) {
+    case BoundsMode::SHELL: {
+      bool was_resizing = IsResizing();
+
+      EndDrag(widget_->GetNativeWindow(), revert);
+
+      // Notify client that resizing state has changed.
+      if (was_resizing)
+        Configure();
+
+      UpdateWidgetBounds();
+      return;
+    }
+    case BoundsMode::CLIENT:
+      EndDrag(surface_->window(), revert);

[reveman, 2017/03/02 02:00:38]

why are we using surface_->window() here instead of widget_->GetNativeWindow()?
what if surface_ is gone?

[Dominik Laskowski, 2017/03/03 13:50:40]

The surface window needs to have capture, because it's the target for mouse
events that get forwarded to the client. Good point, I've added a null check in
AttemptToStartDrag and a DCHECK here as a precaution against a bad client that
sends a "move" request after the surface is destroyed.

[reveman, 2017/03/06 19:02:13]

The general idea of doing a capture on the surface window to have events still
sent to the client seems like a bit of a hack and doesn't work with MUS support
as only the top-level ShellSurface will have a window in the future. I think it
would be best to have these changes to movement sent back to the client as
origin changes in configure events. This would also allow features like window
snapping to work properly.

I'm OK saving this as a follow up but please file bugs and add appropriate
TODO's where needed in the code. 

[Dominik Laskowski, 2017/03/08 23:13:22]

Filed crbug.com/699746 and added TODOs.

+      return;
+    case BoundsMode::FIXED:
+      break;
+  }
+
+  NOTREACHED();
+}
 
+void ShellSurface::EndDrag(aura::Window* window, bool revert) {
   if (revert)
     resizer_->RevertDrag();
   else
     resizer_->CompleteDrag();
 
   WMHelper::GetInstance()->RemovePreTargetHandler(this);
-  widget_->GetNativeWindow()->ReleaseCapture();
+  window->ReleaseCapture();
   resizer_.reset();
-

[reveman, 2017/03/02 02:00:38]

why not keep this the way it is?

[Dominik Laskowski, 2017/03/03 13:50:40]

Refactored to be consistent with AttemptToStartDrag.

-  // Notify client that resizing state has changed.
-  if (was_resizing)
-    Configure();
-
-  UpdateWidgetBounds();
 }
 
 bool ShellSurface::IsResizing() const {
@@ -1411,8 +1476,31 @@ void ShellSurface::UpdateWidgetBounds() {
   // should not result in a configure request.
   DCHECK(!ignore_window_bounds_changes_);
   ignore_window_bounds_changes_ = true;
-  if (widget_->GetWindowBoundsInScreen() != new_widget_bounds)
-    widget_->SetBounds(new_widget_bounds);
+  const gfx::Rect widget_bounds = widget_->GetWindowBoundsInScreen();
+  if (widget_bounds != new_widget_bounds) {
+    if (bounds_mode_ != BoundsMode::CLIENT || !resizer_) {
+      widget_->SetBounds(new_widget_bounds);
+      UpdateSurfaceBounds();
+    } else {
+      // TODO(domlaskowski): Synchronize window state transitions with the
+      // client, and abort client-side dragging on transition to fullscreen.
+      LOG_IF(ERROR, widget_bounds.size() != new_widget_bounds.size())
+          << "Window size changed during client-driven drag";
+
+      // Convert from screen to display coordinates.
+      gfx::Point origin = new_widget_bounds.origin();
+      wm::ConvertPointFromScreen(widget_->GetNativeWindow()->parent(), &origin);
+      new_widget_bounds.set_origin(origin);
+
+      // Move the window relative to the current display.
+      widget_->GetNativeWindow()->SetBounds(new_widget_bounds);
+      UpdateSurfaceBounds();
+
+      // Render phantom windows when beyond the current display.
+      resizer_->Drag(GetMouseLocation(), 0);
+    }
+  }
+
   ignore_window_bounds_changes_ = false;
 }
 
@@ -1576,4 +1664,13 @@ void ShellSurface::UpdateShadow() {
   }
 }
 
+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