[WIN]Save work area of window and adjust bounds to ensure it fit on screen before show.

chrome/browser/ui/extensions/shell_window.cc

Index: chrome/browser/ui/extensions/shell_window.cc
diff --git a/chrome/browser/ui/extensions/shell_window.cc b/chrome/browser/ui/extensions/shell_window.cc
index 5d3f4af6c8a6a10ccbecf89a2503c6dbb22bd386..a2f27340946e40b232b3209cdef8818d4567f8fb 100644
--- a/chrome/browser/ui/extensions/shell_window.cc
+++ b/chrome/browser/ui/extensions/shell_window.cc
@@ -152,6 +152,7 @@ void ShellWindow::Init(const GURL& url,
   // If left and top are left undefined, the native shell window will center
   // the window on the main screen in a platform-defined manner.
 
+  gfx::Rect cached_screen_bounds;
   ui::WindowShowState cached_state = ui::SHOW_STATE_DEFAULT;
   if (!params.window_key.empty()) {
     window_key_ = params.window_key;
@@ -160,8 +161,8 @@ void ShellWindow::Init(const GURL& url,
         apps::ShellWindowGeometryCache::Get(profile());
 
     gfx::Rect cached_bounds;
-    if (cache->GetGeometry(extension()->id(), params.window_key,
-                           &cached_bounds, &cached_state)) {
+    if (cache->GetGeometry(extension()->id(), params.window_key, &cached_bounds,
+                           &cached_screen_bounds, &cached_state)) {
       bounds = cached_bounds;
     }
   }
@@ -195,6 +196,18 @@ void ShellWindow::Init(const GURL& url,
 
   native_app_window_.reset(NativeAppWindow::Create(this, new_params));
 
+  // App window has cached screen bounds, make sure it fits on screen in case of

[scheib, 2013/06/20 00:09:59]

Please add that explanation to the comment here.

[zhchbin, 2013/06/20 06:50:52]

Done.

+  // the screen resolution changed before show.
+  if (!cached_screen_bounds.IsEmpty()) {
+    gfx::Rect new_bounds;
+    gfx::Rect current_screen_bounds = native_app_window_->GetScreenBounds();
+    AdjustBoundsToBeVisibleOnScreen(bounds,
+                                    cached_screen_bounds,
+                                    current_screen_bounds,
+                                    minimum_size,
+                                    &new_bounds);
+    native_app_window_->SetBounds(new_bounds);
+  }
   if (!new_params.hidden) {
     if (window_type_is_panel())
       GetBaseWindow()->ShowInactive();  // Panels are not activated by default.
@@ -618,10 +631,52 @@ void ShellWindow::SaveWindowPosition() {
 
   gfx::Rect bounds = native_app_window_->GetRestoredBounds();
   bounds.Inset(native_app_window_->GetFrameInsets());
+  gfx::Rect screen_bounds = native_app_window_->GetScreenBounds();
   ui::WindowShowState window_state = native_app_window_->GetRestoredState();
-  cache->SaveGeometry(extension()->id(), window_key_, bounds, window_state);
+  cache->SaveGeometry(extension()->id(),
+                      window_key_,
+                      bounds,
+                      screen_bounds,
+                      window_state);
+}
+
+void ShellWindow::AdjustBoundsToBeVisibleOnScreen(

[scheib, 2013/06/20 00:09:59]

Please add tests for AdjustBoundsToBeVisibleOnScreen, e.g. testing scenarios
with cached bounds overlapping or completely off the new screen bounds in both
horizontal and vertical locations. You can refactor the member to be static,
allowing simple 'unit test' style calls for it without requiring much setup.

[zhchbin, 2013/06/20 06:50:52]

I had integrated it into the platform_app_browsertest. And I think private
helper method will be better than the static. Is it OK?

+    const gfx::Rect& cached_bounds,
+    const gfx::Rect& cached_screen_bounds,
+    const gfx::Rect& current_screen_bounds,
+    const gfx::Size& minimum_size,
+    gfx::Rect* bounds) const {
+  if (!native_app_window_)
+    return;
+  if (!bounds)
+    return;
+
+  *bounds = cached_bounds;
+
+  // Reposition and resize the bounds if the cached_screen_bounds is different
+  // from the current screen bounds and the current screen bounds doesn't
+  // completely contain the bounds.
+  if (!cached_screen_bounds.IsEmpty() &&
+      cached_screen_bounds != current_screen_bounds &&
+      !current_screen_bounds.Contains(cached_bounds)) {
+    bounds->set_width(
+        std::max(minimum_size.width(),
+                 std::min(bounds->width(), current_screen_bounds.width())));
+    bounds->set_height(
+        std::max(minimum_size.height(),
+                 std::min(bounds->height(), current_screen_bounds.height())));
+    bounds->set_x(
+        std::max(current_screen_bounds.x(),
+                 std::min(bounds->x(),
+                          current_screen_bounds.right() - bounds->width())));
+    bounds->set_y(
+        std::max(current_screen_bounds.y(),
+                 std::min(bounds->y(),
+                          current_screen_bounds.bottom() - bounds->height())));
+  }
 }
 
+
 // static
 SkRegion* ShellWindow::RawDraggableRegionsToSkRegion(
       const std::vector<extensions::DraggableRegion>& regions) {