exo: Improve window placement.

components/exo/shell_surface.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/shell_surface.h"
 
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #include "ash/wm/window_state.h"
 #include "base/logging.h"
@@ skipping 68 matching lines @@
 
 ShellSurface::~ShellSurface() {
   if (surface_) {
     surface_->SetSurfaceDelegate(nullptr);
     surface_->RemoveSurfaceObserver(this);
   }
   if (widget_)
     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())
     configure_callback_.Run(widget_->GetWindowBoundsInScreen().size());
 }
 
 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())
     configure_callback_.Run(widget_->GetWindowBoundsInScreen().size());
 }
 
 void ShellSurface::SetTitle(const base::string16& title) {
   TRACE_EVENT1("exo", "ShellSurface::SetTitle", "title",
                base::UTF16ToUTF8(title));
 
@@ skipping 53 matching lines @@
       new base::trace_event::TracedValue;
   value->SetString("title", base::UTF16ToUTF8(title_));
   value->SetString("application_id", application_id_);
   return value;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // SurfaceDelegate overrides:
 
 void ShellSurface::OnSurfaceCommit() {
+  if (enabled() && !widget_)
+    CreateShellSurfaceWidget();
+
   surface_->CommitSurfaceHierarchy();
+
   if (widget_) {
     // Update surface bounds and widget size.
     gfx::Point origin;
     views::View::ConvertPointToWidget(this, &origin);
     surface_->SetBounds(gfx::Rect(origin - geometry_.OffsetFromOrigin(),
                                   surface_->layer()->size()));
     widget_->SetSize(widget_->non_client_view()->GetPreferredSize());
 
     // Show widget if not already visible.
     if (!widget_->IsClosed() && !widget_->IsVisible())
@@ skipping 47 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // views::Views overrides:
 
 gfx::Size ShellSurface::GetPreferredSize() const {
   if (!geometry_.IsEmpty())
     return geometry_.size();
 
   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