Make the relationship between the find bar position and the top container height more obvious

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

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 a4c17d6e8c75ffa5ccf86ab53ae155c48e6e55a4..046933222fc21725109b7ae5199483e47456cb0a 100644
--- a/chrome/browser/ui/views/frame/browser_view_layout.cc
+++ b/chrome/browser/ui/views/frame/browser_view_layout.cc
@@ -113,7 +113,6 @@ BrowserViewLayout::BrowserViewLayout()
       contents_split_(NULL),
       contents_container_(NULL),
       download_shelf_(NULL),
-      find_bar_y_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           dialog_host_(new WebContentsModalDialogHostViews(this))),
       web_contents_modal_dialog_top_y_(-1) {
@@ -174,24 +173,26 @@ gfx::Size BrowserViewLayout::GetMinimumSize() {
 }
 
 gfx::Rect BrowserViewLayout::GetFindBarBoundingBox() const {
-  // This function returns the area the Find Bar can be laid out
-  // within. This basically implies the "user-perceived content
-  // area" of the browser window excluding the vertical
-  // scrollbar. This is not quite so straightforward as positioning
-  // based on the TabContentsContainer since the BookmarkBarView may
-  // be visible but not persistent (in the New Tab case) and we
-  // position the Find Bar over the top of it in that case since the
-  // BookmarkBarView is not _visually_ connected to the Toolbar.
+  // This function returns the area the Find Bar can be laid out within. This
+  // basically implies the "user-perceived content area" of the browser
+  // window excluding the vertical scrollbar. The "user-perceived content area"
+  // excludes the detached bookmark bar (in the New Tab case) and any infobars
+  // since they are not _visually_ connected to the Toolbar.
 
   // First determine the bounding box of the content area in Widget
   // coordinates.
   gfx::Rect bounding_box = contents_container_->ConvertRectToWidget(
       contents_container_->GetLocalBounds());
 
-  // Adjust the position and size of the bounding box by the find bar offset
-  // calculated during the last Layout.
-  int height_delta = find_bar_y_ - bounding_box.y();
-  bounding_box.set_y(find_bar_y_);
+  // The find bar is postioned right below the top container.
+  int find_bar_y = browser_view_->y() +

[James Cook, 2013/04/26 16:14:51]

I think this would be clearer if you either did this computation with
ConvertPointToWidget to emphasize that all math is done in Widget coordinates,
or changed the above ConvertRectToWidget to do all the math in BrowserView
coordinates.

I would also document the coordinate system of the returned gfx::Rect of this
function.

[pkotwicz, 2013/04/27 00:02:43]

- Converted explicitly to widget coordinates.
- Added comment to browser_view_layout.h to document the coordinate system. A
better comment is already present in browser_view.h

+      browser_view_->top_container()->bounds().bottom() - 1;
+
+  // Grow the height of |bounding_box| by the height of any elements between
+  // the top container and |contents_container_| such as the detached bookmark
+  // bar and any infobars.
+  int height_delta = find_bar_y - bounding_box.y();
+  bounding_box.set_y(find_bar_y);
   bounding_box.set_height(std::max(0, bounding_box.height() + height_delta));
 
   // Finally decrease the width of the bounding box by the width of
@@ -412,7 +413,6 @@ int BrowserViewLayout::LayoutToolbar(int top) {
 int BrowserViewLayout::LayoutBookmarkAndInfoBars(int top) {
   web_contents_modal_dialog_top_y_ =
       top + browser_view_->y() - kConstrainedWindowOverlap;
-  find_bar_y_ = top + browser_view_->y() - 1;
   if (bookmark_bar_) {
     // If we're showing the Bookmark bar in detached style, then we
     // need to show any Info bar _above_ the Bookmark bar, since the
@@ -423,7 +423,6 @@ int BrowserViewLayout::LayoutBookmarkAndInfoBars(int top) {
     top = std::max(browser_view_->toolbar_->bounds().bottom(),
                    LayoutBookmarkBar(top));
   }
-  find_bar_y_ = top + browser_view_->y() - 1;
   return LayoutInfoBar(top);
 }