exo: Refactor ShellSurface and WaylandRemoteShell

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 "ash/common/frame/custom_frame_view_ash.h"
 #include "ash/common/shelf/wm_shelf.h"
@@ skipping 317 matching lines @@
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ShellSurface, public:
 
 DEFINE_LOCAL_UI_CLASS_PROPERTY_KEY(Surface*, kMainSurfaceKey, nullptr)
 
 ShellSurface::ShellSurface(Surface* surface,
                            ShellSurface* parent,
-                           const gfx::Rect& initial_bounds,
+                           BoundsMode bounds_mode,
+                           const gfx::Point& origin,
                            bool activatable,
                            bool can_minimize,
                            int container)
     : widget_(nullptr),
       surface_(surface),
       parent_(parent ? parent->GetWidget()->GetNativeWindow() : nullptr),
-      initial_bounds_(initial_bounds),
+      bounds_mode_(bounds_mode),
+      origin_(origin),
       activatable_(activatable),
       can_minimize_(can_minimize),
       container_(container) {
   WMHelper::GetInstance()->AddActivationObserver(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,
                    nullptr,
-                   gfx::Rect(),
+                   BoundsMode::SHELL,
+                   gfx::Point(),
                    true,
                    true,
                    ash::kShellWindowId_DefaultContainer) {}
 
 ShellSurface::~ShellSurface() {
   DCHECK(!scoped_configure_);
   if (resizer_)
     EndDrag(false /* revert */);
   if (widget_) {
     ash::wm::GetWindowState(widget_->GetNativeWindow())->RemoveObserver(this);
@@ skipping 185 matching lines @@
 void ShellSurface::SetApplicationId(const std::string& application_id) {
   // Store the value in |application_id_| in case the window does not exist yet.
   application_id_ = application_id;
   if (widget_ && widget_->GetNativeWindow())
     SetApplicationId(widget_->GetNativeWindow(), application_id);
 }
 
 void ShellSurface::Move() {
   TRACE_EVENT0("exo", "ShellSurface::Move");
 
-  if (widget_ && !widget_->movement_disabled())
-    AttemptToStartDrag(HTCAPTION);
+  if (!widget_)
+    return;
+
+  switch (bounds_mode_) {
+    case BoundsMode::SHELL:
+      AttemptToStartDrag(HTCAPTION);
+      break;
+    case BoundsMode::CLIENT:
+    case BoundsMode::FIXED:
+      NOTREACHED();

[reveman, 2017/02/09 04:50:15]

How are we protected from a malicious client causing this code to be executed?

[Dominik Laskowski, 2017/02/09 23:35:28]

NOTREACHED only logs an error in release builds.

[reveman, 2017/02/10 02:10:53]

A malicious client being able to crash a debug build is not OK. NOTREACHED means
it will not be reached unless there's a bug.

+      break;
+  }

[reveman, 2017/02/09 04:50:16]

nit: please add return statements in the switch cases above so you can add a
NOTREACHED() here to detect if bounds_mode_ has entered an invalid state at
runtime

[Dominik Laskowski, 2017/02/09 23:35:28]

Done.

 }
 
 void ShellSurface::Resize(int component) {
   TRACE_EVENT1("exo", "ShellSurface::Resize", "component", component);
 
-  if (widget_ && !widget_->movement_disabled())
-    AttemptToStartDrag(component);
+  if (!widget_)
+    return;
+
+  switch (bounds_mode_) {

[reveman, 2017/02/09 04:50:15]

same comments here

[Dominik Laskowski, 2017/02/09 23:35:28]

Done.

+    case BoundsMode::SHELL:
+      AttemptToStartDrag(component);
+      break;
+    case BoundsMode::CLIENT:
+    case BoundsMode::FIXED:
+      NOTREACHED();
+      break;
+  }
 }
 
 void ShellSurface::Close() {
   if (!close_callback_.is_null())
     close_callback_.Run();
 }
 
 void ShellSurface::SetGeometry(const gfx::Rect& geometry) {
   TRACE_EVENT1("exo", "ShellSurface::SetGeometry", "geometry",
                geometry.ToString());
@@ skipping 56 matching lines @@
 
 void ShellSurface::SetTopInset(int height) {
   TRACE_EVENT1("exo", "ShellSurface::SetTopInset", "height", height);
 
   pending_top_inset_height_ = height;
 }
 
 void ShellSurface::SetOrigin(const gfx::Point& origin) {
   TRACE_EVENT1("exo", "ShellSurface::SetOrigin", "origin", origin.ToString());
 
-  initial_bounds_ = gfx::Rect(origin, gfx::Size(1, 1));
+  origin_ = origin;
 }
 
 void ShellSurface::SetActivatable(bool activatable) {
   TRACE_EVENT1("exo", "ShellSurface::SetActivatable", "activatable",
                activatable);
 
   activatable_ = activatable;
 }
 
 void ShellSurface::SetContainer(int container) {
@@ skipping 34 matching lines @@
   if (enabled() && !widget_) {
     // Defer widget creation until surface contains some contents.
     if (surface_->content_size().IsEmpty())
       Configure();
     else
       CreateShellSurfaceWidget(ui::SHOW_STATE_NORMAL);
   }
 
   // Apply the accumulated pending origin offset to reflect acknowledged
   // configure requests.
-  origin_ += pending_origin_offset_;
+  origin_offset_ += pending_origin_offset_;
   pending_origin_offset_ = gfx::Vector2d();
 
   // Update resize direction to reflect acknowledged configure requests.
   resize_component_ = pending_resize_component_;
 
   if (widget_) {
     // Apply new window geometry.
     geometry_ = pending_geometry_;
 
     UpdateWidgetBounds();
@@ skipping 18 matching lines @@
       if (activatable != CanActivate()) {
         set_can_activate(activatable);
         // Activate or deactivate window if activation state changed.
         if (activatable)
           wm::ActivateWindow(widget_->GetNativeWindow());
         else if (widget_->IsActive())
           wm::DeactivateWindow(widget_->GetNativeWindow());
       }
     }
 
-    gfx::Rect client_view_bounds =
-        widget_->non_client_view()->frame_view()->GetBoundsForClientView();
-
-    // Update surface bounds.
-    surface_->window()->SetBounds(
-        gfx::Rect(GetSurfaceOrigin() + client_view_bounds.OffsetFromOrigin(),
-                  surface_->window()->layer()->size()));
+    UpdateSurfaceBounds();
 
     // Update surface scale.
     if (pending_scale_ != scale_) {
       gfx::Transform transform;
       DCHECK_NE(pending_scale_, 0.0);
       transform.Scale(1.0 / pending_scale_, 1.0 / pending_scale_);
       surface_->window()->SetTransform(transform);
       scale_ = pending_scale_;
     }
 
@@ skipping 36 matching lines @@
   // destroyed callback may destroy the ShellSurface instance. This call needs
   // to be last so that the instance can be destroyed.
   if (!surface_destroyed_callback_.is_null())
     surface_destroyed_callback_.Run();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // views::WidgetDelegate overrides:
 
 bool ShellSurface::CanResize() const {
-  return initial_bounds_.IsEmpty();
+  return bounds_mode_ == BoundsMode::SHELL;
 }
 
 bool ShellSurface::CanMaximize() const {
   // Shell surfaces in system modal container cannot be maximized.
   if (container_ == ash::kShellWindowId_SystemModalContainer)
     return false;
 
   // Non-transient shell surfaces can be maximized.
   return !parent_;
 }
@@ skipping 113 matching lines @@
                                          const gfx::Rect& old_bounds,
                                          const gfx::Rect& new_bounds) {
   if (!widget_ || !surface_ || ignore_window_bounds_changes_)
     return;
 
   if (window == widget_->GetNativeWindow()) {
     if (new_bounds.size() == old_bounds.size())
       return;
 
     // If size changed then give the client a chance to produce new contents
-    // before origin on screen is changed by adding offset to the next configure
-    // request and offset |origin_| by the same distance.
-    gfx::Vector2d origin_offset = new_bounds.origin() - old_bounds.origin();
-    pending_origin_config_offset_ += origin_offset;
-    origin_ -= origin_offset;
+    // before origin on screen is changed. Retain the old origin by reverting
+    // the origin delta until the next configure is acknowledged.
+    const gfx::Vector2d delta = new_bounds.origin() - old_bounds.origin();
+    origin_offset_ -= delta;
+    pending_origin_offset_accumulator_ += delta;
 
-    gfx::Rect client_view_bounds =
-        widget_->non_client_view()->frame_view()->GetBoundsForClientView();
-
-    // Update surface bounds.
-    surface_->window()->SetBounds(
-        gfx::Rect(GetSurfaceOrigin() + client_view_bounds.OffsetFromOrigin(),
-                  surface_->window()->layer()->size()));
+    UpdateSurfaceBounds();
 
     // The shadow size may be updated to match the widget. Change it back
     // to the shadow content size.
     // TODO(oshima): When the arc window reiszing is enabled, we may want to
     // implement shadow management here instead of using shadow controller.
     UpdateShadow();
 
     Configure();
   }
 }
@@ skipping 121 matching lines @@
   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;
   // Make shell surface a transient child if |parent_| has been set.
   params.parent =
       parent_ ? parent_ : WMHelper::GetInstance()->GetContainer(container_);
-  params.bounds = initial_bounds_;
+  params.bounds = gfx::Rect(origin_, gfx::Size());
   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.
   widget_ = new ShellSurfaceWidget(this);
   widget_->Init(params);
 
   aura::Window* window = widget_->GetNativeWindow();
   window->SetName("ExoShellSurface");
   window->SetProperty(aura::client::kAccessibilityFocusFallsbackToWidgetKey,
                       false);
   window->AddChild(surface_->window());
   window->SetEventTargeter(base::WrapUnique(new CustomWindowTargeter(widget_)));
   SetApplicationId(window, application_id_);
   SetMainSurface(window, surface_);
 
+  const bool client_controls_bounds = bounds_mode_ == BoundsMode::CLIENT;

[reveman, 2017/02/09 04:50:15]

nit: remove 'const'. doesn't hurt but typically not something we use for local
variables in exo code

[Dominik Laskowski, 2017/02/09 23:35:28]

Removed variable along with check below.

+
   // Start tracking changes to window bounds and window state.
-  window->AddObserver(this);
+  if (!client_controls_bounds)

[reveman, 2017/02/09 04:50:15]

can we add the observer unconditionally and instead adjust the observer callback
code?

[Dominik Laskowski, 2017/02/09 23:35:28]

Done.

+    window->AddObserver(this);
   ash::wm::WindowState* window_state = ash::wm::GetWindowState(window);
   window_state->AddObserver(this);
 
-  // Absolete positioned shell surfaces may request the bounds that does not
-  // fill the entire work area / display in maximized / fullscreen state.
-  // Allow such clients to update the bounds in these states.
-  if (!initial_bounds_.IsEmpty())
-    window_state->set_allow_set_bounds_in_maximized(true);
+  // Allow the client to request bounds that do not fill the entire work area
+  // when maximized, or the entire display when fullscreen.
+  window_state->set_allow_set_bounds_in_maximized(client_controls_bounds);
 
   // Notify client of initial state if different than normal.
   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());
+  // Disable movement if bounds are controlled by the client or fixed.
+  const bool movement_disabled = bounds_mode_ != BoundsMode::SHELL;

[reveman, 2017/02/09 04:50:15]

nit: remove 'const'

[Dominik Laskowski, 2017/02/09 23:35:28]

Done.

+  widget_->set_movement_disabled(movement_disabled);
+  window_state->set_ignore_keyboard_bounds_change(movement_disabled);
 
   // AutoHide shelf in fullscreen state.
   window_state->set_hide_shelf_when_fullscreen(false);
 
   // Fade visibility animations for non-activatable windows.
   if (!activatable_) {
     wm::SetWindowVisibilityAnimationType(
         window, wm::WINDOW_VISIBILITY_ANIMATION_TYPE_FADE);
   }
 
@@ skipping 12 matching lines @@
   pending_show_widget_ = true;
 }
 
 void ShellSurface::Configure() {
   // Delay configure callback if |scoped_configure_| is set.
   if (scoped_configure_) {
     scoped_configure_->set_needs_configure();
     return;
   }
 
-  gfx::Vector2d origin_offset = pending_origin_config_offset_;
-  pending_origin_config_offset_ = gfx::Vector2d();
+  const gfx::Vector2d origin_offset = pending_origin_offset_accumulator_;

[reveman, 2017/02/09 04:50:15]

nit: remove 'const'

[Dominik Laskowski, 2017/02/09 23:35:28]

Done.

+  pending_origin_offset_accumulator_ = gfx::Vector2d();
 
   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())
       resize_component = window_state->drag_details()->window_component;
   }
@@ skipping 83 matching lines @@
 
   resizer_ = ash::CreateWindowResizer(
       ash::WmWindow::Get(widget_->GetNativeWindow()), drag_location, component,
       aura::client::WINDOW_MOVE_SOURCE_MOUSE);
   if (!resizer_)
     return;
 
   // Apply pending origin offsets and resize direction before starting a new
   // resize operation. These can still be pending if the client has acknowledged
   // the configure request but not yet called Commit().
-  origin_ += pending_origin_offset_;
+  origin_offset_ += pending_origin_offset_;
   pending_origin_offset_ = gfx::Vector2d();
   resize_component_ = pending_resize_component_;
 
   WMHelper::GetInstance()->AddPreTargetHandler(this);
   widget_->GetNativeWindow()->SetCapture();
 
   // Notify client that resizing state has changed.
   if (IsResizing())
     Configure();
 }
 
 void ShellSurface::EndDrag(bool revert) {
   DCHECK(widget_);
   DCHECK(resizer_);
 
-  bool was_resizing = IsResizing();
+  switch (bounds_mode_) {
+    case BoundsMode::SHELL: {
+      bool was_resizing = IsResizing();
 
-  if (revert)
-    resizer_->RevertDrag();
-  else
-    resizer_->CompleteDrag();
+      if (revert)
+        resizer_->RevertDrag();
+      else
+        resizer_->CompleteDrag();
 
-  WMHelper::GetInstance()->RemovePreTargetHandler(this);
-  widget_->GetNativeWindow()->ReleaseCapture();
-  resizer_.reset();
+      WMHelper::GetInstance()->RemovePreTargetHandler(this);
+      widget_->GetNativeWindow()->ReleaseCapture();
+      resizer_.reset();
 
-  // Notify client that resizing state has changed.
-  if (was_resizing)
-    Configure();
+      // Notify client that resizing state has changed.
+      if (was_resizing)
+        Configure();
 
-  UpdateWidgetBounds();
+      UpdateWidgetBounds();
+      break;
+    }
+    case BoundsMode::CLIENT:
+    case BoundsMode::FIXED:
+      NOTREACHED();

[reveman, 2017/02/09 04:50:15]

Can a malicious client cause this?

[Dominik Laskowski, 2017/02/09 23:35:28]

Only debug builds would crash.

[reveman, 2017/02/10 02:10:53]

Debug builds crashing is not ok.

+      break;
+  }

[reveman, 2017/02/09 04:50:15]

return above and NOTREACHED() here?

[Dominik Laskowski, 2017/02/09 23:35:28]

Done.

 }
 
 bool ShellSurface::IsResizing() const {
   ash::wm::WindowState* window_state =
       ash::wm::GetWindowState(widget_->GetNativeWindow());
   if (!window_state->is_dragged())
     return false;
 
   return window_state->drag_details()->bounds_change &
          ash::WindowResizer::kBoundsChange_Resizes;
 }
 
 gfx::Rect ShellSurface::GetVisibleBounds() const {
   // Use |geometry_| if set, otherwise use the visual bounds of the surface.
   return geometry_.IsEmpty() ? gfx::Rect(surface_->window()->layer()->size())
                              : geometry_;
 }
 
 gfx::Point ShellSurface::GetSurfaceOrigin() const {
-  // If initial bounds were specified then surface origin is always relative
-  // to those bounds.
-  if (!initial_bounds_.IsEmpty()) {
-    gfx::Point origin = widget_->GetNativeWindow()->bounds().origin();
-    wm::ConvertPointToScreen(widget_->GetNativeWindow()->parent(), &origin);
-    return initial_bounds_.origin() - origin.OffsetFromOrigin();
+  // For client-positioned shell surfaces, the surface origin corresponds to the
+  // widget position relative to the origin specified by the client. Since the
+  // surface is positioned relative to the widget, negate this vector to align
+  // the surface with the widget.
+  if (bounds_mode_ != BoundsMode::SHELL) {
+    gfx::Point position = widget_->GetNativeWindow()->bounds().origin();
+    wm::ConvertPointToScreen(widget_->GetNativeWindow()->parent(), &position);
+    return origin_ - position.OffsetFromOrigin();
   }
 
   gfx::Rect visible_bounds = GetVisibleBounds();
   gfx::Rect client_bounds =
       widget_->non_client_view()->frame_view()->GetBoundsForClientView();
   switch (resize_component_) {
     case HTCAPTION:
-      return origin_ - visible_bounds.OffsetFromOrigin();
+      return gfx::Point() + origin_offset_ - visible_bounds.OffsetFromOrigin();
     case HTBOTTOM:
     case HTRIGHT:
     case HTBOTTOMRIGHT:
       return gfx::Point() - visible_bounds.OffsetFromOrigin();
     case HTTOP:
     case HTTOPRIGHT:
       return gfx::Point(0, client_bounds.height() - visible_bounds.height()) -
              visible_bounds.OffsetFromOrigin();
-      break;
     case HTLEFT:
     case HTBOTTOMLEFT:
       return gfx::Point(client_bounds.width() - visible_bounds.width(), 0) -
              visible_bounds.OffsetFromOrigin();
     case HTTOPLEFT:
       return gfx::Point(client_bounds.width() - visible_bounds.width(),
                         client_bounds.height() - visible_bounds.height()) -
              visible_bounds.OffsetFromOrigin();
     default:
       NOTREACHED();
       return gfx::Point();
   }
 }
 
 void ShellSurface::UpdateWidgetBounds() {
   DCHECK(widget_);
 
   // 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;
+  if (bounds_mode_ == BoundsMode::SHELL) {
+    // 1) When a window is either maximized/fullscreen/pinned.
+    ash::wm::WindowState* window_state =
+        ash::wm::GetWindowState(widget_->GetNativeWindow());
+    if (window_state->IsMaximizedOrFullscreenOrPinned())
+      return;
+
+    // 2) When a window is being dragged.
+    if (IsResizing())
+      return;
   }
 
-  // 2) When a window is being dragged.
-  if (IsResizing())
-    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);
 
-  // Avoid changing widget origin unless initial bounds were specified and
-  // widget origin is always relative to it.
-  if (initial_bounds_.IsEmpty()) {
-    new_widget_bounds.set_origin(widget_->GetWindowBoundsInScreen().origin());
-  } else {
-    new_widget_bounds.set_origin(initial_bounds_.origin() +
-                                 visible_bounds.OffsetFromOrigin());
-  }
-
-  // Update widget origin using the surface origin if the current location of
-  // surface is being anchored to one side of the widget as a result of a
-  // resize operation.
-  if (resize_component_ != HTCAPTION) {
-    gfx::Point new_widget_origin =
-        GetSurfaceOrigin() + visible_bounds.OffsetFromOrigin();
-    wm::ConvertPointToScreen(widget_->GetNativeWindow(), &new_widget_origin);
-    new_widget_bounds.set_origin(new_widget_origin);
+  switch (bounds_mode_) {
+    case BoundsMode::CLIENT:
+    case BoundsMode::FIXED:
+      // Position is relative to the origin.
+      new_widget_bounds += origin_.OffsetFromOrigin();
+      break;
+    case BoundsMode::SHELL:
+      // Update widget origin using the surface origin if the current location
+      // of surface is being anchored to one side of the widget as a result of a
+      // resize operation.
+      if (resize_component_ != HTCAPTION) {
+        gfx::Point widget_origin =
+            GetSurfaceOrigin() + visible_bounds.OffsetFromOrigin();
+        wm::ConvertPointToScreen(widget_->GetNativeWindow(), &widget_origin);
+        new_widget_bounds.set_origin(widget_origin);
+      } else {
+        // Preserve widget position.
+        new_widget_bounds.set_origin(
+            widget_->GetWindowBoundsInScreen().origin());
+      }
+      break;
   }
 
   // Set |ignore_window_bounds_changes_| as this change to window bounds
   // 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);
   ignore_window_bounds_changes_ = false;
+}
 
-  gfx::Rect client_view_bounds =
+void ShellSurface::UpdateSurfaceBounds() {
+  const gfx::Rect client_view_bounds =
       widget_->non_client_view()->frame_view()->GetBoundsForClientView();
 
-  // A change to the widget size requires surface bounds to be re-adjusted.
   surface_->window()->SetBounds(
       gfx::Rect(GetSurfaceOrigin() + client_view_bounds.OffsetFromOrigin(),
                 surface_->window()->layer()->size()));
 }
 
 void ShellSurface::UpdateShadow() {
   if (!widget_)
     return;
   aura::Window* window = widget_->GetNativeWindow();
   if (!shadow_enabled_) {
@@ skipping 118 matching lines @@
     // small style shadow for them.
     if (!activatable_)
       shadow->SetElevation(wm::ShadowElevation::SMALL);
     // We don't have rounded corners unless frame is enabled.
     if (!frame_enabled_)
       shadow->SetRoundedCornerRadius(0);
   }
 }
 
 }  // namespace exo