MacViews: Implement Set/Get*Bounds for NativeWidgetMac

MacViews: Implement Set/Get*Bounds for NativeWidgetMac.

In toolkit-views, "Bounds" corresponds to the NSWindow frame (including
titlebar and borders). This CL maps that onto NSWindow in
InitNativeWidget, GetWindowBoundsInScreen, GetClientAreaBoundsInScreen,
SetBounds and SetSize, performing the required coordinate system
conversions.

5 more views_unittests pass after this:
BubbleDelegateTest.NonClientHitTest
CustomFrameViewTest.{CannotMaximizeHidesButton, DefaultButtonLayout,
LeadingButtonLayout}, ViewTest.ConversionsToFromScreen.

The CL also adds a cross-platform WidgetTest.GetWindowBoundsInScreen to
ensure the behavior is consistent across platforms, and because there
wasn't really anything to cover just bounds setting (e.g. to verify the
coordinate flipping Cocoa needs to do).

BUG=378134
TEST=views_unittests

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

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

[tapted, 2014-06-25 10:55:56]

Hi Robert and Scott, please take a look.

Note that on aura, client area bounds at this layer always match the window
(there's a NativeWidgetAuraTest [1] for this with a comment about the
distinction). But I'm pretty sure we need them different at this level on Mac.
(e.g. historically, it was ::SetWindowPos(), which includes the frame AFAIK).

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/widget/na...

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h
File ui/gfx/mac/point_utils.h (right):

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h...
ui/gfx/mac/point_utils.h:26: #endif  // UI_GFX_MAC_POINT_UTILS_H_
note: not sure it's the best filename, but I've put this in gfx since it will
come up a bit (e.g. anonymous::ConvertCoordinateSystem() in ui/gfx/screen_mac.mm
can use this, but that's orthogonal).

https://codereview.chromium.org/353643002/diff/80001/ui/views/widget/native_w...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/353643002/diff/80001/ui/views/widget/native_w...
ui/views/widget/native_widget_mac.mm:244: }
Not sure if calling -[NSWindow setContenSize:] is preferable to just

  SetBounds(gfx::Rect(GetWindowBoundsInScreen().origin(), size));

that alternative would result in more coordinate flipping, but it's "less" code
and might dovetail into an animated resize if we ever want that.

[sky, 2014-06-25 16:52:22]

LGTM

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h
File ui/gfx/mac/point_utils.h (right):

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h...
ui/gfx/mac/point_utils.h:26: #endif  // UI_GFX_MAC_POINT_UTILS_H_
On 2014/06/25 10:55:56, tapted wrote:
> note: not sure it's the best filename, but I've put this in gfx since it will
> come up a bit (e.g. anonymous::ConvertCoordinateSystem() in
ui/gfx/screen_mac.mm
> can use this, but that's orthogonal).

If you want this to be focused around screen conversions maybe it should be
screen_coordinate_conversions?

[Robert Sesek, 2014-06-25 17:06:05]

LGTM

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h
File ui/gfx/mac/point_utils.h (right):

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h...
ui/gfx/mac/point_utils.h:5: #ifndef UI_GFX_MAC_POINT_UTILS_H_
Tests for these functions would be good.

[Andre, 2014-06-25 17:59:51]

https://codereview.chromium.org/353643002/diff/80001/ui/views/widget/native_w...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/353643002/diff/80001/ui/views/widget/native_w...
ui/views/widget/native_widget_mac.mm:242: NSRect content_rect = [window
contentRectForFrameRect:frame_rect];
Isn't this supposed to set the window's frame instead of the content view's
frame?

[tapted, 2014-06-26 08:44:32]

Thanks all!

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h
File ui/gfx/mac/point_utils.h (right):

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h...
ui/gfx/mac/point_utils.h:5: #ifndef UI_GFX_MAC_POINT_UTILS_H_
On 2014/06/25 17:06:04, rsesek wrote:
> Tests for these functions would be good.

Done in https://codereview.chromium.org/345243007/ - needs to touch a few files
to move ScopedClassSwizzler to base, and it will conflict with the
EventGenerator CL that's scheduled for an OWNERS review next week, so I'll
publish when that's through.

https://codereview.chromium.org/353643002/diff/80001/ui/gfx/mac/point_utils.h...
ui/gfx/mac/point_utils.h:26: #endif  // UI_GFX_MAC_POINT_UTILS_H_
On 2014/06/25 16:52:22, sky wrote:
> On 2014/06/25 10:55:56, tapted wrote:
> > note: not sure it's the best filename, but I've put this in gfx since it
will
> > come up a bit (e.g. anonymous::ConvertCoordinateSystem() in
> ui/gfx/screen_mac.mm
> > can use this, but that's orthogonal).
> 
> If you want this to be focused around screen conversions maybe it should be
> screen_coordinate_conversions?

Done - went with coordinate_conversions.h/mm (discovered
ash/wm/coordinate_conversions, and this is performing a similar role).

https://codereview.chromium.org/353643002/diff/80001/ui/views/widget/native_w...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/353643002/diff/80001/ui/views/widget/native_w...
ui/views/widget/native_widget_mac.mm:242: NSRect content_rect = [window
contentRectForFrameRect:frame_rect];
On 2014/06/25 17:59:51, Andre wrote:
> Isn't this supposed to set the window's frame instead of the content view's
> frame?

This part of it was OK -- there's no -[NSWindow setFrameSize] -- just
setContentSize, so I was converting it to use the available NSWindow "resize"
function. But!: turns out that function is no good anyway. To be consistent with
views we need to keep the top-left corner of a window in the same place when
resizing. But setContentSize will keep the bottom-left corner in the same place,
so it's not what we want.

So, I've gone with my suggestion from an earlier comment, below, and added more
checks in the test to ensure the gfx::Bounds() origin doesn't move when
resizing. Also a comment here about it.

[tapted, 2014-06-26 08:45:05]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2014-06-26 08:46:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tapted@chromium.org/353643002/140001

[commit-bot: I haz the power, 2014-06-26 10:04:25]

Change committed as 279975

[tapted, 2014-06-26 10:23:34]

A revert of this CL has been created in
https://codereview.chromium.org/351603004/ by tapted@chromium.org.

The reason for reverting is: Breaks on 64-bit mac (doh). NSRect is declared
differently, so a forward declare won't work..

[tapted, 2014-06-26 12:38:32]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2014-06-26 12:39:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tapted@chromium.org/353643002/220001

[commit-bot: I haz the power, 2014-06-26 14:37:26]

Change committed as 280005