Create windows for new app window bounds API

apps/app_window.cc

Index: apps/app_window.cc
diff --git a/apps/app_window.cc b/apps/app_window.cc
index 57047b2819cc534ac9068f3b6899f2c1eea90779..5e515a9d1bb4f2249390bfc49e8f4898cd2ecfab 100644
--- a/apps/app_window.cc
+++ b/apps/app_window.cc
@@ -4,6 +4,8 @@
 
 #include "apps/app_window.h"
 
+#include <algorithm>

[benwells, 2014/03/05 02:33:30]

Is this needed?

[tmdiep, 2014/03/05 03:06:47]

Apparently it is if you use std::min/max on VS2013. Just being paranoid because
someone had to add it for one of my other patches.

+
 #include "apps/app_window_geometry_cache.h"
 #include "apps/app_window_registry.h"
 #include "apps/apps_client.h"
@@ -95,14 +97,62 @@ void SetBoundsProperties(const gfx::Rect& bounds,
   window_properties->Set(bounds_name, bounds_properties.release());
 }
 
+// Combines the constraints of the content and window, and returns constraints
+// for the window.
+gfx::Size GetCombinedWindowConstraints(const gfx::Size& window_size,

[benwells, 2014/03/05 02:33:30]

Nit: I think window_size and content_size should be window_constraints and
content_constraints.

+                                       const gfx::Size& content_size,
+                                       const gfx::Insets& frame_insets) {
+  gfx::Size combined_size(window_size);
+  if (content_size.width() > 0)
+    combined_size.set_width(content_size.width() + frame_insets.width());
+  if (content_size.height() > 0)
+    combined_size.set_height(content_size.height() + frame_insets.height());
+  return combined_size;
+}
+
+// Combines the constraints of the content and window, and returns constraints
+// for the content.
+gfx::Size GetCombinedContentConstraints(const gfx::Size& window_size,
+                                        const gfx::Size& content_size,

[benwells, 2014/03/05 02:33:30]

Same here

+                                        const gfx::Insets& frame_insets) {
+  gfx::Size combined_size(content_size);
+  if (window_size.width() > 0) {
+    combined_size.set_width(
+        std::max(0, window_size.width() - frame_insets.width()));
+  }
+  if (window_size.height() > 0) {
+    combined_size.set_height(
+        std::max(0, window_size.height() - frame_insets.height()));
+  }
+  return combined_size;
+}
+
 }  // namespace
 
+// AppWindow::BoundsSpecification
+
+// static

[tapted, 2014/03/05 03:14:03]

nit: remove - Don't usually see `// static` for data members (I guess it's
redundant)

+const int AppWindow::BoundsSpecification::kUnspecifiedPosition = INT_MIN;
+
+AppWindow::BoundsSpecification::BoundsSpecification()
+    : bounds(kUnspecifiedPosition, kUnspecifiedPosition, 0, 0) {}
+
+void AppWindow::BoundsSpecification::ResetBounds() {
+  bounds.set_x(kUnspecifiedPosition);

[tapted, 2014/03/05 03:14:03]

nit: bounds.SetRect(kUnspecifiedPosition, kUnspecifiedPosition, 0, 0);

+  bounds.set_y(kUnspecifiedPosition);
+  bounds.set_width(0);
+  bounds.set_height(0);
+}
+
+AppWindow::BoundsSpecification::~BoundsSpecification() {}

[tapted, 2014/03/05 03:14:03]

nit: move up below constructor

+
+// AppWindow::CreateParams
+
 AppWindow::CreateParams::CreateParams()
     : window_type(AppWindow::WINDOW_TYPE_DEFAULT),
       frame(AppWindow::FRAME_CHROME),
       has_frame_color(false),
       transparent_background(false),
-      bounds(INT_MIN, INT_MIN, 0, 0),
       creator_process_id(0),
       state(ui::SHOW_STATE_DEFAULT),
       hidden(false),
@@ -112,8 +162,68 @@ AppWindow::CreateParams::CreateParams()
 
 AppWindow::CreateParams::~CreateParams() {}
 
