Correction of sizing of experimental.extension.popup.* windows. When displaying a popup with 'rectangle' chrome, the system makes use of views::Border. The code was not accounting for the decrease in client area due to the border insets when responding to preferred size notifications.

BUG=40688
TEST=ExtensionApiTest.Popup

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

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

5 files changed, 139 insertions, 13 deletions

diff --git a/chrome/browser/views/extensions/extension_popup.cc b/chrome/browser/views/extensions/extension_popup.cc
index ab0032a..6737ac2 100644
--- a/chrome/browser/views/extensions/extension_popup.cc
+++ b/chrome/browser/views/extensions/extension_popup.cc
@@ -280,12 +280,25 @@ void ExtensionPopup::OnExtensionPreferredSizeChanged(ExtensionView* view) {
       std::max(kMinWidth, std::min(kMaxWidth, sz.width())),
       std::max(kMinHeight, std::min(kMaxHeight, sz.height())));
 
+  // If popup_chrome_ == RECTANGLE_CHROME, the border is drawn in the client
+  // area of the ExtensionView, rather than in a window which sits behind it.
+  // In this case, the actual size of the view must be enlarged so that the
+  // web contents portion of the view gets its full PreferredSize area.
+  if (view->border()) {
+    gfx::Insets border_insets;
+    view->border()->GetInsets(&border_insets);
+
+    gfx::Rect bounds(view->bounds());
+    gfx::Size size(bounds.size());
+    size.Enlarge(border_insets.width(), border_insets.height());
+    view->SetBounds(bounds.x(), bounds.y(), size.width(), size.height());
+  }
+
   ResizeToView();
 }
 
 gfx::Rect ExtensionPopup::GetOuterBounds(const gfx::Rect& position_relative_to,
                                          const gfx::Size& contents_size) const {
-  gfx::Size adjusted_size = contents_size;
   // If the popup has a bubble-chrome, then let the BubbleBorder compute
   // the bounds.
   if (BUBBLE_CHROME == popup_chrome_) {
@@ -293,27 +306,23 @@ gfx::Rect ExtensionPopup::GetOuterBounds(const gfx::Rect& position_relative_to,
     // 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.
+    gfx::Size adjusted_size = contents_size;
     adjusted_size.Enlarge(2 * kPopupBubbleCornerRadius,
                           2 * kPopupBubbleCornerRadius);
     return border_->GetBounds(position_relative_to, adjusted_size);
   }
 
-  // Otherwise, enlarge the bounds by the size of the local border.
-  gfx::Insets border_insets;
-  view()->border()->GetInsets(&border_insets);
-  adjusted_size.Enlarge(border_insets.width(), border_insets.height());
-
   // Position the bounds according to the location of the |anchor_position_|.
   int y;
   if ((anchor_position_ == BubbleBorder::TOP_LEFT) ||
       (anchor_position_ == BubbleBorder::TOP_RIGHT)) {
     y = position_relative_to.bottom();
   } else {
-    y = position_relative_to.y() - adjusted_size.height();
+    y = position_relative_to.y() - contents_size.height();
   }
 
-  return gfx::Rect(position_relative_to.x(), y, adjusted_size.width(),
-                   adjusted_size.height());
+  return gfx::Rect(position_relative_to.x(), y, contents_size.width(),
+                   contents_size.height());
 }
 
 // static
diff --git a/chrome/test/data/extensions/api_test/popup/manifest.json b/chrome/test/data/extensions/api_test/popup/manifest.json
index 4f950ff..3ab7d85 100644
--- a/chrome/test/data/extensions/api_test/popup/manifest.json
+++ b/chrome/test/data/extensions/api_test/popup/manifest.json
@@ -5,5 +5,5 @@
   "toolstrips": [
     "toolband.html"
   ],
-  "permissions": ["experimental"]
+  "permissions": ["experimental", "tabs"]
 }
diff --git a/chrome/test/data/extensions/api_test/popup/toolband.html b/chrome/test/data/extensions/api_test/popup/toolband.html
index f0dd50b2..5d04350 100644
--- a/chrome/test/data/extensions/api_test/popup/toolband.html
+++ b/chrome/test/data/extensions/api_test/popup/toolband.html
@@ -13,6 +13,21 @@ function onFormBlurred() {
   formFocused = false;
 }
 
