[Views] Fix WidgetTest.GetRestoredBounds

[Views] Fix WidgetTest.GetRestoredBounds

Test was failing on Mac because ViewsTestHelperMac::TearDown()
now DCHECKs that all Widgets are closed (added in r334985). Fix
this by closing the Widget at end of test.

Also re-enable this test on Win. It's unfortunately still broken on
Linux (see bug).

Also re-write the test a bit to make it clearer.

BUG=378134, 515369
Committed: https://crrev.com/1456431501bdaf8c9362e25343b7393126a722af
Cr-Commit-Position: refs/heads/master@{#341643}

[jackhou1, 2015-07-30 02:49:40]

jackhou@chromium.org changed reviewers:
+ tapted@chromium.org

[jackhou1, 2015-07-30 02:49:41]

tapted, PTAL

[tapted, 2015-07-30 03:04:35]

you have almost depleted my yak cannon

https://codereview.chromium.org/1268643002/diff/1/ui/views/widget/widget_unit...
File ui/views/widget/widget_unittest.cc (right):

https://codereview.chromium.org/1268643002/diff/1/ui/views/widget/widget_unit...
ui/views/widget/widget_unittest.cc:1286: #if defined(OS_MACOSX)
I wonder.... should we try

#if !defined(OS_CHROMEOS)?

Then use CreateNativeDesktopWidget() below. I think that will work.

Then the TODO and NativeWidgetGtk comments can probably be removed. (And the
Aura comment could be something like "Non-Desktop widgets need the shell to..."

[jackhou1, 2015-07-30 06:18:14]

https://codereview.chromium.org/1268643002/diff/1/ui/views/widget/widget_unit...
File ui/views/widget/widget_unittest.cc (right):

https://codereview.chromium.org/1268643002/diff/1/ui/views/widget/widget_unit...
ui/views/widget/widget_unittest.cc:1286: #if defined(OS_MACOSX)
On 2015/07/30 03:04:35, tapted wrote:
> I wonder.... should we try
> 
> #if !defined(OS_CHROMEOS)?
> 
> Then use CreateNativeDesktopWidget() below. I think that will work.
> 
> Then the TODO and NativeWidgetGtk comments can probably be removed. (And the
> Aura comment could be something like "Non-Desktop widgets need the shell
to..."

Done. But then disabled on Linux because it doesn't work.

[tapted, 2015-07-30 06:50:18]

lgtm - just some nits

Looks like there might already be a bug for the window position thing --
http://crbug.com/375779 . It's RVG though, so maybe a new bug is fine. (I guess
you can mark the new bug as blocking the old one)

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
File ui/views/widget/widget_unittest.cc (right):

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1288: #if !defined(OS_CHROMEOS) &&
!defined(OS_LINUX)
I think we can do the MAYBE_GetRestoredbounds thing - will get us slightly less
nesting

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1291: ui::test::ScopedFakeNSWindowFullscreen
fake_fullscreen;
nit: comment here like

// Fullscreen on Mac requires an interactive UI test, but we can fake them here.

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1297:
EXPECT_GT(toplevel->GetRestoredBounds().width(), 0);
nit: EXPECT_FALSE(toplevel->GetRestoredBounds().IsEmpty());

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1300: gfx::Rect bounds(100, 100, 200, 200);
nit: const?

[jackhou1, 2015-07-30 07:25:04]

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
File ui/views/widget/widget_unittest.cc (right):

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1288: #if !defined(OS_CHROMEOS) &&
!defined(OS_LINUX)
On 2015/07/30 06:50:18, tapted wrote:
> I think we can do the MAYBE_GetRestoredbounds thing - will get us slightly
less
> nesting

Done.

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1291: ui::test::ScopedFakeNSWindowFullscreen
fake_fullscreen;
On 2015/07/30 06:50:18, tapted wrote:
> nit: comment here like
> 
> // Fullscreen on Mac requires an interactive UI test, but we can fake them
here.

Done.

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1297:
EXPECT_GT(toplevel->GetRestoredBounds().width(), 0);
On 2015/07/30 06:50:18, tapted wrote:
> nit: EXPECT_FALSE(toplevel->GetRestoredBounds().IsEmpty());

Done.

https://codereview.chromium.org/1268643002/diff/80001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:1300: gfx::Rect bounds(100, 100, 200, 200);
On 2015/07/30 06:50:18, tapted wrote:
> nit: const?

Done.

[jackhou1, 2015-07-31 00:58:39]

jackhou@chromium.org changed reviewers:
+ sky@chromium.org

[jackhou1, 2015-07-31 00:58:39]

sky, please review for OWNERS

[sky, 2015-08-03 15:47:26]

LGTM

[jackhou1, 2015-08-03 23:11:10]

The CQ bit was checked by jackhou@chromium.org

[jackhou1, 2015-08-03 23:11:10]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/1268643002/#ps100001
(title: "Address comments.")

[commit-bot: I haz the power, 2015-08-03 23:11:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1268643002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1268643002/100001

[commit-bot: I haz the power, 2015-08-03 23:38:54]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-08-03 23:39:38]

Patchset 6 (id:??) landed as
https://crrev.com/1456431501bdaf8c9362e25343b7393126a722af
Cr-Commit-Position: refs/heads/master@{#341643}