+gfx::Rect AppWindow::CreateParams::GetInitialWindowBounds(
+    const gfx::Insets& frame_insets) const {
+  // Combine into a single window bounds.
+  gfx::Rect combined_bounds(window_spec.bounds);
+  if (content_spec.bounds.x() != BoundsSpecification::kUnspecifiedPosition)
+    combined_bounds.set_x(content_spec.bounds.x() - frame_insets.left());
+  if (content_spec.bounds.y() != BoundsSpecification::kUnspecifiedPosition)
+    combined_bounds.set_y(content_spec.bounds.y() - frame_insets.top());
+  if (content_spec.bounds.width() > 0) {
+    combined_bounds.set_width(
+        content_spec.bounds.width() + frame_insets.width());
+  }
+  if (content_spec.bounds.height() > 0) {
+    combined_bounds.set_height(
+        content_spec.bounds.height() + frame_insets.height());
+  }
+
+  // Constrain the bounds.
+  SizeConstraints constraints(
+      GetCombinedWindowConstraints(
+          window_spec.minimum_size, content_spec.minimum_size, frame_insets),
+      GetCombinedWindowConstraints(
+          window_spec.maximum_size, content_spec.maximum_size, frame_insets));
+  combined_bounds.set_size(constraints.ClampSize(combined_bounds.size()));
+
+  return combined_bounds;
+}
+
+gfx::Size AppWindow::CreateParams::GetContentMinimumSize(
+    const gfx::Insets& frame_insets) const {
+  return GetCombinedContentConstraints(window_spec.minimum_size,
+                                       content_spec.minimum_size,
+                                       frame_insets);
+}
+
+gfx::Size AppWindow::CreateParams::GetContentMaximumSize(
+    const gfx::Insets& frame_insets) const {
+  return GetCombinedContentConstraints(window_spec.maximum_size,
+                                       content_spec.maximum_size,
+                                       frame_insets);
+}
+
+gfx::Size AppWindow::CreateParams::GetWindowMinimumSize(
+    const gfx::Insets& frame_insets) const {
+  return GetCombinedWindowConstraints(window_spec.minimum_size,
+                                      content_spec.minimum_size,
+                                      frame_insets);
+}
+
+gfx::Size AppWindow::CreateParams::GetWindowMaximumSize(
+    const gfx::Insets& frame_insets) const {
+  return GetCombinedWindowConstraints(window_spec.maximum_size,
+                                      content_spec.maximum_size,
+                                      frame_insets);
+}
+
+// AppWindow::Delegate
+
 AppWindow::Delegate::~Delegate() {}
 
+// AppWindow
+
 AppWindow::AppWindow(BrowserContext* context,
                      Delegate* delegate,
                      const extensions::Extension* extension)
