A couple of fixes to bubble view:

A couple of fixes to bubble view:
1. Mirror arrow location even for centered arrows if the bubble is offscreen
2. Apply insets to |anchor_point|. This give the user the ability to specify a
generic "anchor_rect" (as a combination of center point and insets) instead of
either a |anchor_point| or an |anchor_view|.

BUG=none


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

[msw, 2013-03-14 20:17:58]

Awesome, the test probably needs work, and just a couple other nits.

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_del...
File ui/views/bubble/bubble_delegate.cc (right):

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_del...
ui/views/bubble/bubble_delegate.cc:234: gfx::Rect anchor_bounds = anchor_view()?
anchor_view()->GetBoundsInScreen() :
nit: space before '?'

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
File ui/views/bubble/bubble_frame_view.cc (right):

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
ui/views/bubble/bubble_frame_view.cc:187:
MirrorArrowIfOffScreen(BubbleBorder::is_arrow_on_horizontal(arrow),
nit: Add a minor comment like "Mirror as needed vertically if the arrow is on a
horizontal edge and vice-versa."

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
File ui/views/bubble/bubble_frame_view_unittest.cc (right):

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
ui/views/bubble/bubble_frame_view_unittest.cc:231: TEST_F(BubbleFrameViewTest,
TestMirroringForCenteredArrow) {
I suspect this test will fail if the test machine has a larger screen size.
Can you generalize the anchor positioning to fit near the edge of any screen?
See gfx::Screen::GetScreenFor(...)->GetDisplayNearestWindow(...).bounds();

[varunjain, 2013-03-14 20:54:59]

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_del...
File ui/views/bubble/bubble_delegate.cc (right):

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_del...
ui/views/bubble/bubble_delegate.cc:234: gfx::Rect anchor_bounds = anchor_view()?
anchor_view()->GetBoundsInScreen() :
On 2013/03/14 20:17:58, msw wrote:
> nit: space before '?'

Done.

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
File ui/views/bubble/bubble_frame_view.cc (right):

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
ui/views/bubble/bubble_frame_view.cc:187:
MirrorArrowIfOffScreen(BubbleBorder::is_arrow_on_horizontal(arrow),
On 2013/03/14 20:17:58, msw wrote:
> nit: Add a minor comment like "Mirror as needed vertically if the arrow is on
a
> horizontal edge and vice-versa."

Done.

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
File ui/views/bubble/bubble_frame_view_unittest.cc (right):

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
ui/views/bubble/bubble_frame_view_unittest.cc:231: TEST_F(BubbleFrameViewTest,
TestMirroringForCenteredArrow) {
On 2013/03/14 20:17:58, msw wrote:
> I suspect this test will fail if the test machine has a larger screen size.
> Can you generalize the anchor positioning to fit near the edge of any screen?
> See gfx::Screen::GetScreenFor(...)->GetDisplayNearestWindow(...).bounds();

I think the TestBubbleFrameView overrides the monitor bounds to 1000x1000 (line
70). So It does not matter what screen size the test runs on.

[msw, 2013-03-14 20:59:43]

LGTM with a nit; thanks!

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
File ui/views/bubble/bubble_frame_view_unittest.cc (right):

https://codereview.chromium.org/12545040/diff/4001/ui/views/bubble/bubble_fra...
ui/views/bubble/bubble_frame_view_unittest.cc:231: TEST_F(BubbleFrameViewTest,
TestMirroringForCenteredArrow) {
On 2013/03/14 20:54:59, varunjain wrote:
> On 2013/03/14 20:17:58, msw wrote:
> > I suspect this test will fail if the test machine has a larger screen size.
> > Can you generalize the anchor positioning to fit near the edge of any
screen?
> > See gfx::Screen::GetScreenFor(...)->GetDisplayNearestWindow(...).bounds();
> 
> I think the TestBubbleFrameView overrides the monitor bounds to 1000x1000
(line
> 70). So It does not matter what screen size the test runs on.

Good call!

https://codereview.chromium.org/12545040/diff/10001/ui/views/bubble/bubble_fr...
File ui/views/bubble/bubble_frame_view.cc (right):

https://codereview.chromium.org/12545040/diff/10001/ui/views/bubble/bubble_fr...
ui/views/bubble/bubble_frame_view.cc:190: anchor_rect, client_size);
nit: indent to match the open paren above.

[varunjain, 2013-03-14 21:15:08]

https://codereview.chromium.org/12545040/diff/10001/ui/views/bubble/bubble_fr...
File ui/views/bubble/bubble_frame_view.cc (right):

https://codereview.chromium.org/12545040/diff/10001/ui/views/bubble/bubble_fr...
ui/views/bubble/bubble_frame_view.cc:190: anchor_rect, client_size);
On 2013/03/14 20:59:44, msw wrote:
> nit: indent to match the open paren above.

Done.

[commit-bot: I haz the power, 2013-03-14 21:19:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/12545040/15001

[commit-bot: I haz the power, 2013-03-15 07:30:41]

Change committed as 188285