From 4f64d0af5908e36d356c834005d08cca98d579fe Mon Sep 17 00:00:00 2001 From: "beng@google.com" Date: Wed, 20 Aug 2008 22:07:28 +0000 Subject: 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 --- chrome/browser/browser.cc | 90 +++++++++++++++---------- chrome/browser/browser_window.h | 1 + chrome/browser/views/constrained_window_impl.cc | 26 ++++++- chrome/browser/views/frame/browser_view2.cc | 46 +++++++++++-- chrome/views/window.cc | 19 +++--- 5 files changed, 131 insertions(+), 51 deletions(-) diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 63c91ea..8015f26 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -240,6 +240,8 @@ Browser::Browser(const gfx::Rect& initial_bounds, // See note where SIZE_TO_CONTENTS is defined in browser.h for an explanation // of this hack. if (show_command == SIZE_TO_CONTENTS) { + // This codepath is deprecated with the new frames. + DCHECK(!g_browser_process->IsUsingNewFrames()); // SizeToContents causes a Layout so make sure the tab strip and toolbar // are already initialized. window_->SizeToContents(initial_bounds); @@ -755,20 +757,35 @@ void Browser::StartDraggingDetachedContents(TabContents* source, const gfx::Rect& contents_bounds, const gfx::Point& mouse_pt, int frame_component) { - BrowserType::Type new_type = BrowserType::BROWSER; - - // If this is a minimal chrome browser, propagate to detached contents to - // avoid having URL fields in popups. - if (type_ == BrowserType::APPLICATION) - new_type = type_; + if (!g_browser_process->IsUsingNewFrames()) { + BrowserType::Type new_type = BrowserType::BROWSER; - Browser* browser = new Browser(contents_bounds, SIZE_TO_CONTENTS, profile_, - new_type, L""); - browser->AddNewContents( - source, new_contents, NEW_FOREGROUND_TAB, contents_bounds, true); - browser->Show(); - browser->window_->ContinueDetachConstrainedWindowDrag( - mouse_pt, frame_component); + // If this is a minimal chrome browser, propagate to detached contents to + // avoid having URL fields in popups. + if (type_ == BrowserType::APPLICATION) + new_type = type_; + + Browser* browser = new Browser(contents_bounds, SIZE_TO_CONTENTS, profile_, + new_type, L""); + browser->AddNewContents( + source, new_contents, NEW_FOREGROUND_TAB, contents_bounds, true); + browser->Show(); + browser->window_->ContinueDetachConstrainedWindowDrag( + mouse_pt, frame_component); + } else { + // If we're inside an application frame, preserve that type (i.e. don't + // show a location bar on the new window), otherwise open a tab-less + // browser window with a location bar. + BrowserType::Type new_type = + type_ == BrowserType::APPLICATION ? type_ : BrowserType::BROWSER; + Browser* browser = new Browser(contents_bounds, SW_SHOWNORMAL, profile_, + BrowserType::BROWSER, std::wstring()); + browser->AddNewContents(source, new_contents, + NEW_FOREGROUND_TAB, gfx::Rect(), true); + browser->Show(); + browser->window()->ContinueDetachConstrainedWindowDrag(mouse_pt, + frame_component); + } } void Browser::ActivateContents(TabContents* contents) { @@ -926,7 +943,6 @@ StatusBubble* Browser::GetStatusBubble() { // Called whenever the window is moved so that we can update the position // of any WS_POPUP HWNDs. -// TODO(beng): This should move to BrowserView2! void Browser::WindowMoved() { DCHECK(!g_browser_process->IsUsingNewFrames()); GetStatusBubble()->Reposition(); @@ -1665,32 +1681,34 @@ void Browser::ConvertToTabbedBrowser() { void Browser::BuildPopupWindow(TabContents* source, TabContents* new_contents, const gfx::Rect& initial_pos) { - Browser* browser = new Browser(gfx::Rect(), SW_SHOWNORMAL, profile_, + Browser* browser = new Browser(initial_pos, SW_SHOWNORMAL, profile_, BrowserType::BROWSER, std::wstring()); browser->AddNewContents(source, new_contents, NEW_FOREGROUND_TAB, gfx::Rect(), true); - // TODO(beng): (1031854) Move most of this to the frames!! - // For newly opened popup windows, the incoming width/height - // numbers are for the content area, but x/y are for the actual - // window position. Thus we can't just call MoveContents(). - gfx::Rect window_rect = - browser->window()->GetBoundsForContentBounds(initial_pos); - window_rect.set_origin(initial_pos.origin()); - - // 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 = window()->GetNormalBounds().origin(); - origin.set_x(origin.x() + kWindowTilePixels); - origin.set_y(origin.y() + kWindowTilePixels); - window_rect.set_origin(origin); - } - - ::SetWindowPos(browser->GetTopLevelHWND(), NULL, - window_rect.x(), window_rect.y(), - window_rect.width(), window_rect.height(), 0); - win_util::AdjustWindowToFit(browser->GetTopLevelHWND()); + if (!g_browser_process->IsUsingNewFrames()) { + // TODO(beng): (1031854) Move most of this to the frames!! + // For newly opened popup windows, the incoming width/height + // numbers are for the content area, but x/y are for the actual + // window position. Thus we can't just call MoveContents(). + gfx::Rect window_rect = + browser->window()->GetBoundsForContentBounds(initial_pos); + window_rect.set_origin(initial_pos.origin()); + + // 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 = window()->GetNormalBounds().origin(); + origin.set_x(origin.x() + kWindowTilePixels); + origin.set_y(origin.y() + kWindowTilePixels); + window_rect.set_origin(origin); + } + + ::SetWindowPos(browser->GetTopLevelHWND(), NULL, + window_rect.x(), window_rect.y(), + window_rect.width(), window_rect.height(), 0); + win_util::AdjustWindowToFit(browser->GetTopLevelHWND()); + } browser->Show(); } diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h index c5a9420..2b5093d 100644 --- a/chrome/browser/browser_window.h +++ b/chrome/browser/browser_window.h @@ -131,6 +131,7 @@ class BrowserWindow { // Sizes the frame to match the specified desired bounds for the contents. // |contents_bounds| are in screen coordinates. + // TODO(beng): REMOVE virtual void SizeToContents(const gfx::Rect& contents_bounds) = 0; // Set the accelerator table. This is called once after LoadAccelerators 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(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 diff --git a/chrome/views/window.cc b/chrome/views/window.cc index 681a802..9cf1763 100644 --- a/chrome/views/window.cc +++ b/chrome/views/window.cc @@ -490,15 +490,8 @@ void Window::SetInitialFocus() { } void Window::SetInitialBounds(const gfx::Rect& create_bounds) { - // If we were created with some bounds, use those instead of trying to figure - // out the initial size from other sources. - if (!create_bounds.IsEmpty()) { - SetBounds(create_bounds); - return; - } - // Restore the window's placement from the controller. - CRect saved_bounds(0, 0, 0, 0); + CRect saved_bounds(create_bounds.ToRECT()); bool maximized = false; if (window_delegate_->RestoreWindowPosition(&saved_bounds, &maximized, @@ -528,8 +521,14 @@ void Window::SetInitialBounds(const gfx::Rect& create_bounds) { if (is_always_on_top_ != window_delegate_->IsAlwaysOnTop()) AlwaysOnTopChanged(); } else { - // Size the window to the content and center over the parent. - SizeWindowToDefault(); + if (create_bounds.IsEmpty()) { + // No initial bounds supplied, so size the window to its content and + // center over its parent. + SizeWindowToDefault(); + } else { + // Use the supplied initial bounds. + SetBounds(create_bounds); + } } } -- cgit v1.1