exo: Refactor ShellSurfaceBase::OnSurfaceCommit

This CL turns ShellSurfaceBase::OnSurfaceCommit into a template method
to isolate logic for ShellSurface and ClientControlledShellSurface.

Bug: b:77972398
Test: exo_unittests
Change-Id: Iad359c3754f9955fd34ea372a06e6d53999b8877
Reviewed-on: https://chromium-review.googlesource.com/1176182Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Dominik Laskowski <domlaskowski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583436}

components/exo/client_controlled_shell_surface.cc

@@ -577,74 +577,10 @@ void ClientControlledShellSurface::OnDragFinished(bool canceled,
   if (drag_finished_callback_)
     drag_finished_callback_.Run(location.x(), location.y(), canceled);
 }
+
 ////////////////////////////////////////////////////////////////////////////////
 // SurfaceDelegate overrides:
 
-void ClientControlledShellSurface::OnSurfaceCommit() {
-  if (!widget_) {
-    // Modify the |origin_| to the |pending_geometry_| to place the window
-    // on the intended display. See b/77472684 for the details.
-    if (!pending_geometry_.IsEmpty())
-      origin_ = pending_geometry_.origin();
-    CreateShellSurfaceWidget(ash::ToWindowShowState(pending_window_state_));
-  }
-
-  ash::wm::WindowState* window_state = GetWindowState();
-  if (window_state->GetStateType() != pending_window_state_) {
-    if (!IsPinned(window_state)) {
-      ash::wm::ClientControlledState::BoundsChangeAnimationType animation_type =
-          ash::wm::ClientControlledState::kAnimationNone;
-      switch (pending_window_state_) {
-        case ash::mojom::WindowStateType::NORMAL:
-          if (widget_->IsMaximized() || widget_->IsFullscreen()) {
-            animation_type =
-                ash::wm::ClientControlledState::kAnimationCrossFade;
-          }
-          break;
-
-        case ash::mojom::WindowStateType::MAXIMIZED:
-        case ash::mojom::WindowStateType::FULLSCREEN:
-          animation_type = ash::wm::ClientControlledState::kAnimationCrossFade;
-          break;
-
-        default:
-          break;
-      }
-      client_controlled_state_->EnterNextState(
-          window_state, pending_window_state_, animation_type);
-    } else {
-      VLOG(1) << "State change was requested while it is pinned";
-    }
-  }
-
-  ShellSurfaceBase::OnSurfaceCommit();
-  UpdateFrame();
-  UpdateBackdrop();
-
-  if (geometry_changed_callback_)
-    geometry_changed_callback_.Run(GetVisibleBounds());
-
-  // Apply new top inset height.
-  if (pending_top_inset_height_ != top_inset_height_) {
-    widget_->GetNativeWindow()->SetProperty(aura::client::kTopViewInset,
-                                            pending_top_inset_height_);
-    top_inset_height_ = pending_top_inset_height_;
-  }
-
-  // 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_);
-    host_window()->SetTransform(transform);
-    scale_ = pending_scale_;
-  }
-
-  orientation_ = pending_orientation_;
-  if (expected_orientation_ == orientation_)
-    orientation_compositor_lock_.reset();
-}
-
 bool ClientControlledShellSurface::IsInputEnabled(Surface* surface) const {
   // Client-driven dragging/resizing relies on implicit grab, which ensures that
   // mouse/touch events are delivered to the focused surface until release, even
@@ -795,7 +731,7 @@ void ClientControlledShellSurface::CompositorLockTimedOut() {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// ShellSurface overrides:
+// ShellSurfaceBase overrides:
 
 void ClientControlledShellSurface::SetWidgetBounds(const gfx::Rect& bounds) {
   if (((!client_controlled_move_resize_ && !GetWindowState()->is_dragged()) ||
@@ -912,6 +848,75 @@ gfx::Point ClientControlledShellSurface::GetSurfaceOrigin() const {
   return gfx::Point();
 }
 
+void ClientControlledShellSurface::OnPreWidgetCommit() {
+  if (!widget_) {
+    // Modify the |origin_| to the |pending_geometry_| to place the window on
+    // the intended display. See b/77472684 for details.
+    if (!pending_geometry_.IsEmpty())
+      origin_ = pending_geometry_.origin();
+    CreateShellSurfaceWidget(ash::ToWindowShowState(pending_window_state_));
+  }
+
+  ash::wm::WindowState* window_state = GetWindowState();
+  if (window_state->GetStateType() == pending_window_state_)
+    return;
+
+  if (IsPinned(window_state)) {
+    VLOG(1) << "State change was requested while pinned";
+    return;
+  }
+
+  auto animation_type = ash::wm::ClientControlledState::kAnimationNone;
+  switch (pending_window_state_) {
+    case ash::mojom::WindowStateType::NORMAL:
+      if (widget_->IsMaximized() || widget_->IsFullscreen()) {
+        animation_type = ash::wm::ClientControlledState::kAnimationCrossFade;
+      }
+      break;
+
+    case ash::mojom::WindowStateType::MAXIMIZED:
+    case ash::mojom::WindowStateType::FULLSCREEN:
+      animation_type = ash::wm::ClientControlledState::kAnimationCrossFade;
+      break;
+
+    default:
+      break;
+  }
+
+  client_controlled_state_->EnterNextState(window_state, pending_window_state_,
+                                           animation_type);
+}
+
+void ClientControlledShellSurface::OnPostWidgetCommit() {
+  DCHECK(widget_);
+
+  UpdateFrame();
+  UpdateBackdrop();
+
+  if (geometry_changed_callback_)
+    geometry_changed_callback_.Run(GetVisibleBounds());
+
+  // Apply new top inset height.
+  if (pending_top_inset_height_ != top_inset_height_) {
+    widget_->GetNativeWindow()->SetProperty(aura::client::kTopViewInset,
+                                            pending_top_inset_height_);
+    top_inset_height_ = pending_top_inset_height_;
+  }
+
+  // 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_);
+    host_window()->SetTransform(transform);
+    scale_ = pending_scale_;
+  }
+
+  orientation_ = pending_orientation_;
+  if (expected_orientation_ == orientation_)
+    orientation_compositor_lock_.reset();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // ClientControlledShellSurface, private:
 

components/exo/client_controlled_shell_surface.h

@@ -183,7 +183,6 @@ class ClientControlledShellSurface
   void SetOrientationLock(ash::OrientationLockType orientation_lock);
 
   // Overridden from SurfaceDelegate:
-  void OnSurfaceCommit() override;
   bool IsInputEnabled(Surface* surface) const override;
   void OnSetFrame(SurfaceFrameType type) override;
   void OnSetFrameColors(SkColor active_color, SkColor inactive_color) override;
@@ -230,13 +229,15 @@ class ClientControlledShellSurface
   class ScopedSetBoundsLocally;
   class ScopedLockedToRoot;
 
-  // Overridden from ShellSurface:
+  // Overridden from ShellSurfaceBase:
   void SetWidgetBounds(const gfx::Rect& bounds) override;
   gfx::Rect GetShadowBounds() const override;
   void InitializeWindowState(ash::wm::WindowState* window_state) override;
   float GetScale() const override;
   gfx::Rect GetWidgetBounds() const override;
   gfx::Point GetSurfaceOrigin() const override;
+  void OnPreWidgetCommit() override;
+  void OnPostWidgetCommit() override;
 
   // Update frame status. This may create (or destroy) a wide frame
   // that spans the full work area width if the surface didn't cover

components/exo/shell_surface.cc

@@ -236,6 +236,26 @@ void ShellSurface::OnPostWindowStateTypeChange(
   scoped_animations_disabled_.reset();
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ShellSurfaceBase overrides:
+
+void ShellSurface::OnPreWidgetCommit() {
+  if (!widget_ && enabled()) {
+    // Defer widget creation until surface contains some contents.
+    if (host_window()->bounds().IsEmpty()) {
+      Configure();
+      return;
+    }
+
+    CreateShellSurfaceWidget(ui::SHOW_STATE_NORMAL);
+  }
+}
+
+void ShellSurface::OnPostWidgetCommit() {}
+
+////////////////////////////////////////////////////////////////////////////////
+// ShellSurface, private:
+
 void ShellSurface::AttemptToStartDrag(int component) {
   ash::wm::WindowState* window_state =
       ash::wm::GetWindowState(widget_->GetNativeWindow());

components/exo/shell_surface.h

@@ -72,6 +72,10 @@ class ShellSurface : public ShellSurfaceBase,
  private:
   class ScopedAnimationsDisabled;
 
+  // Overridden from ShellSurfaceBase:
+  void OnPreWidgetCommit() override;
+  void OnPostWidgetCommit() override;
+
   void AttemptToStartDrag(int component);
 
   void EndDrag();

components/exo/shell_surface_base.cc

@@ -741,81 +741,11 @@ void ShellSurfaceBase::OnSurfaceCommit() {
 
   SurfaceTreeHost::OnSurfaceCommit();
 
-  if (enabled() && !widget_) {
-    // Defer widget creation until surface contains some contents.
-    if (host_window()->bounds().IsEmpty()) {
-      Configure();
-      return;
-    }
-
-    CreateShellSurfaceWidget(ui::SHOW_STATE_NORMAL);
-  }
-
-  // Apply the accumulated pending origin offset to reflect acknowledged
-  // configure requests.
-  origin_offset_ += pending_origin_offset_;
-  pending_origin_offset_ = gfx::Vector2d();
-
-  // Update resize direction to reflect acknowledged configure requests.
-  resize_component_ = pending_resize_component_;
-
-  // Apply new window geometry.
-  geometry_ = pending_geometry_;
-
-  // Apply new minimum/maximium size.
-  bool size_constraint_changed = minimum_size_ != pending_minimum_size_ ||
-                                 maximum_size_ != pending_maximum_size_;
-  minimum_size_ = pending_minimum_size_;
-  maximum_size_ = pending_maximum_size_;
-
-  if (widget_) {
-    UpdateWidgetBounds();
-    UpdateShadow();
-
-    // System modal container is used by clients to implement overlay
-    // windows using a single ShellSurface instance.  If hit-test
-    // region is empty, then it is non interactive window and won't be
-    // activated.
-    if (container_ == ash::kShellWindowId_SystemModalContainer) {
-      // Prevent window from being activated when hit test region is empty.
-      bool activatable = activatable_ && HasHitTestRegion();
-      if (activatable != CanActivate()) {
-        set_can_activate(activatable);
-        // Activate or deactivate window if activation state changed.
-        if (activatable) {
-          // Automatically activate only if the window is modal.
-          // Non modal window should be activated by a user action.
-          // TODO(oshima): Non modal system window does not have an associated
-          // task ID, and as a result, it cannot be activated from client side.
-          // Fix this (b/65460424) and remove this if condition.
-          if (system_modal_)
-            wm::ActivateWindow(widget_->GetNativeWindow());
-        } else if (widget_->IsActive()) {
-          wm::DeactivateWindow(widget_->GetNativeWindow());
-        }
-      }
-    }
-
-    UpdateSurfaceBounds();
-
-    // Show widget if needed.
-    if (pending_show_widget_) {
-      DCHECK(!widget_->IsClosed());
-      DCHECK(!widget_->IsVisible());
-      pending_show_widget_ = false;
-      widget_->Show();
-      if (has_grab_)
-        StartCapture();
-
-      if (container_ == ash::kShellWindowId_SystemModalContainer)
-        UpdateSystemModal();
-    }
-  }
+  OnPreWidgetCommit();
+  CommitWidget();
+  OnPostWidgetCommit();
 
   SubmitCompositorFrame();
-
-  if (size_constraint_changed)
-    widget_->OnSizeConstraintsChanged();
 }
 
 bool ShellSurfaceBase::IsInputEnabled(Surface*) const {
@@ -1505,4 +1435,71 @@ void ShellSurfaceBase::StartCapture() {
   widget_->SetCapture(nullptr /* view */);
 }
 
+void ShellSurfaceBase::CommitWidget() {
+  // Apply the accumulated pending origin offset to reflect acknowledged
+  // configure requests.
+  origin_offset_ += pending_origin_offset_;
+  pending_origin_offset_ = gfx::Vector2d();
+
+  // Update resize direction to reflect acknowledged configure requests.
+  resize_component_ = pending_resize_component_;
+
+  // Apply new window geometry.
+  geometry_ = pending_geometry_;
+
+  // Apply new minimum/maximium size.
+  bool size_constraint_changed = minimum_size_ != pending_minimum_size_ ||
+                                 maximum_size_ != pending_maximum_size_;
+  minimum_size_ = pending_minimum_size_;
+  maximum_size_ = pending_maximum_size_;
+
+  if (!widget_)
+    return;
+
+  UpdateWidgetBounds();
+  UpdateShadow();
+
+  // System modal container is used by clients to implement overlay
+  // windows using a single ShellSurface instance.  If hit-test
+  // region is empty, then it is non interactive window and won't be
+  // activated.
+  if (container_ == ash::kShellWindowId_SystemModalContainer) {
+    // Prevent window from being activated when hit test region is empty.
+    bool activatable = activatable_ && HasHitTestRegion();
+    if (activatable != CanActivate()) {
+      set_can_activate(activatable);
+      // Activate or deactivate window if activation state changed.
+      if (activatable) {
+        // Automatically activate only if the window is modal.
+        // Non modal window should be activated by a user action.
+        // TODO(oshima): Non modal system window does not have an associated
+        // task ID, and as a result, it cannot be activated from client side.
+        // Fix this (b/65460424) and remove this if condition.
+        if (system_modal_)
+          wm::ActivateWindow(widget_->GetNativeWindow());
+      } else if (widget_->IsActive()) {
+        wm::DeactivateWindow(widget_->GetNativeWindow());
+      }
+    }
+  }
+
+  UpdateSurfaceBounds();
+
+  // Show widget if needed.
+  if (pending_show_widget_) {
+    DCHECK(!widget_->IsClosed());
+    DCHECK(!widget_->IsVisible());
+    pending_show_widget_ = false;
+    widget_->Show();
+    if (has_grab_)
+      StartCapture();
+
+    if (container_ == ash::kShellWindowId_SystemModalContainer)
+      UpdateSystemModal();
+  }
+
+  if (size_constraint_changed)
+    widget_->OnSizeConstraintsChanged();
+}
+
 }  // namespace exo

components/exo/shell_surface_base.h

@@ -345,6 +345,11 @@ class ShellSurfaceBase : public SurfaceTreeHost,
   virtual gfx::Rect GetWidgetBounds() const;
   virtual gfx::Point GetSurfaceOrigin() const;
 
+  virtual void OnPreWidgetCommit() = 0;
+  virtual void OnPostWidgetCommit() = 0;
+
+  void CommitWidget();
+
   bool activatable_ = true;
   bool can_minimize_ = true;
   bool has_frame_colors_ = false;