+// Callback that validates popup repositioning, and is invoked during execution
+// of the following tests:
+//   popupRectangleSizing and popupChromeSizing.
+// |offset| specifies the delta in screen-space by which the browser was moved.
+// |initialSize| specfies the rect of the popup before the brower move.
+// |movedSize| specifies the rect of the popup after the browser move.
+function onWindowMoveCompleted(offset, initialSize, movedSize) {
+  chrome.test.assertEq(initialSize.width, movedSize.width);
+  chrome.test.assertEq(initialSize.height, movedSize.height);
+  chrome.test.assertTrue(
+      initialSize.top + offset.y == movedSize.top &&
+      initialSize.left + offset.x == movedSize.left,
+      "Popup repositioned incorrectly after browser move.");
+}
+
 window.onload = function() {
   chrome.test.runTests([
     function showNoFocusShift() {
@@ -27,7 +42,7 @@ window.onload = function() {
       };
 
       // The focus should also remain untouched during closing of the popup.
-      chrome.test.listenOnce(chrome.experimental.popup.onClosed, function(){
+      chrome.test.listenOnce(chrome.experimental.popup.onClosed, function() {
         chrome.test.assertTrue(formFocused);
       });
 
@@ -85,7 +100,8 @@ window.onload = function() {
       chrome.test.succeed();
     },
     function closePopup() {
-      chrome.test.listenOnce(chrome.experimental.popup.onClosed, function(){
+      // Ensure that the test waits until the popup is dismissed.
+      chrome.test.listenOnce(chrome.experimental.popup.onClosed, function() {
         // TODO(twiz):  getPopupView() should return undefined, but, due to
         // shut-down races, it is sometimes still defined.  See BUG 27658.
         // chrome.test.assertTrue(
@@ -94,6 +110,10 @@ window.onload = function() {
       chrome.experimental.extension.getPopupView().close();
     },
     function popupBlackBorder() {
+      // Ensure that the test waits until the popup is dismissed.
+      chrome.test.listenOnce(chrome.experimental.popup.onClosed,
+                             function() {});
+
       // Validate that displaying a pop-up with a black border still invokes
       // the callback successfully.  Note that this test does not validate
       // the actual style of the border displayed.
@@ -106,6 +126,33 @@ window.onload = function() {
                                      chrome.test.callbackPass(function() {
         chrome.experimental.extension.getPopupView().close();
       }));
+    },
+    function popupChromeSizing() {
+      // Ensure that the test waits until the popup is dismissed.
+      chrome.test.listenOnce(chrome.experimental.popup.onClosed);
+
+      // Ensure that popups with a chrome border are repositioned and sized
+      // correctly.
+      var showDetails = {
+        "relativeTo": document.getElementById("anchorHere")
+      };
+
+      chrome.experimental.popup.show("toolband_popup_sizing.html",
+                                     showDetails);
+    },
+    function popupRectangleSizing() {
+      // Ensure that the test waits until the popup is dismissed.
+      chrome.test.listenOnce(chrome.experimental.popup.onClosed);
+
+      // Ensure that popups with a rectangle border are repositioned and sized
+      // correctly.
+      var showDetails = {
+        "relativeTo": document.getElementById("anchorHere"),
+        "borderStyle": "rectangle"
+      };
+
+      chrome.experimental.popup.show("toolband_popup_sizing.html",
+                                     showDetails);
     }
   ]);
 }
diff --git a/chrome/test/data/extensions/api_test/popup/toolband_popup_sizing.html b/chrome/test/data/extensions/api_test/popup/toolband_popup_sizing.html
new file mode 100644
index 0000000..07d57857
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/popup/toolband_popup_sizing.html
@@ -0,0 +1,69 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<script>

+// Returns the current size of the displayed popup view.

+function getCurrentWindowSize() {

+  return {

+    "top": window.screenTop,

+    "left": window.screenLeft,

+    "width": window.outerWidth,

+    "height": window.outerHeight

+  };

+}

+

+// Utility that captures the size of the popup window before and after a browser

+// move and notifies the test's parent view of the respective sizes.

+function doSizingValidation() {

+  var initialSize = getCurrentWindowSize();

+

+  // Move the browser, and ensure that the popup is repositioned correctly,

+  // and retains its proper size.

+  var offset = {'x': 5, 'y': 5};

+  chrome.windows.getCurrent(function(browserWindow) {

+    chrome.windows.update(browserWindow.id,

+                          {

+                            "left": browserWindow.left + offset.x,

+                            "top": browserWindow.top + offset.y

+                          },

+                          function(UpdatedWindow) {

+      // Yield so that the window move notification may be processed before

+      // invoking the callback.  This is required because chrome.windows.update

+      // calls its callback in a race with the windows message that repositions

+      // the browser.

+      // TODO:  Fix this race condition so that the update callback is invoked

+      // after all of the update machinery has been invoked.

+      var updatePoller = setInterval(function() {

+        var newPosition = getCurrentWindowSize();

+        if (newPosition.top != initialSize.top) {

+          clearInterval(updatePoller);

+          chrome.experimental.popup.getParentWindow().onWindowMoveCompleted(

+              offset,

+              initialSize,

+              newPosition);

+          window.close();

+        }

+      }, 50);

+    });

+  });
+}

+

+window.onload = function() {

+  // Delay invocation of the sizing test so that layout of the popup may

+  // complete.  On windows, onload is called before layout has been performed,

+  // so window.screenTop, and the other fields used in getCurrentWindowSize will

+  // return 0 until the layout has been performed.

+  // TODO: Fix the order of the onload and layout processing.

+  var positionPoller = setInterval(function() {

+    var initialSize = getCurrentWindowSize();

+    if (initialSize.width != 0) {

+      clearInterval(positionPoller);

+      doSizingValidation();

+    }

+  }, 50);

+}
+</script>
+</head>
+<body style='width:128px; height:128px'>
+Testing Popup Sizing
+</body>
+</html>
diff --git a/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js b/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js
index 76971d4..cf7cf4f 100644
--- a/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js
+++ b/chrome/test/data/extensions/api_test/tabs/basics/tabs_util.js
@@ -51,7 +51,8 @@ function createWindow(tabUrls, winOptions, callback) {
 
 // Waits until all tabs (yes, in every window) have status "complete".
 // This is useful to prevent test overlap when testing tab events.
-// |callback| should look like function() {...}.
+// |callback| should look like function() {...}.  Note that |callback| expects
+// zero arguments.
 function waitForAllTabs(callback) {
   // Wait for all tabs to load.
   function waitForTabs(){