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

chrome/browser/ui/extensions/shell_window.cc

 // Copyright (c) 2012 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 "chrome/browser/ui/extensions/shell_window.h"
 
 #include "apps/shell_window_geometry_cache.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/app_window_contents.h"
@@ skipping 134 matching lines @@
   gfx::Rect bounds = params.bounds;
 
   if (bounds.width() == 0)
     bounds.set_width(kDefaultWidth);
   if (bounds.height() == 0)
     bounds.set_height(kDefaultHeight);
 
   // 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;
 
     apps::ShellWindowGeometryCache* cache =
         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;
     }
   }
 
   CreateParams new_params = params;
 
   gfx::Size& minimum_size = new_params.minimum_size;
   gfx::Size& maximum_size = new_params.maximum_size;
 
   // In the case that minimum size > maximum size, we consider the minimum
@@ skipping 13 matching lines @@
   if (bounds.height() != INT_MIN && bounds.height() < minimum_size.height())
     bounds.set_height(minimum_size.height());
 
   new_params.bounds = bounds;
 
   if (cached_state != ui::SHOW_STATE_DEFAULT)
     new_params.state = cached_state;
 
   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.
     else
       GetBaseWindow()->Show();
   }
 
   if (new_params.state == ui::SHOW_STATE_FULLSCREEN)
     Fullscreen();
   else if (new_params.state == ui::SHOW_STATE_MAXIMIZED)
@@ skipping 403 matching lines @@
   if (window_key_.empty())
     return;
   if (!native_app_window_)
     return;
 
   apps::ShellWindowGeometryCache* cache =
       apps::ShellWindowGeometryCache::Get(profile());
 
   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) {
   SkRegion* sk_region = new SkRegion;
   for (std::vector<extensions::DraggableRegion>::const_iterator iter =
            regions.begin();
        iter != regions.end(); ++iter) {
     const extensions::DraggableRegion& region = *iter;
     sk_region->op(
         region.bounds.x(),
         region.bounds.y(),
         region.bounds.right(),
         region.bounds.bottom(),
         region.draggable ? SkRegion::kUnion_Op : SkRegion::kDifference_Op);
   }
   return sk_region;
 }
 
 void ShellWindow::DisableExternalOpenForTesting() {
   disable_external_open_for_testing_ = true;
 }