alternate ntp: fix website page jankiness when suggestions show and top bars are hidden

chrome/browser/ui/views/frame/browser_view_layout.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/frame/browser_view_layout.h"
 
 #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"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/contents_container.h"
 #include "chrome/browser/ui/views/immersive_mode_controller.h"
 #include "chrome/browser/ui/views/infobars/infobar_container_view.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/toolbar_view.h"
 #include "ui/base/hit_test.h"
 #include "ui/gfx/point.h"
 #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 {
 
 // The visible height of the shadow above the tabs. Clicks in this area are
 // treated as clicks to the frame, rather than clicks to the tab.
 const int kTabShadowSize = 2;
 // The vertical overlap between the TabStrip and the Toolbar.
 const int kToolbarTabStripVerticalOverlap = 3;
 // The number of pixels the bookmark bar should overlap the spacer by if the
 // spacer is visible.
@@ skipping 215 matching lines @@
 
 void BrowserViewLayout::ViewRemoved(views::View* host, views::View* view) {
   switch (view->id()) {
     case VIEW_ID_BOOKMARK_BAR:
       active_bookmark_bar_ = NULL;
       break;
   }
 }
 
 void BrowserViewLayout::Layout(views::View* host) {
+  // Showing instant extended suggestions causes us to temporarily hide any
+  // visible bookmark bar and infobars.  In turn, this hiding would normally
+  // cause the content below the suggestions to shift upwards, which looks
+  // surprising (since from the user's perspective, we're "covering" rather than
+  // "removing" the bookmark bar/infobars).  To prevent this, we save off the
+  // content origin here, then once we finish laying things out, force the
+  // contents to continue to display from that origin.
+  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()) {
     int x = browser_view_->tabstrip_->GetMirroredX() +
         browser_view_->GetMirroredX() +
         browser_view_->frame()->GetThemeBackgroundXInset();
     browser_view_->tabstrip_->SetBackgroundOffset(gfx::Point(x,
         browser_view_->frame()->GetTabStripInsets(false).top));
   }
   top = LayoutToolbar(top);
   top = LayoutBookmarkAndInfoBars(top);
   // During immersive mode reveal the content stays near the top of the view.
   if (browser_view_->immersive_mode_controller()->IsRevealed()) {
     top = browser_view_->tabstrip_->y();
     if (!browser_view_->immersive_mode_controller()->hide_tab_indicators())
       top += TabStrip::GetImmersiveHeight();
   }
 
   int bottom = LayoutDownloadShelf(browser_view_->height());
   int active_top_margin = GetTopMarginForActiveContent();
   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.

[Peter Kasting, 2013/01/29 00:36:29]

Nit: How about just:

// Now set the contents to display at their previous origin if we just hid the
bookmark and/or infobars.

[kuan, 2013/01/29 01:00:11]

Done.

+  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);
+    int preview_height = contents_container_->preview_height();
+    // Only preserve the previous content origin if:
+    // - suggesions are not ready yet i.e. |preview_height| is 0
+    // - suggestions completely cover the bookmark bar/infobars that are hidden.

[Peter Kasting, 2013/01/29 00:36:29]

Nit: This confused me for a while.  See if this explanation is accurate instead:

// Special case: While normally the suggestions appear to "cover" any
bookmark/infobars, if the suggestions are very short, they might not fully cover
that gap, and leaving the contents at their original height would leave an
odd-looking blank space.  In this case, we allow the contents to go ahead and
shift upward.  (We don't shift up in the zero-preview-height case since that
corresponds to when the suggestions aren't actually ready to be displayed yet.)

This also makes me wonder: Do you really need to check for a zero preview height
here, if you ensure that we don't hide the bookmark bar/infobars until the
preview is ready?

[kuan, 2013/01/29 01:00:11]

Done.

The zero-preview-height case was what i tried, but failed, to explain in my
previous comments.

Layout() is triggered separately when bookmark bar and infobars are hidden.
Bookmark bar triggers Layout() from Browser class after ContentsContainer has
setup the preview for suggestions.
Infobar container triggers Layout() from itself, regardless if ContentsContainer
has setup the preview.  In this case, preview height cld be 0 in Layout() if
ContentsContainer hasn't setup the preview yet.

i slightly modified the last sentence of your comments to hopefully explain
this.

+    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
   // TabContentsContainer's bounds being up to date.
   if (browser()->HasFindBarController()) {
     browser()->GetFindBarController()->find_bar()->MoveWindowIfNecessary(
         gfx::Rect(), true);
   }
 }
 
@@ skipping 20 matching lines @@
     tabstrip->SetVisible(false);
     tabstrip->SetBounds(0, 0, 0, 0);
     return 0;
   }
   // This retrieves the bounds for the tab strip based on whether or not we show
   // anything to the left of it, like the incognito avatar.
   gfx::Rect tabstrip_bounds(
       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);
   tabstrip->SetBoundsRect(tabstrip_bounds);
   int bottom = tabstrip_bounds.bottom();
 
   // The metro window switcher sits at the far right edge of the tabstrip
   // a |kWindowSwitcherOffsetX| pixels from the right edge.
   // Only visible if there is more than one type of window to switch between.
   // TODO(mad): update this code when more window types than just incognito
@@ skipping 162 matching lines @@
   }
   return bottom;
 }
 
 bool BrowserViewLayout::InfobarVisible() const {
   views::View* infobar_container = browser_view_->infobar_container_;
   // NOTE: Can't check if the size IsEmpty() since it's always 0-width.
   return browser()->SupportsWindowFeature(Browser::FEATURE_INFOBAR) &&
       (infobar_container->GetPreferredSize().height() != 0);
 }