Update host's window bounds after the widget bounds are updated

Fixes buggy restore animation

BUG=955981
TEST=
Manual: maximize Play Store app then click restore: animation is done correctly
Unit tests: exo_unittests

Change-Id: I4a43b7f52e589603d20768bc4e8b30cb0d07dad4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1634418
Commit-Queue: Jorge Gil <jorgegil@google.com>
Auto-Submit: Jorge Gil <jorgegil@google.com>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671756}

components/exo/shell_surface.cc

@@ -461,6 +461,23 @@ void ShellSurface::OnPostWindowStateTypeChange(
   scoped_animations_disabled_.reset();
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// wm::ActivationChangeObserver overrides:
+
+void ShellSurface::OnWindowActivated(ActivationReason reason,
+                                     aura::Window* gained_active,
+                                     aura::Window* lost_active) {
+  ShellSurfaceBase::OnWindowActivated(reason, gained_active, lost_active);
+
+  if (!widget_)
+    return;
+
+  if (gained_active == widget_->GetNativeWindow() ||
+      lost_active == widget_->GetNativeWindow()) {
+    Configure();
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // ShellSurfaceBase overrides:
 
@@ -482,7 +499,8 @@ void ShellSurface::SetWidgetBounds(const gfx::Rect& bounds) {
 bool ShellSurface::OnPreWidgetCommit() {
   if (!widget_ && GetEnabled()) {
     // Defer widget creation and commit until surface has contents.
-    if (host_window()->bounds().IsEmpty()) {
+    if (host_window()->bounds().IsEmpty() &&
+        root_surface()->surface_hierarchy_content_bounds().IsEmpty()) {
       Configure();
       return false;
     }
@@ -503,23 +521,6 @@ bool ShellSurface::OnPreWidgetCommit() {
 
 void ShellSurface::OnPostWidgetCommit() {}
 
-////////////////////////////////////////////////////////////////////////////////
-// wm::ActivationChangeObserver overrides:
-
-void ShellSurface::OnWindowActivated(ActivationReason reason,
-                                     aura::Window* gained_active,
-                                     aura::Window* lost_active) {
-  ShellSurfaceBase::OnWindowActivated(reason, gained_active, lost_active);
-
-  if (!widget_)
-    return;
-
-  if (gained_active == widget_->GetNativeWindow() ||
-      lost_active == widget_->GetNativeWindow()) {
-    Configure();
-  }
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // ShellSurface, private:
 

components/exo/shell_surface.h

@@ -107,6 +107,11 @@ class ShellSurface : public ShellSurfaceBase,
                          aura::Window* gained_active,
                          aura::Window* lost_active) override;
 
+  // Overridden from ShellSurfaceBase:
+  void SetWidgetBounds(const gfx::Rect& bounds) override;
+  bool OnPreWidgetCommit() override;
+  void OnPostWidgetCommit() override;
+
  private:
   class ScopedAnimationsDisabled;
   struct Config;
@@ -132,11 +137,6 @@ class ShellSurface : public ShellSurfaceBase,
     DISALLOW_COPY_AND_ASSIGN(ScopedConfigure);
   };
 
-  // Overridden from ShellSurfaceBase:
-  void SetWidgetBounds(const gfx::Rect& bounds) override;
-  bool OnPreWidgetCommit() override;
-  void OnPostWidgetCommit() override;
-
   // Set the parent window of this surface.
   void SetParentWindow(aura::Window* parent);
 

components/exo/shell_surface_base.cc

@@ -546,7 +546,8 @@ void ShellSurfaceBase::OnSurfaceCommit() {
   if (shadow_bounds_changed_)
     host_window()->AllocateLocalSurfaceId();
 
-  SurfaceTreeHost::OnSurfaceCommit();
+  DCHECK(presentation_callbacks().empty());
+  root_surface()->CommitSurfaceHierarchy(false);
 
   if (!OnPreWidgetCommit())
     return;
@@ -773,7 +774,12 @@ gfx::Size ShellSurfaceBase::CalculatePreferredSize() const {
   if (!geometry_.IsEmpty())
     return geometry_.size();
 
-  return host_window()->bounds().size();
+  // The root surface's content bounds should be used instead of the host window
+  // bounds because the host window bounds are not updated until the widget is
+  // committed, meaning that if we need to calculate the preferred size before
+  // then (e.g. in OnPreWidgetCommit()), then we need to use the root surface's
+  // to ensure that we're using the correct bounds' size.
+  return root_surface()->surface_hierarchy_content_bounds().size();
 }
 
 gfx::Size ShellSurfaceBase::GetMinimumSize() const {
@@ -1108,6 +1114,7 @@ void ShellSurfaceBase::CommitWidget() {
   }
 
   UpdateWidgetBounds();
+  SurfaceTreeHost::UpdateHostWindowBounds();
   UpdateShadow();
 
   // System modal container is used by clients to implement overlay

components/exo/shell_surface_base.h

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "ash/display/window_tree_host_manager.h"
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/optional.h"
 #include "base/strings/string16.h"
@@ -254,6 +255,9 @@ class ShellSurfaceBase : public SurfaceTreeHost,
   bool has_grab_ = false;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(ShellSurfaceTest,
+                           HostWindowBoundsUpdatedAfterCommitWidget);
+
   // Called on widget creation to initialize its window state.
   // TODO(reveman): Remove virtual functions below to avoid FBC problem.
   virtual void InitializeWindowState(ash::wm::WindowState* window_state) = 0;

components/exo/shell_surface_unittest.cc

@@ -42,7 +42,6 @@
 #include "ui/wm/core/window_util.h"
 
 namespace exo {
-namespace {
 
 using ShellSurfaceTest = test::ExoTestBase;
 
@@ -232,6 +231,26 @@ TEST_F(ShellSurfaceTest, Restore) {
       shell_surface->GetWidget()->GetWindowBoundsInScreen().size().ToString());
 }
 
+TEST_F(ShellSurfaceTest, HostWindowBoundsUpdatedAfterCommitWidget) {
+  gfx::Size buffer_size(256, 256);
+  std::unique_ptr<Buffer> buffer(
+      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
+  std::unique_ptr<Surface> surface(new Surface);
+  std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(surface.get()));
+
+  surface->Attach(buffer.get());
+  shell_surface->SurfaceTreeHost::OnSurfaceCommit();
+  shell_surface->root_surface()->SetSurfaceHierarchyContentBoundsForTest(
+      gfx::Rect(0, 0, 50, 50));
+
+  // Host Window Bounds size before committing.
+  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), shell_surface->host_window()->bounds());
+  EXPECT_TRUE(shell_surface->OnPreWidgetCommit());
+  shell_surface->CommitWidget();
+  // CommitWidget should update the Host Window Bounds.
+  EXPECT_EQ(gfx::Rect(0, 0, 50, 50), shell_surface->host_window()->bounds());
+}
+
 TEST_F(ShellSurfaceTest, SetFullscreen) {
   gfx::Size buffer_size(256, 256);
   std::unique_ptr<Buffer> buffer(
@@ -783,5 +802,4 @@ TEST_F(ShellSurfaceTest, Popup) {
   }
 }
 
-}  // namespace
 }  // namespace exo

components/exo/surface.cc

@@ -804,6 +804,11 @@ void Surface::SetOcclusionTracking(bool tracking) {
     window()->TrackOcclusionState();
 }
 
+void Surface::SetSurfaceHierarchyContentBoundsForTest(
+    const gfx::Rect& content_bounds) {
+  surface_hierarchy_content_bounds_ = content_bounds;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Buffer, private:
 

components/exo/surface.h

@@ -266,6 +266,9 @@ class Surface final : public ui::PropertyHandler {
   // True if the window for this surface has its occlusion tracked.
   bool is_tracking_occlusion() const { return is_tracking_occlusion_; }
 
+  // Sets the |surface_hierarchy_content_bounds_|.
+  void SetSurfaceHierarchyContentBoundsForTest(const gfx::Rect& content_bounds);
+
  private:
   struct State {
     State();

components/exo/surface_tree_host.cc

@@ -260,9 +260,6 @@ void SurfaceTreeHost::SubmitEmptyCompositorFrame() {
   layer_tree_frame_sink_holder_->SubmitCompositorFrame(std::move(frame));
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// SurfaceTreeHost, private:
-
 void SurfaceTreeHost::UpdateHostWindowBounds() {
   // This method applies multiple changes to the window tree. Use ScopedPause
   // to ensure that occlusion isn't recomputed before all changes have been
@@ -282,6 +279,9 @@ void SurfaceTreeHost::UpdateHostWindowBounds() {
       root_surface_origin_, root_surface_->window()->bounds().size()));
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// SurfaceTreeHost, private:
+
 viz::CompositorFrame SurfaceTreeHost::PrepareToSubmitCompositorFrame() {
   DCHECK(root_surface_);
 

components/exo/surface_tree_host.h

@@ -62,6 +62,11 @@ class SurfaceTreeHost : public SurfaceDelegate,
   }
 
   using PresentationCallbacks = std::list<Surface::PresentationCallback>;
+
+  const PresentationCallbacks& presentation_callbacks() const {
+    return presentation_callbacks_;
+  }
+
   base::flat_map<uint32_t, PresentationCallbacks>&
   GetActivePresentationCallbacksForTesting() {
     return active_presentation_callbacks_;
@@ -90,8 +95,10 @@ class SurfaceTreeHost : public SurfaceDelegate,
   // need to be released back to the client.
   void SubmitEmptyCompositorFrame();
 
- private:
+  // Update the host window's size to cover entire surfaces.
   void UpdateHostWindowBounds();
+
+ private:
   viz::CompositorFrame PrepareToSubmitCompositorFrame();
 
   Surface* root_surface_ = nullptr;