Make the TopContainerView always fully overlap the window header because we...

Make the TopContainerView always fully overlap the window header because we use the TopContainerView bounds in computing the find bar's position

BUG=341636
TEST=BrowserViewHostedAppTest.Layout

Review URL: https://codereview.chromium.org/160093006

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

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

@@ -523,13 +523,32 @@ void BrowserViewLayout::LayoutContentsContainerView(int top, int bottom) {
 }
 
 void BrowserViewLayout::UpdateTopContainerBounds() {
-  gfx::Rect top_container_bounds(top_container_->GetPreferredSize());
+  // Set the bounds of the top container view such that it is tall enough to
+  // fully show all of its children. In particular, the bottom of the bookmark
+  // bar can be above the bottom of the toolbar while the bookmark bar is
+  // animating. The top container view is positioned relative to the top of the
+  // client view instead of relative to GetTopInsetInBrowserView() because the
+  // top container view paints parts of the frame (title, window controls)
+  // during an immersive fullscreen reveal.
+  int height = 0;
+  for (int i = 0; i < top_container_->child_count(); ++i) {
+    views::View* child = top_container_->child_at(i);
+    if (!child->visible())
+      continue;
+    int child_bottom = child->bounds().bottom();
+    if (child_bottom > height)
+      height = child_bottom;
+  }
+
+  // Ensure that the top container view reaches the topmost view in the
+  // ClientView because the bounds of the top container view are used in
+  // layout and we assume that this is the case.
+  height = std::max(height, delegate_->GetTopInsetInBrowserView());
+
+  gfx::Rect top_container_bounds(vertical_layout_rect_.width(), height);
 
   // If the immersive mode controller is animating the top container, it may be
-  // partly offscreen. The top container is positioned relative to the top of
-  // the client view instead of relative to GetTopInsetInBrowserView() because
-  // the top container paints parts of the frame (title, window controls) during
-  // an immersive reveal.
+  // partly offscreen.
   top_container_bounds.set_y(
       immersive_mode_controller_->GetTopContainerVerticalOffset(
           top_container_bounds.size()));

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

@@ -103,6 +103,7 @@ TEST_F(BrowserViewTest, BrowserViewLayout) {
       toolbar->y());
   EXPECT_EQ(0, contents_container->x());
   EXPECT_EQ(toolbar->bounds().bottom(), contents_container->y());
+  EXPECT_EQ(top_container->bounds().bottom(), contents_container->y());
   EXPECT_EQ(0, devtools_web_view->x());
   EXPECT_EQ(0, devtools_web_view->y());
   EXPECT_EQ(0, contents_web_view->x());
@@ -164,6 +165,54 @@ TEST_F(BrowserViewTest, BrowserViewLayout) {
   BookmarkBarView::DisableAnimationsForTesting(false);
 }
 
+class BrowserViewHostedAppTest : public TestWithBrowserView {
+ public:
+  BrowserViewHostedAppTest()
+      : TestWithBrowserView(Browser::TYPE_POPUP,
+                            chrome::HOST_DESKTOP_TYPE_NATIVE,
+                            true) {
+  }
+  virtual ~BrowserViewHostedAppTest() {
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserViewHostedAppTest);
+};
+
+// Test basic layout for hosted apps.
+TEST_F(BrowserViewHostedAppTest, Layout) {
+  // Add a tab because the browser starts out without any tabs at all.
+  AddTab(browser(), GURL("about:blank"));
+
+  views::View* contents_container =
+      browser_view()->GetContentsContainerForTest();
+
+  // The tabstrip, toolbar and bookmark bar should not be visible for hosted
+  // apps.
+  EXPECT_FALSE(browser_view()->tabstrip()->visible());
+  EXPECT_FALSE(browser_view()->toolbar()->visible());
+  EXPECT_FALSE(browser_view()->IsBookmarkBarVisible());
+
+  gfx::Point header_offset;
+  views::View::ConvertPointToTarget(
+      browser_view(),
+      browser_view()->frame()->non_client_view()->frame_view(),
+      &header_offset);
+
+  // The position of the bottom of the header (the bar with the window
+  // controls) in the coordinates of BrowserView.
+  int bottom_of_header = browser_view()->frame()->GetTopInset() -
+      header_offset.y();
+
+  // The web contents should be flush with the bottom of the header.
+  EXPECT_EQ(bottom_of_header, contents_container->y());
+
+  // The find bar should overlap the 1px header/web-contents separator at the
+  // bottom of the header.
+  EXPECT_EQ(browser_view()->frame()->GetTopInset() - 1,
+            browser_view()->GetFindBarBoundingBox().y());
+}
+
 #if defined(OS_WIN)
 
 // This class provides functionality to test the incognito window/normal window

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

@@ -15,28 +15,6 @@ TopContainerView::TopContainerView(BrowserView* browser_view)
 TopContainerView::~TopContainerView() {
 }
 
-gfx::Size TopContainerView::GetPreferredSize() {
-  // The view wants to be as wide as its parent and tall enough to fully show
-  // all its children. In particular, the bottom of the bookmark bar can be
-  // be above the bottom of the toolbar while the bookmark bar is animating.
-  int height = 0;
-  for (int i = 0; i < child_count(); ++i) {
-    views::View* child = child_at(i);
-    if (!child->visible())
-      continue;
-    int child_bottom = child->bounds().bottom();
-    if (child_bottom > height)
-      height = child_bottom;
-  }
-  if (browser_view_->immersive_mode_controller()->IsRevealed()) {
-    // In immersive fullscreen, the TopContainerView paints the window header
-    // (themes, window title, window controls). The TopContainerView must still
-    // paint these even if it does not have any visible children.
-    height = std::max(height, browser_view_->frame()->GetTopInset());
-  }
-  return gfx::Size(browser_view_->width(), height);
-}
-
 const char* TopContainerView::GetClassName() const {
   return "TopContainerView";
 }

chrome/browser/ui/views/frame/top_container_view.h

@@ -13,15 +13,13 @@ class BrowserView;
 
 // Container for the BrowserView's tab strip, toolbar, and sometimes bookmark
 // bar. In Chrome OS immersive fullscreen it stacks on top of other views in
-// order to slide in and out over the web contents. It informs the immersive
-// mode controller when its children lose focus to trigger a slide out.
+// order to slide in and out over the web contents.
 class TopContainerView : public views::View {
  public:
   explicit TopContainerView(BrowserView* browser_view);
   virtual ~TopContainerView();
 
   // views::View overrides:
-  virtual gfx::Size GetPreferredSize() OVERRIDE;
   virtual const char* GetClassName() const OVERRIDE;
   virtual void OnPaintBackground(gfx::Canvas* canvas) OVERRIDE;