Constrain extension popups to a min/max size.

Also, fix some glitches in sizing the popups.

BUG=25214
TEST=ExtensionApiTest.BrowserActionPopup

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

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

chrome/browser/extensions/browser_action_apitest.cc

@@ -149,25 +149,50 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_BrowserActionPopup) {
       browser()->window()->GetBrowserWindowTesting()->GetToolbarView()->
       browser_actions();
 
+  // This value is in api_test/popup/popup.html.
+  const int growFactor = 500;
+  ASSERT_GT(ExtensionPopup::kMinHeight + growFactor * 2,
+            ExtensionPopup::kMaxHeight);
+  ASSERT_GT(ExtensionPopup::kMinWidth + growFactor * 2,
+            ExtensionPopup::kMaxWidth);
+
+  // Our initial expected size.
+  int width = ExtensionPopup::kMinWidth;
+  int height = ExtensionPopup::kMinHeight;
+
   // Simulate a click on the browser action and verify the size of the resulting
-  // popup.
+  // popup.  The first one tries to be 0x0, so it should be the min values.
   browser_actions->TestExecuteBrowserAction(0);
   EXPECT_TRUE(browser_actions->TestGetPopup() != NULL);
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
   gfx::Rect bounds = browser_actions->TestGetPopup()->view()->bounds();
-  EXPECT_EQ(100, bounds.width());
-  EXPECT_EQ(100, bounds.height());
+  EXPECT_EQ(width, bounds.width());
+  EXPECT_EQ(height, bounds.height());
   browser_actions->HidePopup();
   EXPECT_TRUE(browser_actions->TestGetPopup() == NULL);
 
   // Do it again, and verify the new bigger size (the popup grows each time it's
   // opened).
+  width = growFactor;
+  height = growFactor;
+  browser_actions->TestExecuteBrowserAction(0);
+  EXPECT_TRUE(browser_actions->TestGetPopup() != NULL);
+  ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
+  bounds = browser_actions->TestGetPopup()->view()->bounds();
+  EXPECT_EQ(width, bounds.width());
+  EXPECT_EQ(height, bounds.height());
+  browser_actions->HidePopup();
+  EXPECT_TRUE(browser_actions->TestGetPopup() == NULL);
+
+  // One more time, but this time it should be constrained by the max values.
+  width = ExtensionPopup::kMaxWidth;
+  height = ExtensionPopup::kMaxHeight;
   browser_actions->TestExecuteBrowserAction(0);
   EXPECT_TRUE(browser_actions->TestGetPopup() != NULL);
   ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
   bounds = browser_actions->TestGetPopup()->view()->bounds();
-  EXPECT_EQ(200, bounds.width());
-  EXPECT_EQ(200, bounds.height());
+  EXPECT_EQ(width, bounds.width());
+  EXPECT_EQ(height, bounds.height());
   browser_actions->HidePopup();
   EXPECT_TRUE(browser_actions->TestGetPopup() == NULL);
 }

chrome/browser/views/browser_bubble.cc

@@ -95,6 +95,5 @@ void BrowserBubble::Reposition() {
 }
 
 void BrowserBubble::ResizeToView() {
-  gfx::Size size = view_->GetPreferredSize();
-  SetBounds(bounds_.x(), bounds_.y(), size.width(), size.height());
+  SetBounds(bounds_.x(), bounds_.y(), view_->width(), view_->height());
 }

chrome/browser/views/extensions/extension_popup.cc

@@ -13,11 +13,18 @@
 #include "chrome/common/notification_details.h"
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
-
 #include "views/widget/root_view.h"
 
 using views::Widget;
 
+// The minimum/maximum dimensions of the popup.
+// The minimum is just a little larger than the size of the button itself.
+// The maximum is an arbitrary number that should be smaller than most screens.
+const int ExtensionPopup::kMinWidth = 25;
+const int ExtensionPopup::kMinHeight = 25;
+const int ExtensionPopup::kMaxWidth = 800;
+const int ExtensionPopup::kMaxHeight = 600;
+
 ExtensionPopup::ExtensionPopup(ExtensionHost* host,
                                Widget* frame,
                                const gfx::Rect& relative_to)
@@ -66,14 +73,16 @@ void ExtensionPopup::Show() {
 }
 
 void ExtensionPopup::ResizeToView() {
-  BrowserBubble::ResizeToView();
+  // We'll be sizing ourselves to this size shortly, but wait until we
+  // know our position to do it.
+  gfx::Size new_size = view()->size();
 
   // The rounded corners cut off more of the view than the border insets claim.
   // Since we can't clip the ExtensionView's corners, we need to increase the
   // inset by half the corner radius as well as lying about the size of the
   // contents size to compensate.
   int corner_inset = BubbleBorder::GetCornerRadius() / 2;
-  gfx::Size adjusted_size = bounds().size();
+  gfx::Size adjusted_size = new_size;
   adjusted_size.Enlarge(2 * corner_inset, 2 * corner_inset);
   gfx::Rect rect = border_->GetBounds(relative_to_, adjusted_size);
   border_widget_->SetBounds(rect);
@@ -87,7 +96,8 @@ void ExtensionPopup::ResizeToView() {
   views::View::ConvertPointToView(NULL, frame_->GetRootView(), &origin);
   origin.set_x(origin.x() + border_insets.left() + corner_inset);
   origin.set_y(origin.y() + border_insets.top() + corner_inset);
-  MoveTo(origin.x(), origin.y());
+
+  SetBounds(origin.x(), origin.y(), new_size.width(), new_size.height());
 }
 
 void ExtensionPopup::Observe(NotificationType type,
@@ -104,7 +114,12 @@ void ExtensionPopup::Observe(NotificationType type,
 }
 
 void ExtensionPopup::OnExtensionPreferredSizeChanged(ExtensionView* view) {
-  view->SizeToPreferredSize();
+  // Constrain the size to popup min/max.
+  gfx::Size sz = view->GetPreferredSize();
+  view->SetBounds(view->x(), view->y(),
+      std::max(kMinWidth, std::min(kMaxWidth, sz.width())),
+      std::max(kMinHeight, std::min(kMaxHeight, sz.height())));
+
   ResizeToView();
 }
 

chrome/browser/views/extensions/extension_popup.h

@@ -47,6 +47,12 @@ class ExtensionPopup : public BrowserBubble,
   virtual void OnExtensionMouseLeave(ExtensionView* view) { };
   virtual void OnExtensionPreferredSizeChanged(ExtensionView* view);
 
+  // The min/max height of popups.
+  static const int kMinWidth;
+  static const int kMinHeight;
+  static const int kMaxWidth;
+  static const int kMaxHeight;
+
  private:
   ExtensionPopup(ExtensionHost* host,
                  views::Widget* frame,

chrome/test/data/extensions/api_test/popup/popup.html

@@ -2,8 +2,8 @@
 <script>
 function run_tests() {
   // Compute the size of the popup.
-  var width = 100;
-  var height = 100;
+  var width = 0;
+  var height = 0;
   if (localStorage.height) {
     height = parseInt(localStorage.height);
   }
@@ -22,8 +22,8 @@ function run_tests() {
   //window.setTimeout(chrome.test.notifyPass, 0);
   chrome.test.notifyPass();
 
-  height += 100;
-  width += 100;
+  height += 500;
+  width += 500;
   localStorage.height = JSON.stringify(height);
   localStorage.width = JSON.stringify(width);
 }
@@ -40,5 +40,5 @@ div {
 </style>
 </head>
 <body onload="window.setTimeout(run_tests, 0)">
-<div id="test">TEST</div>
+<div id="test">A</div>
 </body>