exo: Improve window placement.

components/exo/wayland/server.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/wayland/server.h"
 
 #include <linux/input.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <wayland-server-core.h>
@@ skipping 620 matching lines @@
 
 void shell_surface_resize(wl_client* client,
                           wl_resource* resource,
                           wl_resource* seat_resource,
                           uint32_t serial,
                           uint32_t edges) {
   NOTIMPLEMENTED();
 }
 
 void shell_surface_set_toplevel(wl_client* client, wl_resource* resource) {
-  GetUserDataAs<ShellSurface>(resource)->Init();
+  ShellSurface* shell_surface = GetUserDataAs<ShellSurface>(resource);
+
+  // Early out if shell surface has already been mapped.
+  if (shell_surface->enabled())
+    return;

[lpique, 2016/01/28 20:48:32]

Note: SetEnabled() basically does this same early out check (DRY).

[reveman, 2016/01/28 21:35:19]

Good point. Changed this to unconditionally call SetEnabled(true).

+
+  shell_surface->SetEnabled(true);
 }
 
 void shell_surface_set_transient(wl_client* client,
                                  wl_resource* resource,
                                  wl_resource* parent_resource,
                                  int x,
                                  int y,
                                  uint32_t flags) {
   NOTIMPLEMENTED();
 }
 
 void shell_surface_set_fullscreen(wl_client* client,
                                   wl_resource* resource,
                                   uint32_t method,
                                   uint32_t framerate,
                                   wl_resource* output_resource) {
-  GetUserDataAs<ShellSurface>(resource)->Init();
-  GetUserDataAs<ShellSurface>(resource)->SetFullscreen(true);
+  ShellSurface* shell_surface = GetUserDataAs<ShellSurface>(resource);
+
+  // Early out if shell surface has already been mapped.
+  if (shell_surface->enabled())
+    return;

[lpique, 2016/01/28 20:48:32]

If you remove this new check here or below (as above), note that
SetFullscreen/Maximize() do not check that if it is already full screen, though
this means that it is no worse than the code prior to this change.

[reveman, 2016/01/28 21:35:20]

Correct. It shouldn't be possible to change the state of a wl_shell_surface once
mapped if I understand the interface correctly but I changed the code to match
the old behavior and if we want to improve it we can instead do that in a
separate patch.

+
+  shell_surface->SetEnabled(true);
+  shell_surface->SetFullscreen(true);
 }
 
 void shell_surface_set_popup(wl_client* client,
                              wl_resource* resource,
                              wl_resource* seat_resource,
                              uint32_t serial,
                              wl_resource* parent_resource,
                              int32_t x,
                              int32_t y,
                              uint32_t flags) {
   NOTIMPLEMENTED();
 }
 
 void shell_surface_set_maximized(wl_client* client,
                                  wl_resource* resource,
                                  wl_resource* output_resource) {
-  GetUserDataAs<ShellSurface>(resource)->Init();
-  GetUserDataAs<ShellSurface>(resource)->Maximize();
+  ShellSurface* shell_surface = GetUserDataAs<ShellSurface>(resource);
+
+  // Early out if shell surface has already been mapped.
+  if (shell_surface->enabled())
+    return;
+
+  shell_surface->SetEnabled(true);
+  shell_surface->Maximize();
 }
 
 void shell_surface_set_title(wl_client* client,
                              wl_resource* resource,
                              const char* title) {
   GetUserDataAs<ShellSurface>(resource)
       ->SetTitle(base::string16(base::UTF8ToUTF16(title)));
 }
 
 void shell_surface_set_class(wl_client* client,
@@ skipping 30 matching lines @@
     return;
   }
 
   wl_resource* shell_surface_resource =
       wl_resource_create(client, &wl_shell_surface_interface, 1, id);
   if (!shell_surface_resource) {
     wl_resource_post_no_memory(resource);
     return;
   }
 
+  // Shell surfaces are initially disabled and needs to be explicitly mapped
+  // before they are enabled and can become visible.
+  shell_surface->SetEnabled(false);
+
   shell_surface->set_surface_destroyed_callback(base::Bind(
       &wl_resource_destroy, base::Unretained(shell_surface_resource)));
 
   shell_surface->set_configure_callback(
       base::Bind(&HandleShellSurfaceConfigureCallback,
                  base::Unretained(shell_surface_resource)));
 
   SetImplementation(shell_surface_resource, &shell_surface_implementation,
                     std::move(shell_surface));
 }
@@ skipping 207 matching lines @@
     return;
   }
 
   wl_resource* xdg_surface_resource =
       wl_resource_create(client, &xdg_surface_interface, 1, id);
   if (!xdg_surface_resource) {
     wl_resource_post_no_memory(resource);
     return;
   }
 
-  // An XdgSurface is a toplevel shell surface.
-  shell_surface->Init();
-
   shell_surface->set_close_callback(base::Bind(
       &xdg_surface_send_close, base::Unretained(xdg_surface_resource)));
 
   shell_surface->set_configure_callback(
       base::Bind(&HandleXdgSurfaceConfigureCallback,
                  base::Unretained(xdg_surface_resource)));
 
   SetImplementation(xdg_surface_resource, &xdg_surface_implementation,
                     std::move(shell_surface));
 }
@@ skipping 571 matching lines @@
   DCHECK(event_loop);
   wl_event_loop_dispatch(event_loop, timeout.InMilliseconds());
 }
 
 void Server::Flush() {
   wl_display_flush_clients(wl_display_.get());
 }
 
 }  // namespace wayland
 }  // namespace exo