Mac: Don't restore very small windows

I'm not sure how the situation in the bug happened, but this should protect us from the effect in the future.

BUG=39625
TEST=unit test

Review URL: http://codereview.chromium.org/2864042

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

2 files changed, 66 insertions, 7 deletions

diff --git a/chrome/browser/cocoa/window_size_autosaver.mm b/chrome/browser/cocoa/window_size_autosaver.mm
index 4db6f28..a200090 100644
--- a/chrome/browser/cocoa/window_size_autosaver.mm
+++ b/chrome/browser/cocoa/window_size_autosaver.mm
@@ -8,6 +8,14 @@
 
 #include "chrome/browser/pref_service.h"
 
+// If the window width stored in the prefs is smaller than this, the size is
+// not restored but instead cleared from the profile -- to protect users from
+// accidentally making their windows very small and then not finding them again.
+const int kMinWindowWidth = 101;
+
+// Minimum restored window height, see |kMinWindowWidth|.
+const int kMinWindowHeight = 17;
+
 @interface WindowSizeAutosaver (Private)
 - (void)save:(NSNotification*)notification;
 - (void)restore;
@@ -77,7 +85,18 @@
         !windowPrefs->GetInteger(L"bottom", &y2)) {
       return;
     }
-    [window_ setFrame:NSMakeRect(x1, y1, x2 - x1, y2 - y1) display:YES];
+    if (x2 - x1 < kMinWindowWidth || y2 - y1 < kMinWindowHeight) {
+      // Windows should never be very small.
+      windowPrefs->Remove(L"left", NULL);
+      windowPrefs->Remove(L"right", NULL);
+      windowPrefs->Remove(L"top", NULL);
+      windowPrefs->Remove(L"bottom", NULL);
+    } else {
+      [window_ setFrame:NSMakeRect(x1, y1, x2 - x1, y2 - y1) display:YES];
+
+      // Make sure the window is on-screen.
+      [window_ cascadeTopLeftFromPoint:NSZeroPoint];
+    }
   } else if (state_ == kSaveWindowPos) {
     int x, y;
     if (!windowPrefs->GetInteger(L"x", &x) ||
diff --git a/chrome/browser/cocoa/window_size_autosaver_unittest.mm b/chrome/browser/cocoa/window_size_autosaver_unittest.mm
index 6507a85..b3ce0d2 100644
--- a/chrome/browser/cocoa/window_size_autosaver_unittest.mm
+++ b/chrome/browser/cocoa/window_size_autosaver_unittest.mm
@@ -13,6 +13,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
+namespace {
+
 class WindowSizeAutosaverTest : public CocoaTest {
   virtual void SetUp() {
     CocoaTest::SetUp();
@@ -62,7 +64,7 @@ TEST_F(WindowSizeAutosaverTest, RestoresAndSavesPos) {
     EXPECT_EQ(NSHeight(frame), NSHeight([window_ frame]));
 
     // Move and resize window, should store position but not size.
-    [window_ setFrame:NSMakeRect(300, 310, 50, 52) display:NO];
+    [window_ setFrame:NSMakeRect(300, 310, 250, 252) display:NO];
   }
 
   // Another window movement -- shouldn't be recorded.
@@ -121,7 +123,7 @@ TEST_F(WindowSizeAutosaverTest, RestoresAndSavesRect) {
     EXPECT_EQ(NSHeight(frame), NSHeight([window_ frame]));
 
     // Move and resize window, should store
-    [window_ setFrame:NSMakeRect(300, 310, 50, 52) display:NO];
+    [window_ setFrame:NSMakeRect(300, 310, 250, 252) display:NO];
   }
 
   // Another window movement -- shouldn't be recorded.
@@ -136,8 +138,8 @@ TEST_F(WindowSizeAutosaverTest, RestoresAndSavesRect) {
                  state:kSaveWindowRect]);
     EXPECT_EQ(300, NSMinX([window_ frame]));
     EXPECT_EQ(310, NSMinY([window_ frame]));
-    EXPECT_EQ(50, NSWidth([window_ frame]));
-    EXPECT_EQ(52, NSHeight([window_ frame]));
+    EXPECT_EQ(250, NSWidth([window_ frame]));
+    EXPECT_EQ(252, NSHeight([window_ frame]));
   }
 
   // ...and it should be in the profile, too.
@@ -152,6 +154,44 @@ TEST_F(WindowSizeAutosaverTest, RestoresAndSavesRect) {
   ASSERT_TRUE(windowPref->GetInteger(L"bottom", &y2));
   EXPECT_EQ(300, x1);
   EXPECT_EQ(310, y1);
-  EXPECT_EQ(300 + 50, x2);
-  EXPECT_EQ(310 + 52, y2);
+  EXPECT_EQ(300 + 250, x2);
+  EXPECT_EQ(310 + 252, y2);
+}
+
+// http://crbug.com/39625
+TEST_F(WindowSizeAutosaverTest, DoesNotRestoreButClearsEmptyRect) {
+  PrefService* pref = browser_helper_.profile()->GetPrefs();
+  ASSERT_TRUE(pref != NULL);
+
+  DictionaryValue* windowPref = pref->GetMutableDictionary(path_);
+  windowPref->SetInteger(L"left", 50);
+  windowPref->SetInteger(L"right", 50);
+  windowPref->SetInteger(L"top", 60);
+  windowPref->SetInteger(L"bottom", 60);
+
+  {
+    // Window rect shouldn't change...
+    NSRect frame = [window_ frame];
+    scoped_nsobject<WindowSizeAutosaver> sizeSaver([[WindowSizeAutosaver alloc]
+        initWithWindow:window_
+           prefService:pref
+                  path:path_
+                 state:kSaveWindowRect]);
+    EXPECT_EQ(NSMinX(frame), NSMinX([window_ frame]));
+    EXPECT_EQ(NSMinY(frame), NSMinY([window_ frame]));
+    EXPECT_EQ(NSWidth(frame), NSWidth([window_ frame]));
+    EXPECT_EQ(NSHeight(frame), NSHeight([window_ frame]));
+  }
+
+  // ...and it should be gone from the profile, too.
+  EXPECT_TRUE(pref->GetDictionary(path_) != NULL);
+  int x1, y1, x2, y2;
+  EXPECT_FALSE(windowPref->GetInteger(L"x", &x1));
+  EXPECT_FALSE(windowPref->GetInteger(L"y", &x1));
+  ASSERT_FALSE(windowPref->GetInteger(L"left", &x1));
+  ASSERT_FALSE(windowPref->GetInteger(L"right", &x2));
+  ASSERT_FALSE(windowPref->GetInteger(L"top", &y1));
+  ASSERT_FALSE(windowPref->GetInteger(L"bottom", &y2));
 }
+
+}  // namespace