exo: Improve window placement.

components/exo/shell_surface.cc

Index: components/exo/shell_surface.cc
diff --git a/components/exo/shell_surface.cc b/components/exo/shell_surface.cc
index efe73aa6ee60588c1562c6002cae3d85baf32b94..086e56c303e6318ed3c1e75ea371852ae667b32f 100644
--- a/components/exo/shell_surface.cc
+++ b/components/exo/shell_surface.cc
@@ -86,39 +86,14 @@ ShellSurface::~ShellSurface() {
     widget_->CloseNow();
 }
 
-void ShellSurface::Init() {
-  TRACE_EVENT0("exo", "ShellSurface::Init");
-
-  if (widget_) {
-    DLOG(WARNING) << "Shell surface already initialized";
-    return;
-  }
-
-  views::Widget::InitParams params;
-  params.type = views::Widget::InitParams::TYPE_WINDOW;
-  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
-  params.delegate = this;
-  params.shadow_type = views::Widget::InitParams::SHADOW_TYPE_NONE;
-  params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
-  params.show_state = ui::SHOW_STATE_NORMAL;
-  params.parent = ash::Shell::GetContainer(
-      ash::Shell::GetPrimaryRootWindow(), ash::kShellWindowId_DefaultContainer);
-  widget_.reset(new ShellSurfaceWidget(this));
-  widget_->Init(params);
-  widget_->GetNativeWindow()->set_owned_by_parent(false);
-  widget_->GetNativeWindow()->SetName("ExoShellSurface");
-  widget_->GetNativeWindow()->AddChild(surface_);
-  SetApplicationId(widget_->GetNativeWindow(), &application_id_);
-
-  // The position of a top-level shell surface is managed by Ash.
-  ash::wm::GetWindowState(widget_->GetNativeWindow())
-      ->set_window_position_managed(true);
-}
-
 void ShellSurface::Maximize() {
   TRACE_EVENT0("exo", "ShellSurface::Maximize");
 
-  DCHECK(widget_);
+  DCHECK(enabled());

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

Should this be a precondition check inside CreateShellSurfaceWidget() instead?

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

Good idea. Done.

+
+  if (!widget_)
+    CreateShellSurfaceWidget();
+
   widget_->Maximize();
 
   if (!configure_callback_.is_null())
@@ -128,7 +103,11 @@ void ShellSurface::Maximize() {
 void ShellSurface::SetFullscreen(bool fullscreen) {
   TRACE_EVENT1("exo", "ShellSurface::SetFullscreen", "fullscreen", fullscreen);
 
-  DCHECK(widget_);
+  DCHECK(enabled());
+
+  if (!widget_)
+    CreateShellSurfaceWidget();
+
   widget_->SetFullscreen(fullscreen);
 
   if (!configure_callback_.is_null())
@@ -202,7 +181,11 @@ scoped_refptr<base::trace_event::TracedValue> ShellSurface::AsTracedValue()
 // SurfaceDelegate overrides:
 
 void ShellSurface::OnSurfaceCommit() {
+  if (enabled() && !widget_)
+    CreateShellSurfaceWidget();
+
   surface_->CommitSurfaceHierarchy();
+
   if (widget_) {
     // Update surface bounds and widget size.
     gfx::Point origin;
@@ -270,4 +253,31 @@ gfx::Size ShellSurface::GetPreferredSize() const {
   return surface_ ? surface_->GetPreferredSize() : gfx::Size();
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ShellSurface, private:
+
+void ShellSurface::CreateShellSurfaceWidget() {
+  DCHECK(!widget_);
+

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

Was dropping the TRACE_EVENT intentional?

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

Yes, I've been limiting TRACE_EVENT statements to public API entry points and
code that is known to take a significant time to execute. FYI, the first is
useful in combination with --trace-to-console=exo when debugging clients.

+  views::Widget::InitParams params;
+  params.type = views::Widget::InitParams::TYPE_WINDOW;
+  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.delegate = this;
+  params.shadow_type = views::Widget::InitParams::SHADOW_TYPE_NONE;
+  params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
+  params.show_state = ui::SHOW_STATE_NORMAL;
+  params.parent = ash::Shell::GetContainer(
+      ash::Shell::GetPrimaryRootWindow(), ash::kShellWindowId_DefaultContainer);
+  widget_.reset(new ShellSurfaceWidget(this));
+  widget_->Init(params);
+  widget_->GetNativeWindow()->set_owned_by_parent(false);
+  widget_->GetNativeWindow()->SetName("ExoShellSurface");
+  widget_->GetNativeWindow()->AddChild(surface_);
+  SetApplicationId(widget_->GetNativeWindow(), &application_id_);
+
+  // The position of a top-level shell surface is managed by Ash.
+  ash::wm::GetWindowState(widget_->GetNativeWindow())
+      ->set_window_position_managed(true);
+}
+
 }  // namespace exo