Index: chrome/browser/ui/views/frame/browser_view_layout.cc |
diff --git a/chrome/browser/ui/views/frame/browser_view_layout.cc b/chrome/browser/ui/views/frame/browser_view_layout.cc |
index 00311c5eb48adc645296308410b000f51fd54005..37bad4755153cd38fb94856e8a0797e80509ebfb 100644 |
--- a/chrome/browser/ui/views/frame/browser_view_layout.cc |
+++ b/chrome/browser/ui/views/frame/browser_view_layout.cc |
@@ -7,6 +7,7 @@ |
#include "chrome/browser/ui/browser_finder.h" |
#include "chrome/browser/ui/find_bar/find_bar.h" |
#include "chrome/browser/ui/find_bar/find_bar_controller.h" |
+#include "chrome/browser/ui/search/search_model.h" |
#include "chrome/browser/ui/view_ids.h" |
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h" |
#include "chrome/browser/ui/views/download/download_shelf_view.h" |
@@ -22,6 +23,7 @@ |
#include "ui/gfx/scrollbar_size.h" |
#include "ui/gfx/size.h" |
#include "ui/views/controls/single_split_view.h" |
+#include "ui/views/controls/webview/webview.h" |
namespace { |
@@ -257,6 +259,35 @@ void BrowserViewLayout::ViewRemoved(views::View* host, views::View* view) { |
} |
void BrowserViewLayout::Layout(views::View* host) { |
+ // If pinned bookmark bar and infobars are hidden because suggestions are |
+ // showing on a website page, the real estate released by the hidden bars |
+ // after this layout will cause the contents to shift up visually, resulting |
+ // in jankiness. To prevent the contents from shifting up, remember its |
+ // origin (relative to |BrowserView|) before this layout starts, so that it |
+ // can be restored after this layout finishes. |
+ // Another way to preserve the contents page position is to determine the |
+ // total height of all hidden bars. While that's possible with bookmark bar |
+ // (i.e active_bookmark_bar_->height() - |
+ // views::NonClientFrameView::kClientEdgeThickness + |
+ // active_bookmark_bar_->GetToolbarOverlap(false)), |
+ // it's not possible with infobar container. By the time Layout() is called, |
+ // infobar container has already hidden all its bars, causing its |
+ // GetVerticalOverlap() to return 0; as a result, the actual height of the |
+ // infobars without the overlap cannot be determined. |
Peter Kasting
2013/01/28 21:51:00
This comment confused the heck out of me.
How abo
kuan
2013/01/29 00:17:08
Done.
|
+ const chrome::search::Mode& mode = browser()->search_model()->mode(); |
+ views::WebView* contents = browser_view_->contents_container_; |
+ gfx::Point old_contents_origin; |
+ if (mode.is_search_suggestions() && mode.is_origin_default() && |
+ // Bookmark bar is visible now but will be hidden during layout. |
+ ((active_bookmark_bar_ && active_bookmark_bar_->visible() && |
+ browser()->bookmark_bar_state() == BookmarkBar::HIDDEN) || |
+ // Infobar container is visible now but will be hidden during layout. |
+ (browser_view_->infobar_container_->visible() && !InfobarVisible()))) { |
+ old_contents_origin = contents->bounds().origin(); |
+ views::View::ConvertPointToTarget(contents->parent(), browser_view_, |
+ &old_contents_origin); |
+ } |
+ |
vertical_layout_rect_ = browser_view_->GetLocalBounds(); |
int top = LayoutTabStripRegion(); |
if (browser_view_->IsTabStripVisible()) { |
@@ -280,6 +311,37 @@ void BrowserViewLayout::Layout(views::View* host) { |
top -= active_top_margin; |
contents_container_->SetActiveTopMargin(active_top_margin); |
LayoutTabContents(top, bottom); |
+ |
+ // If previous origin of contents page relative to |BrowserView| needs to be |
+ // preserved, set the vertical difference between new and previous origins as |
+ // the active top margin in contents container, so that the contents don't |
+ // shift up due to hiding of bookmark and info bars. |
+ if (active_top_margin == 0 && !old_contents_origin.IsOrigin()) { |
+ gfx::Point new_contents_origin(contents->bounds().origin()); |
+ views::View::ConvertPointToTarget(contents->parent(), browser_view_, |
+ &new_contents_origin); |
+ active_top_margin = old_contents_origin.y() - new_contents_origin.y(); |
+ DCHECK_GE(active_top_margin, 0); |
+ // If ContentsContainer::SetPreview() has not been called yet, preview |
+ // height will be 0, in which case, preserve the previous contents y origin, |
+ // so that it can be restored in an upcoming Layout() call after the preview |
+ // has been set. |
+ // Otherwise, if preview is set, preserve previous contents origin only if |
+ // preview is taller than all hidden bars, so as to prevent a blank area |
+ // from showing above the contents (i.e. below the preview). |
+ // Note that if Layout() is triggered by hiding of: |
+ // - bookmark bar, preview would have been set because bookmark bar state is |
+ // updated from |Browser| after |InstantPreviewControllerView| receives |
+ // the notification that preview is ready and sets it. |
+ // - infobars, there's no guarantee that preview has been set; if infobar |
+ // container receives instant-preview-ready notification before |
+ // |InstantPreviewControllerView|, there won't be preview here yet; |
+ // hence the speical handling of the 0 preview height case. |
Peter Kasting
2013/01/28 21:51:00
Again, all these comments are really confusing. D
kuan
2013/01/29 00:17:08
Done. i apologize my attempt to explain everythin
|
+ int preview_height = contents_container_->preview_height(); |
+ if (preview_height == 0 || active_top_margin < preview_height) |
+ contents_container_->SetActiveTopMargin(active_top_margin); |
+ } |
+ |
// This must be done _after_ we lay out the WebContents since this |
// code calls back into us to find the bounding box the find bar |
// must be laid out within, and that code depends on the |
@@ -320,7 +382,7 @@ int BrowserViewLayout::LayoutTabStripRegion() { |
browser_view_->frame()->GetBoundsForTabStrip(tabstrip)); |
gfx::Point tabstrip_origin(tabstrip_bounds.origin()); |
views::View::ConvertPointToTarget(browser_view_->parent(), browser_view_, |
- &tabstrip_origin); |
+ &tabstrip_origin); |
tabstrip_bounds.set_origin(tabstrip_origin); |
tabstrip->SetVisible(true); |