Add WidgetTest::GetAllWidgets() to find dialogs created by TestBrowserDialog.

Add WidgetTest::GetAllWidgets() to find dialogs created by TestBrowserDialog.

TestBrowserDialog currently fails to detect some browser dialogs on
Windows 10.

This is because TestBrowserDialog uses the set difference from calls to
Widget::GetAllChildWidgets() to detect when a dialog is added to a
browser window. On Aura, this is populated from Aura child windows (in
the same WindowTreeHost). However, a parented Widget using a
DesktopNativeWidgetAura (e.g. for a top-level Widget on Windows Aero),
rather than a NativeWidgetAura, will not be reported in its parent
window's "GetAllChildWidgets()" (nor in Widget::GetAllOwnedWidgets()).
Ownership of these top-level Widgets is managed in mus-, win-, or
x11-specific code.

To fix, add WidgetTest::GetAllWidgets() which just returns all Widget*s
that can be found in the current process.

In addition, this approach is able to detect Widgets created with no
parent at all, such as the HungRenderer dialog.

BUG=683808, 654151
Review-Url: https://codereview.chromium.org/2660813002
Cr-Commit-Position: refs/heads/master@{#447216}
Committed: https://chromium.googlesource.com/chromium/src/+/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4

[tapted, 2017-01-30 00:50:29]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 00:50:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 00:56:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 00:56:16]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tapted, 2017-01-30 02:04:24]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 02:04:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 02:10:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 02:10:29]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[tapted, 2017-01-30 02:43:20]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 02:43:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-01-30 03:12:56]

Description was changed from

==========
browser test


cl format


Add Goo

BUG=683808, 654151
==========

to

==========
Add WidgetTest::GetAllWidgets() to find dialogs created by TestBrowserDialog.

TestBrowserDialog currently fails to detect some browser dialogs on
Windows 10.

This is because TestBrowserDialog uses the set difference from calls to
Widget::GetAllChildWidgets() to detect when a dialog is added to a
browser window. On Aura, this is populated from Aura child windows (in
the same WindowTreeHost). However, a parented Widget using a
DesktopNativeWidgetAura (e.g. for a top-level Widget on Windows Aero),
rather than a NativeWidgetAura, will not be reported in its parent
window's "GetAllChildWidgets()" (nor in Widget::GetAllOwnedWidgets()).
Ownership of these top-level Widgets is managed in mus-, win-, or
x11-specific code.

To fix, add WidgetTest::GetAllWidgets() which just returns all Widget*s
that can be found in the current process.

BUG=683808, 654151
==========

[tapted, 2017-01-30 03:12:56]

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

[commit-bot: I haz the power, 2017-01-30 03:39:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 03:39:16]

Dry run: This issue passed the CQ dry run.

[tapted, 2017-01-30 03:49:29]

Hi sky, please take a look - this is what I came up with as an alternative to
crrev/2645253002

[tapted, 2017-01-30 10:38:05]

Description was changed from

==========
Add WidgetTest::GetAllWidgets() to find dialogs created by TestBrowserDialog.

TestBrowserDialog currently fails to detect some browser dialogs on
Windows 10.

This is because TestBrowserDialog uses the set difference from calls to
Widget::GetAllChildWidgets() to detect when a dialog is added to a
browser window. On Aura, this is populated from Aura child windows (in
the same WindowTreeHost). However, a parented Widget using a
DesktopNativeWidgetAura (e.g. for a top-level Widget on Windows Aero),
rather than a NativeWidgetAura, will not be reported in its parent
window's "GetAllChildWidgets()" (nor in Widget::GetAllOwnedWidgets()).
Ownership of these top-level Widgets is managed in mus-, win-, or
x11-specific code.

To fix, add WidgetTest::GetAllWidgets() which just returns all Widget*s
that can be found in the current process.

BUG=683808, 654151
==========

to

==========
Add WidgetTest::GetAllWidgets() to find dialogs created by TestBrowserDialog.

TestBrowserDialog currently fails to detect some browser dialogs on
Windows 10.

This is because TestBrowserDialog uses the set difference from calls to
Widget::GetAllChildWidgets() to detect when a dialog is added to a
browser window. On Aura, this is populated from Aura child windows (in
the same WindowTreeHost). However, a parented Widget using a
DesktopNativeWidgetAura (e.g. for a top-level Widget on Windows Aero),
rather than a NativeWidgetAura, will not be reported in its parent
window's "GetAllChildWidgets()" (nor in Widget::GetAllOwnedWidgets()).
Ownership of these top-level Widgets is managed in mus-, win-, or
x11-specific code.

To fix, add WidgetTest::GetAllWidgets() which just returns all Widget*s
that can be found in the current process.

In addition, this approach is able to detect Widgets created with no
parent at all, such as the HungRenderer dialog.

BUG=683808, 654151
==========

[sky, 2017-01-30 17:07:04]

I definitely like this patch better than the last.

https://codereview.chromium.org/2660813002/diff/40001/chrome/browser/ui/test/...
File chrome/browser/ui/test/test_browser_dialog.cc (right):

https://codereview.chromium.org/2660813002/diff/40001/chrome/browser/ui/test/...
chrome/browser/ui/test/test_browser_dialog.cc:20: #if defined(USE_ASH)
USE_ASH is going to become OS_CHROMEOS, so you may as well start using it now.

