Allow popups using the new frames to be sized correctly. This involved a slight tweak to how RestoreWindowPosition on Window works - if the window is opened with specified bounds, we still ask the delegate to try and restore the position. The delegate can use the provided bounds as a hint (in the popup case the bounds aren't the true layout bounds of the window, rather the rect contains the window position and content area size) to return the actual bounds of the window.

B=1031854


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

2 files changed, 67 insertions, 5 deletions

diff --git a/chrome/browser/views/constrained_window_impl.cc b/chrome/browser/views/constrained_window_impl.cc
index 3dfdc20..cfcdee0 100644
--- a/chrome/browser/views/constrained_window_impl.cc
+++ b/chrome/browser/views/constrained_window_impl.cc
@@ -1233,12 +1233,36 @@ void ConstrainedWindowImpl::Detach() {
   // DetachContents, but we clear the delegate pointing to us just in case.
   constrained_contents_->set_delegate(NULL);
 
+  // We want to detach the constrained window at the same position on screen
+  // as the constrained window, so we need to get its screen bounds.
+  CRect constrained_window_bounds;
+  GetBounds(&constrained_window_bounds, true);
+
+  // Obtain the constrained TabContents' size from its HWND...
   CRect bounds;
   ::GetWindowRect(constrained_contents_->GetContainerHWND(), &bounds);
+
+  // ... but overwrite its screen position with the screen position of its
+  // containing ConstrainedWindowImpl. We do this because the code called by
+  // |DetachContents| assumes the bounds contains position and size information
+  // similar to what is sent when a popup is not suppressed and must be opened,
+  // i.e. the position is the screen position of the top left of the detached
+  // popup window, and the size is the size of the content area.
+  bounds.SetRect(constrained_window_bounds.left, constrained_window_bounds.top,
+                 constrained_window_bounds.left + bounds.Width(),
+                 constrained_window_bounds.top + bounds.Height());
+
+  // Save the cursor position so that we know where to send a mouse message
+  // when the new detached window is created.
   CPoint cursor_pos;
   ::GetCursorPos(&cursor_pos);
   gfx::Point screen_point(cursor_pos.x, cursor_pos.y);
+
+  // Determine what aspect of the constrained frame was clicked on, so that we
+  // can continue the mouse move on this aspect of the detached frame.
   int frame_component = static_cast<int>(OnNCHitTest(screen_point.ToPOINT()));
+
+  // Finally we actually detach the TabContents, and then clean up.
   owner_->DetachContents(this, constrained_contents_, gfx::Rect(bounds),
                          screen_point, frame_component);
   constrained_contents_ = NULL;
@@ -1345,7 +1369,7 @@ LRESULT ConstrainedWindowImpl::OnMouseActivate(HWND window,
   // We only detach the window if the user clicked on the title bar. That
   // way, users can click inside the contents of legitimate popups obtained
   // with a mouse gesture.
-  if (hittest_code == HTCAPTION) {
+  if (hittest_code != HTCLIENT && hittest_code != HTNOWHERE) {
     ActivateConstrainedWindow();
   } else {
     // If the user did not click on the title bar, don't stop message
diff --git a/chrome/browser/views/frame/browser_view2.cc b/chrome/browser/views/frame/browser_view2.cc
index 8ef7dd3..bc667cd 100644
--- a/chrome/browser/views/frame/browser_view2.cc
+++ b/chrome/browser/views/frame/browser_view2.cc
@@ -61,6 +61,7 @@ static const int kStatusBubbleOffset = 2;
 static const int kSeparationLineHeight = 1;
 static const SkColor kSeparationLineColor = SkColorSetRGB(178, 178, 178);
 static const wchar_t* kBrowserWindowKey = L"__BROWSER_WINDOW__";
+static const int kWindowTilePixels = 10;
 
 
 static const struct { bool separator; int command; int label; } kMenuLayout[] = {
@@ -573,10 +574,47 @@ bool BrowserView2::RestoreWindowPosition(CRect* bounds,
                                          bool* maximized,
                                          bool* always_on_top) {
   DCHECK(bounds && maximized && always_on_top);
-  // TODO(beng): (http://b/1317622) Make these functions take gfx::Rects.
-  gfx::Rect b;
-  browser_->RestoreWindowPosition(&b, maximized);
-  *bounds = b.ToRECT();
+  *always_on_top = false;
+
+  if (browser_->GetType() == BrowserType::BROWSER) {
+    // We are a popup window. The value passed in |bounds| represents two
+    // pieces of information:
+    // - the position of the window, in screen coordinates (outer position).
+    // - the size of the content area (inner size).
+    // We need to use these values to determine the appropriate size and
+    // position of the resulting window.
+    if (SupportsWindowFeature(FEATURE_TOOLBAR) ||
+        SupportsWindowFeature(FEATURE_LOCATIONBAR)) {
+      // If we're showing the toolbar, we need to adjust |*bounds| to include
+      // its desired height, since the toolbar is considered part of the
+      // window's client area as far as GetWindowBoundsForClientBounds is
+      // concerned...
+      CSize ps;
+      toolbar_->GetPreferredSize(&ps);
+      bounds->bottom += ps.cy;
+    }
+
+    gfx::Rect window_rect =
+        frame_->GetWindowBoundsForClientBounds(gfx::Rect(*bounds));
+    window_rect.set_origin(gfx::Point(bounds->left, bounds->top));
+
+    // When we are given x/y coordinates of 0 on a created popup window,
+    // assume none were given by the window.open() command.
+    if (window_rect.x() == 0 && window_rect.y() == 0) {
+      gfx::Point origin = GetNormalBounds().origin();
+      origin.set_x(origin.x() + kWindowTilePixels);
+      origin.set_y(origin.y() + kWindowTilePixels);
+      window_rect.set_origin(origin);
+    }
+
+    *bounds = window_rect.ToRECT();
+    *maximized = false;
+  } else {
+    // TODO(beng): (http://b/1317622) Make these functions take gfx::Rects.
+    gfx::Rect b;
+    browser_->RestoreWindowPosition(&b, maximized);
+    *bounds = b.ToRECT();
+  }
 
   // We return true because we can _always_ locate reasonable bounds using the
   // WindowSizer, and we don't want to trigger the Window's built-in "size to