@@ -154,7 +264,7 @@ void AppWindow::Init(const GURL& url,
   extensions::SetViewType(web_contents, extensions::VIEW_TYPE_APP_WINDOW);
 
   // Initialize the window
-  CreateParams new_params = LoadDefaultsAndConstrain(params);
+  CreateParams new_params = LoadDefaults(params);
   window_type_ = new_params.window_type;
   window_key_ = new_params.window_key;
 
@@ -208,7 +318,10 @@ void AppWindow::Init(const GURL& url,
     // that to happen, we need to define a size for the content, otherwise the
     // layout will happen in a 0x0 area.
     // Note: WebContents::GetView() is guaranteed to be non-null.
-    web_contents->GetView()->SizeContents(new_params.bounds.size());
+    gfx::Insets frame_insets = native_app_window_->GetFrameInsets();
+    gfx::Rect initial_bounds = new_params.GetInitialWindowBounds(frame_insets);
+    initial_bounds.Inset(frame_insets);
+    web_contents->GetView()->SizeContents(initial_bounds.size());
   }
 
   // Prevent the browser process from shutting down while this window is open.
@@ -519,13 +632,13 @@ void AppWindow::ForcedFullscreen() {
   SetNativeWindowFullscreen();
 }
 
-void AppWindow::SetMinimumSize(const gfx::Size& min_size) {
-  native_app_window_->SetMinimumSize(min_size);
+void AppWindow::SetContentMinimumSize(const gfx::Size& min_size) {
+  native_app_window_->SetContentMinimumSize(min_size);
   OnSizeConstraintsChanged();
 }
 
-void AppWindow::SetMaximumSize(const gfx::Size& max_size) {
-  native_app_window_->SetMaximumSize(max_size);
+void AppWindow::SetContentMaximumSize(const gfx::Size& max_size) {
+  native_app_window_->SetContentMaximumSize(max_size);
   OnSizeConstraintsChanged();
 }
 
@@ -588,17 +701,23 @@ void AppWindow::GetSerializedState(base::DictionaryValue* properties) const {
   properties->SetInteger("frameColor", native_app_window_->FrameColor());
 
   gfx::Rect content_bounds = GetClientBounds();
+  gfx::Size content_min_size = native_app_window_->GetContentMinimumSize();
+  gfx::Size content_max_size = native_app_window_->GetContentMaximumSize();
   SetBoundsProperties(content_bounds,
-                      native_app_window_->GetMinimumSize(),
-                      native_app_window_->GetMaximumSize(),
+                      content_min_size,
+                      content_max_size,
                       "innerBounds",
                       properties);
 
-  // TODO(tmdiep): Frame constraints will be implemented in a future patch.
+  gfx::Insets frame_insets = native_app_window_->GetFrameInsets();
   gfx::Rect frame_bounds = native_app_window_->GetBounds();
+  gfx::Size frame_min_size =
+      SizeConstraints::InsetConstraints(content_min_size, -frame_insets);
+  gfx::Size frame_max_size =
+      SizeConstraints::InsetConstraints(content_max_size, -frame_insets);
   SetBoundsProperties(frame_bounds,
-                      gfx::Size(),
-                      gfx::Size(),
+                      frame_min_size,
+                      frame_max_size,
                       "outerBounds",
                       properties);
 }
@@ -663,12 +782,13 @@ void AppWindow::UpdateExtensionAppIcon() {
 }
 
 void AppWindow::OnSizeConstraintsChanged() {
-  SizeConstraints size_constraints(native_app_window_->GetMinimumSize(),
-                                   native_app_window_->GetMaximumSize());
+  SizeConstraints size_constraints(native_app_window_->GetContentMinimumSize(),
+                                   native_app_window_->GetContentMaximumSize());
   gfx::Rect bounds = GetClientBounds();
   gfx::Size constrained_size = size_constraints.ClampSize(bounds.size());
   if (bounds.size() != constrained_size) {
     bounds.set_size(constrained_size);
+    bounds.Inset(-native_app_window_->GetFrameInsets());

[tmdiep, 2014/03/04 03:13:14]

Fixed bug - NativeAppWindow::SetBounds() sets frame bounds, so the frame insets
should have been added.

[tapted, 2014/03/05 03:14:03]

Will this shift the origin too? (do we want that?)

[tmdiep, 2014/03/05 04:29:06]

The existing code shifts the origin. With this change, the origin stays the
same, which is what I would expect.

     native_app_window_->SetBounds(bounds);
   }
   OnNativeWindowChanged();
@@ -858,7 +978,6 @@ void AppWindow::SaveWindowPosition() {
       AppWindowGeometryCache::Get(browser_context());
 
   gfx::Rect bounds = native_app_window_->GetRestoredBounds();
-  bounds.Inset(native_app_window_->GetFrameInsets());

[tmdiep, 2014/03/04 03:13:14]

If we store the content bounds in the window geometry cache, during the
restoration of the bounds, AdjustBoundsToBeVisibleOnScreen() will position the
content wrt the screen. You can potentially end up with the title bar off the
screen, so it's better to work with frame bounds, not content bounds when
positioning windows.

   gfx::Rect screen_bounds =
       gfx::Screen::GetNativeScreen()->GetDisplayMatching(bounds).work_area();
   ui::WindowShowState window_state = native_app_window_->GetRestoredState();
@@ -896,12 +1015,17 @@ void AppWindow::AdjustBoundsToBeVisibleOnScreen(
   }
 }
 
-AppWindow::CreateParams AppWindow::LoadDefaultsAndConstrain(CreateParams params)
+AppWindow::CreateParams AppWindow::LoadDefaults(CreateParams params)
     const {
-  if (params.bounds.width() == 0)
-    params.bounds.set_width(kDefaultWidth);
-  if (params.bounds.height() == 0)
-    params.bounds.set_height(kDefaultHeight);
+  // Ensure width and height are specified.
+  if (params.content_spec.bounds.width() == 0 &&
+      params.window_spec.bounds.width() == 0) {
+    params.window_spec.bounds.set_width(kDefaultWidth);

[benwells, 2014/03/05 02:33:30]

I think we should set the default on the content_spec so the content size is the
same on all platforms. I think this also matches what we did before.

[tapted, 2014/03/05 03:14:03]

should this be setting the content_spec instead of the window_spec? (otherwise I
think we would get different default sizes on different platforms)

+  }
+  if (params.content_spec.bounds.height() == 0 &&
+      params.window_spec.bounds.height() == 0) {
+    params.window_spec.bounds.set_height(kDefaultHeight);
+  }
 
   // If left and top are left undefined, the native app window will center
   // the window on the main screen in a platform-defined manner.
@@ -919,25 +1043,27 @@ AppWindow::CreateParams AppWindow::LoadDefaultsAndConstrain(CreateParams params)
                            &cached_bounds,
                            &cached_screen_bounds,
                            &cached_state)) {
+
       // App window has cached screen bounds, make sure it fits on screen in
       // case the screen resolution changed.
       gfx::Screen* screen = gfx::Screen::GetNativeScreen();
       gfx::Display display = screen->GetDisplayMatching(cached_bounds);
       gfx::Rect current_screen_bounds = display.work_area();
+      SizeConstraints constraints(params.GetWindowMinimumSize(gfx::Insets()),
+                                  params.GetWindowMaximumSize(gfx::Insets()));
       AdjustBoundsToBeVisibleOnScreen(cached_bounds,
                                       cached_screen_bounds,
                                       current_screen_bounds,
-                                      params.minimum_size,
-                                      &params.bounds);
+                                      constraints.GetMinimumSize(),
+                                      &params.window_spec.bounds);
       params.state = cached_state;
+
+      // Since we are restoring a cached state, reset the content bounds spec to
+      // ensure it is not used.
+      params.content_spec.ResetBounds();
     }
   }
 
-  SizeConstraints size_constraints(params.minimum_size, params.maximum_size);

[tmdiep, 2014/03/04 03:13:14]

Because the API lets you specify content or window constraints, we may not be
able to resolve the constraints until we have the frame insets, so this is now
done later.

-  params.bounds.set_size(size_constraints.ClampSize(params.bounds.size()));
-  params.minimum_size = size_constraints.GetMinimumSize();
-  params.maximum_size = size_constraints.GetMaximumSize();
-
   return params;
 }