exo: Do not commit if widget creation is deferred

Bug: 877254
Test: gtk3-demo
Test: exo_unittests
Change-Id: Iec316067589088e85dec3429df4fe9f5a5604a65
Reviewed-on: https://chromium-review.googlesource.com/1188343Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Dominik Laskowski <domlaskowski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586116}

components/exo/client_controlled_shell_surface.cc

@@ -846,7 +846,7 @@ gfx::Point ClientControlledShellSurface::GetSurfaceOrigin() const {
   return gfx::Point();
 }
 
-void ClientControlledShellSurface::OnPreWidgetCommit() {
+bool ClientControlledShellSurface::OnPreWidgetCommit() {
   if (!widget_) {
     // Modify the |origin_| to the |pending_geometry_| to place the window on
     // the intended display. See b/77472684 for details.
@@ -857,11 +857,11 @@ void ClientControlledShellSurface::OnPreWidgetCommit() {
 
   ash::wm::WindowState* window_state = GetWindowState();
   if (window_state->GetStateType() == pending_window_state_)
-    return;
+    return true;
 
   if (IsPinned(window_state)) {
     VLOG(1) << "State change was requested while pinned";
-    return;
+    return true;
   }
 
   auto animation_type = ash::wm::ClientControlledState::kAnimationNone;
@@ -897,6 +897,7 @@ void ClientControlledShellSurface::OnPreWidgetCommit() {
 
   client_controlled_state_->EnterNextState(window_state, pending_window_state_,
                                            animation_type);
+  return true;
 }
 
 void ClientControlledShellSurface::OnPostWidgetCommit() {

components/exo/client_controlled_shell_surface.h

@@ -232,7 +232,7 @@ class ClientControlledShellSurface
   float GetScale() const override;
   base::Optional<gfx::Rect> GetWidgetBounds() const override;
   gfx::Point GetSurfaceOrigin() const override;
-  void OnPreWidgetCommit() override;
+  bool OnPreWidgetCommit() override;
   void OnPostWidgetCommit() override;
 
   // Update frame status. This may create (or destroy) a wide frame

components/exo/shell_surface.cc

@@ -463,12 +463,12 @@ void ShellSurface::SetWidgetBounds(const gfx::Rect& bounds) {
   ignore_window_bounds_changes_ = false;
 }
 
-void ShellSurface::OnPreWidgetCommit() {
+bool ShellSurface::OnPreWidgetCommit() {
   if (!widget_ && enabled()) {
-    // Defer widget creation until surface contains some contents.
+    // Defer widget creation and commit until surface has contents.
     if (host_window()->bounds().IsEmpty()) {
       Configure();
-      return;
+      return false;
     }
 
     CreateShellSurfaceWidget(ui::SHOW_STATE_NORMAL);
@@ -481,6 +481,8 @@ void ShellSurface::OnPreWidgetCommit() {
 
   // Update resize direction to reflect acknowledged configure requests.
   resize_component_ = pending_resize_component_;
+
+  return true;
 }
 
 void ShellSurface::OnPostWidgetCommit() {}

components/exo/shell_surface.h

@@ -136,7 +136,7 @@ class ShellSurface : public ShellSurfaceBase,
 
   // Overridden from ShellSurfaceBase:
   void SetWidgetBounds(const gfx::Rect& bounds) override;
-  void OnPreWidgetCommit() override;
+  bool OnPreWidgetCommit() override;
   void OnPostWidgetCommit() override;
 
   // Asks the client to configure its surface.

components/exo/shell_surface_base.cc

@@ -658,7 +658,9 @@ void ShellSurfaceBase::OnSurfaceCommit() {
 
   SurfaceTreeHost::OnSurfaceCommit();
 
-  OnPreWidgetCommit();
+  if (!OnPreWidgetCommit())
+    return;
+
   CommitWidget();
   OnPostWidgetCommit();
 

components/exo/shell_surface_base.h

@@ -281,7 +281,8 @@ class ShellSurfaceBase : public SurfaceTreeHost,
   virtual base::Optional<gfx::Rect> GetWidgetBounds() const = 0;
   virtual gfx::Point GetSurfaceOrigin() const = 0;
 
-  virtual void OnPreWidgetCommit() = 0;
+  // Commit is deferred if this returns false.
+  virtual bool OnPreWidgetCommit() = 0;
   virtual void OnPostWidgetCommit() = 0;
 
   void CommitWidget();

components/exo/shell_surface_unittest.cc

@@ -473,11 +473,18 @@ TEST_F(ShellSurfaceTest, ConfigureCallback) {
       base::Bind(&Configure, base::Unretained(&suggested_size),
                  base::Unretained(&has_state_type),
                  base::Unretained(&is_resizing), base::Unretained(&is_active)));
+
+  gfx::Rect geometry(16, 16, 32, 32);
+  shell_surface->SetGeometry(geometry);
+
   // Commit without contents should result in a configure callback with empty
   // suggested size as a mechanims to ask the client size itself.
   surface->Commit();
   EXPECT_EQ(gfx::Size(), suggested_size);
 
+  // Geometry should not be committed until surface has contents.
+  EXPECT_EQ(gfx::Size(), shell_surface->CalculatePreferredSize());
+
   shell_surface->Maximize();
   shell_surface->AcknowledgeConfigure(0);
   EXPECT_EQ(CurrentContext()->bounds().width(), suggested_size.width());
@@ -498,6 +505,9 @@ TEST_F(ShellSurfaceTest, ConfigureCallback) {
       new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
   surface->Attach(buffer.get());
   surface->Commit();
+
+  EXPECT_EQ(geometry.size(), shell_surface->CalculatePreferredSize());
+
   shell_surface->GetWidget()->Activate();
   shell_surface->AcknowledgeConfigure(0);
   EXPECT_TRUE(is_active);