https://codereview.chromium.org/2660813002/diff/40001/chrome/browser/ui/test/...
chrome/browser/ui/test/test_browser_dialog.cc:99:
views::Widget::GetAllChildWidgets(ash::Shell::GetPrimaryRootWindow(),
I assume only the primary display is ok for tests, right?

https://codereview.chromium.org/2660813002/diff/40001/ui/views/test/widget_te...
File ui/views/test/widget_test_unittest.cc (right):

https://codereview.chromium.org/2660813002/diff/40001/ui/views/test/widget_te...
ui/views/test/widget_test_unittest.cc:50: ExpectAdd(&expected, platform,
"platform");
It seems like the order of WidgetTest::GetAllWidgets() matches creation order.
Are you sure that is true? At least on windows I'm not sure EnumThreadWindow()
enumerates in any particular order.

[tapted, 2017-01-30 23:26:04]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 23:26:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-01-30 23:36:31]

https://codereview.chromium.org/2660813002/diff/40001/chrome/browser/ui/test/...
File chrome/browser/ui/test/test_browser_dialog.cc (right):

https://codereview.chromium.org/2660813002/diff/40001/chrome/browser/ui/test/...
chrome/browser/ui/test/test_browser_dialog.cc:20: #if defined(USE_ASH)
On 2017/01/30 17:07:04, sky wrote:
> USE_ASH is going to become OS_CHROMEOS, so you may as well start using it now.

Done.

https://codereview.chromium.org/2660813002/diff/40001/chrome/browser/ui/test/...
chrome/browser/ui/test/test_browser_dialog.cc:99:
views::Widget::GetAllChildWidgets(ash::Shell::GetPrimaryRootWindow(),
On 2017/01/30 17:07:04, sky wrote:
> I assume only the primary display is ok for tests, right?

Yes, I think it should be correct for these tests. The dialog should always pop
on the same display as InProcessBrowserTest's browser() instance.

https://codereview.chromium.org/2660813002/diff/40001/ui/views/test/widget_te...
File ui/views/test/widget_test_unittest.cc (right):

https://codereview.chromium.org/2660813002/diff/40001/ui/views/test/widget_te...
ui/views/test/widget_test_unittest.cc:50: ExpectAdd(&expected, platform,
"platform");
On 2017/01/30 17:07:04, sky wrote:
> It seems like the order of WidgetTest::GetAllWidgets() matches creation order.
> Are you sure that is true? At least on windows I'm not sure EnumThreadWindow()
> enumerates in any particular order.

Ah, it's coincidence here that |expected| is updated mostly LIFO order --
Widget::Widgets is a std::set, so the collections should be sorted by pointer
value. Added/updated comments.

[commit-bot: I haz the power, 2017-01-31 01:07:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 01:07:39]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sky, 2017-01-31 05:04:55]

LGTM

[tapted, 2017-01-31 10:57:35]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2017-01-31 10:57:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-31 11:01:41]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485860255442900,
"parent_rev": "a8852ef7dedb30c2b6b4ceb0f778c616e243bb63", "commit_rev":
"3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4"}

[commit-bot: I haz the power, 2017-01-31 11:02:27]

Description was changed from

==========
Add WidgetTest::GetAllWidgets() to find dialogs created by TestBrowserDialog.

TestBrowserDialog currently fails to detect some browser dialogs on
Windows 10.

This is because TestBrowserDialog uses the set difference from calls to
Widget::GetAllChildWidgets() to detect when a dialog is added to a
browser window. On Aura, this is populated from Aura child windows (in
the same WindowTreeHost). However, a parented Widget using a
DesktopNativeWidgetAura (e.g. for a top-level Widget on Windows Aero),
rather than a NativeWidgetAura, will not be reported in its parent
window's "GetAllChildWidgets()" (nor in Widget::GetAllOwnedWidgets()).
Ownership of these top-level Widgets is managed in mus-, win-, or
x11-specific code.

To fix, add WidgetTest::GetAllWidgets() which just returns all Widget*s
that can be found in the current process.

In addition, this approach is able to detect Widgets created with no
parent at all, such as the HungRenderer dialog.

BUG=683808, 654151
==========

to

==========
Add WidgetTest::GetAllWidgets() to find dialogs created by TestBrowserDialog.

TestBrowserDialog currently fails to detect some browser dialogs on
Windows 10.

This is because TestBrowserDialog uses the set difference from calls to
Widget::GetAllChildWidgets() to detect when a dialog is added to a
browser window. On Aura, this is populated from Aura child windows (in
the same WindowTreeHost). However, a parented Widget using a
DesktopNativeWidgetAura (e.g. for a top-level Widget on Windows Aero),
rather than a NativeWidgetAura, will not be reported in its parent
window's "GetAllChildWidgets()" (nor in Widget::GetAllOwnedWidgets()).
Ownership of these top-level Widgets is managed in mus-, win-, or
x11-specific code.

To fix, add WidgetTest::GetAllWidgets() which just returns all Widget*s
that can be found in the current process.

In addition, this approach is able to detect Widgets created with no
parent at all, such as the HungRenderer dialog.

BUG=683808, 654151

Review-Url: https://codereview.chromium.org/2660813002
Cr-Commit-Position: refs/heads/master@{#447216}
Committed:
https://chromium.googlesource.com/chromium/src/+/3b39bb8d4942bbf79cbd36f24ae8...
==========

[commit-bot: I haz the power, 2017-01-31 11:02:28]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3b39bb8d4942bbf79cbd36f24ae8...