Do not restore corrupt cached app window bounds.

Corrupted cached bounds happen durring development, 
and possibly to all users. This change hardens the
loading code to not use invalid cached state.

Related clean up in shell_window for adjusting
screen bounds as well.

BUG=266578

Review URL: https://chromiumcodereview.appspot.com/21444002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@215098 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 83 insertions, 29 deletions

diff --git a/apps/shell_window.cc b/apps/shell_window.cc
index 3fd2314..428ca5c 100644
--- a/apps/shell_window.cc
+++ b/apps/shell_window.cc
@@ -108,7 +108,9 @@ 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.
 
-  ui::WindowShowState cached_state = ui::SHOW_STATE_DEFAULT;
+  CreateParams new_params = params;
+
+  // Load cached state if it exists.
   if (!params.window_key.empty()) {
     window_key_ = params.window_key;
 
@@ -116,26 +118,23 @@ void ShellWindow::Init(const GURL& url,
 
     gfx::Rect cached_bounds;
     gfx::Rect cached_screen_bounds;
+    ui::WindowShowState cached_state = ui::SHOW_STATE_DEFAULT;
     if (cache->GetGeometry(extension()->id(), params.window_key, &cached_bounds,
                            &cached_screen_bounds, &cached_state)) {
-      bounds = cached_bounds;
       // App window has cached screen bounds, make sure it fits on screen in
       // case the screen resolution changed.
-      if (!cached_screen_bounds.IsEmpty()) {
-        gfx::Screen* screen = gfx::Screen::GetNativeScreen();
-        gfx::Display display = screen->GetDisplayMatching(cached_bounds);
-        gfx::Rect current_screen_bounds = display.work_area();
-        AdjustBoundsToBeVisibleOnScreen(cached_bounds,
-                                        cached_screen_bounds,
-                                        current_screen_bounds,
-                                        params.minimum_size,
-                                        &bounds);
-      }
+      gfx::Screen* screen = gfx::Screen::GetNativeScreen();
+      gfx::Display display = screen->GetDisplayMatching(cached_bounds);
+      gfx::Rect current_screen_bounds = display.work_area();
+      AdjustBoundsToBeVisibleOnScreen(cached_bounds,
+                                      cached_screen_bounds,
+                                      current_screen_bounds,
+                                      params.minimum_size,
+                                      &bounds);
+      new_params.state = cached_state;
     }
   }
 
-  CreateParams new_params = params;
-
   gfx::Size& minimum_size = new_params.minimum_size;
   gfx::Size& maximum_size = new_params.maximum_size;
 
@@ -158,9 +157,6 @@ void ShellWindow::Init(const GURL& url,
 
   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));
 
   if (!new_params.hidden) {
@@ -589,16 +585,12 @@ void ShellWindow::AdjustBoundsToBeVisibleOnScreen(
     const gfx::Rect& current_screen_bounds,
     const gfx::Size& minimum_size,
     gfx::Rect* bounds) const {
-  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 &&
+  if (cached_screen_bounds != current_screen_bounds &&
       !current_screen_bounds.Contains(cached_bounds)) {
     bounds->set_width(
         std::max(minimum_size.width(),
diff --git a/apps/shell_window_geometry_cache.cc b/apps/shell_window_geometry_cache.cc
index caa83ac..31b44a4 100644
--- a/apps/shell_window_geometry_cache.cc
+++ b/apps/shell_window_geometry_cache.cc
@@ -147,18 +147,26 @@ bool ShellWindowGeometryCache::GetGeometry(
     DCHECK(extension_data_it != cache_.end());
   }
 
-  ExtensionData::const_iterator window_data = extension_data_it->second.find(
+  ExtensionData::const_iterator window_data_it = extension_data_it->second.find(
       window_id);
 
-  if (window_data == extension_data_it->second.end())
+  if (window_data_it == extension_data_it->second.end())
+    return false;
+
+  const WindowData& window_data = window_data_it->second;
+
+  // Check for and do not return corrupt data.
+  if ((bounds && window_data.bounds.IsEmpty()) ||
+      (screen_bounds && window_data.screen_bounds.IsEmpty()) ||
+      (window_state && window_data.window_state == ui::SHOW_STATE_DEFAULT))
     return false;
 
   if (bounds)
-    *bounds = window_data->second.bounds;
+    *bounds = window_data.bounds;
   if (screen_bounds)
-    *screen_bounds = window_data->second.screen_bounds;
+    *screen_bounds = window_data.screen_bounds;
   if (window_state)
-    *window_state = window_data->second.window_state;
+    *window_state = window_data.window_state;
   return true;
 }
 
diff --git a/apps/shell_window_geometry_cache.h b/apps/shell_window_geometry_cache.h
index d4b3581..b6709df 100644
--- a/apps/shell_window_geometry_cache.h
+++ b/apps/shell_window_geometry_cache.h
@@ -73,8 +73,8 @@ class ShellWindowGeometryCache
                     ui::WindowShowState state);
 
   // Get any saved geometry and state associated with |extension_id| and
-  // |window_id|. If saved data exists, sets |bounds| and |state| if not NULL
-  // and returns true.
+  // |window_id|. If saved data exists, sets |bounds|, |screen_bounds| and
+  // |state| if not NULL and returns true.
   bool GetGeometry(const std::string& extension_id,
                    const std::string& window_id,
                    gfx::Rect* bounds,
diff --git a/apps/shell_window_geometry_cache_unittest.cc b/apps/shell_window_geometry_cache_unittest.cc
index 7cc3dac..98c4bdc 100644
--- a/apps/shell_window_geometry_cache_unittest.cc
+++ b/apps/shell_window_geometry_cache_unittest.cc
@@ -141,6 +141,60 @@ TEST_F(ShellWindowGeometryCacheTest, GetGeometryAndStateFromStore) {
   ASSERT_EQ(state, new_state);
 }
 
+// Test corrupt bounds will not be loaded.
+TEST_F(ShellWindowGeometryCacheTest, CorruptBounds) {
+  const std::string extension_id = prefs_->AddExtensionAndReturnId("ext1");
+  gfx::Rect bounds;
+  gfx::Rect screen_bounds(0, 0, 1600, 900);
+  ui::WindowShowState state = ui::SHOW_STATE_NORMAL;
+  AddGeometryAndLoadExtension(extension_id, kWindowId, bounds,
+                              screen_bounds, state);
+  gfx::Rect new_bounds;
+  gfx::Rect new_screen_bounds;
+  ui::WindowShowState new_state = ui::SHOW_STATE_DEFAULT;
+  ASSERT_FALSE(cache_->GetGeometry(
+      extension_id, kWindowId, &new_bounds, &new_screen_bounds, &new_state));
+  ASSERT_TRUE(new_bounds.IsEmpty());
+  ASSERT_TRUE(new_screen_bounds.IsEmpty());
+  ASSERT_EQ(new_state, ui::SHOW_STATE_DEFAULT);
+}
+
+// Test corrupt screen bounds will not be loaded.
+TEST_F(ShellWindowGeometryCacheTest, CorruptScreenBounds) {
+  const std::string extension_id = prefs_->AddExtensionAndReturnId("ext1");
+  gfx::Rect bounds(4, 5, 31, 43);
+  gfx::Rect screen_bounds;
+  ui::WindowShowState state = ui::SHOW_STATE_NORMAL;
+  AddGeometryAndLoadExtension(extension_id, kWindowId, bounds,
+                              screen_bounds, state);
+  gfx::Rect new_bounds;
+  gfx::Rect new_screen_bounds;
+  ui::WindowShowState new_state = ui::SHOW_STATE_DEFAULT;
+  ASSERT_FALSE(cache_->GetGeometry(
+      extension_id, kWindowId, &new_bounds, &new_screen_bounds, &new_state));
+  ASSERT_TRUE(new_bounds.IsEmpty());
+  ASSERT_TRUE(new_screen_bounds.IsEmpty());
+  ASSERT_EQ(new_state, ui::SHOW_STATE_DEFAULT);
+}
+
+// Test corrupt state will not be loaded.
+TEST_F(ShellWindowGeometryCacheTest, CorruptState) {
+  const std::string extension_id = prefs_->AddExtensionAndReturnId("ext1");
+  gfx::Rect bounds(4, 5, 31, 43);
+  gfx::Rect screen_bounds(0, 0, 1600, 900);
+  ui::WindowShowState state = ui::SHOW_STATE_DEFAULT;
+  AddGeometryAndLoadExtension(extension_id, kWindowId, bounds,
+                              screen_bounds, state);
+  gfx::Rect new_bounds;
+  gfx::Rect new_screen_bounds;
+  ui::WindowShowState new_state = ui::SHOW_STATE_DEFAULT;
+  ASSERT_FALSE(cache_->GetGeometry(
+      extension_id, kWindowId, &new_bounds, &new_screen_bounds, &new_state));
+  ASSERT_TRUE(new_bounds.IsEmpty());
+  ASSERT_TRUE(new_screen_bounds.IsEmpty());
+  ASSERT_EQ(new_state, ui::SHOW_STATE_DEFAULT);
+}
+
 // Test saving geometry, screen_bounds and state to the cache and state store,
 // and reading it back.
 TEST_F(ShellWindowGeometryCacheTest, SaveGeometryAndStateToStore) {