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

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

BUG=234863
Test=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=197257

[pkotwicz, 2013-04-25 21:47:53]

James, can you please take a look?

This CL is useful for making it clearer that the find bar should animate
whenever the top container animates.

This CL depends on https://codereview.chromium.org/13868010/.

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

Code looks correct and is an improvement, but one suggestion on making
coordinate systems clearer.

https://codereview.chromium.org/14230025/diff/1/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view_layout.cc (right):

https://codereview.chromium.org/14230025/diff/1/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view_layout.cc:188: int find_bar_y =
browser_view_->y() +
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.

https://codereview.chromium.org/14230025/diff/1/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view_layout.h (left):

https://codereview.chromium.org/14230025/diff/1/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view_layout.h:163: int find_bar_y_;
Oh, hooray, you've eliminated this!  I've been meaning to get rid of it. 
vertical_layout_rect_ needs to die too but I was going to try to write my unit
tests first.

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

James, can you please take another look?

https://codereview.chromium.org/14230025/diff/1/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view_layout.cc (right):

https://codereview.chromium.org/14230025/diff/1/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view_layout.cc:188: int find_bar_y =
browser_view_->y() +
- 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

https://codereview.chromium.org/14230025/diff/8001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/browser_view_layout.cc (right):

https://codereview.chromium.org/14230025/diff/8001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/browser_view_layout.cc:198: int height_delta =
bounding_box.y() - find_bar_y;
I flipped the sign of |height_delta|. This allows showing the find bar if the
browser window is sized small enough that only the infobars are visible. (You
can use www.corp.google.com/~pkotwicz/infobar for testing if you are interested)

[James Cook, 2013-04-27 01:22:04]

lgtm

https://codereview.chromium.org/14230025/diff/8001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/browser_view_layout.cc (right):

https://codereview.chromium.org/14230025/diff/8001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/browser_view_layout.cc:198: int height_delta =
bounding_box.y() - find_bar_y;
It looks like the height_delta computation was incorrect in the original code
(before your patches).  Good catch.

[pkotwicz, 2013-04-29 03:15:47]

sky for OWNERS

[sky, 2013-04-29 16:16:46]

LGTM

https://codereview.chromium.org/14230025/diff/8001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/browser_view_layout.cc (right):

https://codereview.chromium.org/14230025/diff/8001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/browser_view_layout.cc:193: int find_bar_y =
top_container_bounds.bottom() - 1;
Why the -1? Please document this since it isn't obvious.

[commit-bot: I haz the power, 2013-04-29 20:40:47]

Failed to apply the patch.

[commit-bot: I haz the power, 2013-04-30 00:23:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/14230025/25001

[commit-bot: I haz the power, 2013-04-30 03:42:57]

Change committed as 197257