Just an update on this issue - I had a look through all the bugs in that list you linked, all of them don't seem to have associated browser tests (a couple have unit_tests etc though, which might be more easily reusable). The one exception is  Issue 654128  (extension message bubbles), which does have a bunch of browser tests, but these have actually been failing on MacViews for quite a while, so will have to look into fixing them first.

(Also, just as a FYI, I'm allocating one hour a (work)day to this.)

I'm exploring some stuff in https://codereview.chromium.org/2532793002/ using some crazy C++ tricks.

Example follow-up which would enable this for collected cookies looks like https://codereview.chromium.org/2534743002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6725376ebed11cd889ede0c146fdda81c84ee56

commit d6725376ebed11cd889ede0c146fdda81c84ee56
Author: tapted <tapted@chromium.org>
Date: Tue Nov 29 06:28:42 2016

MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG= 654151 

Review-Url: https://codereview.chromium.org/2535163002
Cr-Commit-Position: refs/heads/master@{#434911}

[modify] https://crrev.com/d6725376ebed11cd889ede0c146fdda81c84ee56/ui/views/widget/native_widget_mac.mm
[modify] https://crrev.com/d6725376ebed11cd889ede0c146fdda81c84ee56/ui/views/widget/native_widget_mac_unittest.mm

+ robliao/kylixrd - pkasting mentioned you might have in interest in ways to make Views easier to use/build/test in general.

There's some crazy stuff I'm exploring in https://codereview.chromium.org/2532793002/ to help with this bug. Feel free to chime in :)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/658e7bd30bc3164953da7796a4198b58b5514655

commit 658e7bd30bc3164953da7796a4198b58b5514655
Author: tapted <tapted@chromium.org>
Date: Wed Dec 21 22:23:36 2016

Add a TestBrowserDialog helper class for testing browser dialogs in a consistent way.

Allows a test harness to opt in to a framework for testing browser
dialogs. Merely override a method, `void ShowDialog(const std::string&
name)`, and add any number of

IN_PROC_BROWSER_TEST_F(FooDialogTest, InvokeDialog_name) {
  RunDialog();
}

to foo_dialog_browsertest.cc.

This will
- Invoke ShowDialog("name") in the test case and perform some basic
checks.
- register the test name with the dialog testing framework.

Running
  browser_tests --gtest_filter=BrowserDialogTest.Invoke

will list all dialogs registered with the testing framework. And with
the additional arguments,
  --dialog=FooDialogTest.InvokeDialog_name --interactive

BrowserDialogTest will invoke the dialog "interactively". This allows
for manual testing and screenshots for UI review.

A test harness can register multiple "styles" of its dialog by providing
multiple tests invoking RunDialog() using different suffixes after
"InvokeDialog_". See in-file documentation for more info.

BUG= 654151 

Review-Url: https://codereview.chromium.org/2532793002
Cr-Commit-Position: refs/heads/master@{#440240}

[modify] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/base/test/test_timeouts.cc
[modify] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/base/test/test_timeouts.h
[modify] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/chrome/browser/ui/autofill/card_unmask_prompt_view_browsertest.cc
[add] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/chrome/browser/ui/test/browser_dialog_browsertest.cc
[add] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/chrome/browser/ui/test/test_browser_dialog.cc
[add] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/chrome/browser/ui/test/test_browser_dialog.h
[modify] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/chrome/test/BUILD.gn
[modify] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/ui/base/test/material_design_controller_test_api.cc
[modify] https://crrev.com/658e7bd30bc3164953da7796a4198b58b5514655/ui/base/test/material_design_controller_test_api.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3888fbf6c8994f26f65ba506341a475b0c2f4e1f

commit 3888fbf6c8994f26f65ba506341a475b0c2f4e1f
Author: tapted <tapted@chromium.org>
Date: Thu Jan 12 02:55:21 2017

MacViews: Harmony for TabDialogs' dialogs (starting with Collected Cookies)

Collected cookies when invoked from the site info bubble (padlock ->
Cookies -> click the "X in use" link) is already hooked up to
--secondary-ui-md. However, it can also be shown via the content
settings bubble when a cookie is blocked.

We also need TabDialogs' Signin confirmation dialog switched to Harmony,
so set up a way to show Harmony dialogs from the TabDialogs interface.

Opt collected cookies into DialogBrowserTest to make it easy to invoke
for testing.

BUG= 654151 ,  677012 , 610428

Review-Url: https://codereview.chromium.org/2534743002
Cr-Commit-Position: refs/heads/master@{#443130}

[delete] https://crrev.com/590f8e6768702a8172c67b3cda7745107b6b1e0d/chrome/browser/collected_cookies_browsertest.cc
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_cocoa.h
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_cocoa.mm
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_views_mac.h
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_views_mac.mm
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/collected_cookies_browsertest.cc
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/test/BUILD.gn

Is this basically fixed, now that we have BrowserDialogTest, and what's needed is to go add every individual dialog to that?

Or should this stay open for more infrastructure, or to track someone trying to explicitly add all dialogs to that?

Some dialogs require an appreciable amount of setup, and mock data. While this provides a consistent framework, some dialogs will still require significant mocking effort to get them to work. An example is the "Register Protocol Handler" bubble.

There's a CL in https://codereview.chromium.org/2645253002/ to fix some corner cases for Windows+Aero where the framework may currently fail to detect the dialog.

And I think there's something to resolve on Windows to resolve wrt the call to base::LaunchProcess(command, options) getting unexpectedly killed when not setting `options.wait = true` -- might be something to do with the console I was using though :/


(I'm also thinking about renaming some of the classes, since I keep mixing up the order of words on the Camel(Case)..).

Then, yep, there's some work to add a bunch of dialogs (eventually all of them!) into the framework. That might be useful to track in a new bug.

> There's a CL in https://codereview.chromium.org/2645253002/ to fix some corner cases for Windows+Aero where the framework may currently fail to detect the dialog.

new CL: https://codereview.chromium.org/2660813002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4

commit 3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4
Author: tapted <tapted@chromium.org>
Date: Tue Jan 31 11:01:38 2017

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}

[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/chrome/browser/ui/test/test_browser_dialog.cc
[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/chrome/browser/ui/test/test_browser_dialog.h
[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/ui/aura/test/aura_test_helper.cc
[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/ui/aura/test/aura_test_helper.h
[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/ui/views/BUILD.gn
[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/ui/views/test/widget_test.h
[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/ui/views/test/widget_test_aura.cc
[modify] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/ui/views/test/widget_test_mac.mm
[add] https://crrev.com/3b39bb8d4942bbf79cbd36f24ae8a861e3f78fa4/ui/views/test/widget_test_unittest.cc

Guh - found one more weirdness on desktop linux. Fix is in https://codereview.chromium.org/2663163004/, but I want to land it with a test.

I'm running into the issue referenced in #14. It's a blocker for https://codereview.chromium.org/2668833003. I manually applied the patch to a linux build and it seems to work. What is the ETA on this landing?

I've got other cleanup work on the on the CL, so it's not super critical at this point.

Attaching rendered output of https://codereview.chromium.org/2684513002/ to help review (note it's missing the css though, so it will be prettier when served up properly).

Deleted: test_browser_dialog.md.html 14.2 KB

test_browser_dialog.md.html 14.2 KB View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f38739a13a880a4641302d42914d44714c0ad7dc

commit f38739a13a880a4641302d42914d44714c0ad7dc
Author: tapted <tapted@chromium.org>
Date: Thu Feb 09 04:34:52 2017

Markdown docs for TestBrowserDialog

TestBrowserDialog landed in r440240 and is ready for wider use. Excerpt:

  `TestBrowserDialog` provides a way to register an `InProcessBrowserTest`
  testing harness with a framework that invokes Chrome browser dialogs in
  a consistent way.

The contents are taken from the public doc on chromium.org at https://goo.gl/EFz4r2

BUG= 654151 

Review-Url: https://codereview.chromium.org/2684513002
Cr-Commit-Position: refs/heads/master@{#449200}

[add] https://crrev.com/f38739a13a880a4641302d42914d44714c0ad7dc/docs/testing/test_browser_dialog.md

Missed #c15 earlier (my inbox exploded during my sheriffing shift..). I think I responded in the CL, but to close the loop here, https://codereview.chromium.org/2663163004/ has landed (r448452). There's a fix for one last "gotcha" for the --interactive mode on Windows in https://codereview.chromium.org/2660553005/ . Then, I'll make an announcement to chromium-dev about this to encourage more folks to use it.

Closing this out as fixed. The solution for this works pretty well. We can make a hotlist for dialogs that still want tests, or just address them ad-hoc.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b80997e5423f29e5ea1853cadec881f7d306616a

commit b80997e5423f29e5ea1853cadec881f7d306616a
Author: Patti <patricialor@chromium.org>
Date: Thu Oct 26 04:58:44 2017

Profile Chooser: Add TestBrowserDialog tests for the profile chooser.

Add a bunch of TestBrowserDialog tests for ProfileChooserView, including signed
in and non-signed in, multiple profiles, guest profiles, supervised user owners,
and the additional manage account link that appears when account consistency is
enabled for signed in profiles.

Change-Id: Ia071502f292b17b86db413f90ba04c350e0760ba
Bug:  654151 
Reviewed-on: https://chromium-review.googlesource.com/734925
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511740}
[modify] https://crrev.com/b80997e5423f29e5ea1853cadec881f7d306616a/chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/248691eb9358643325f0d670ba2b3759c595d0ca

commit 248691eb9358643325f0d670ba2b3759c595d0ca
Author: Patti <patricialor@chromium.org>
Date: Thu Nov 02 02:44:52 2017

Desktop Page Info/Views: Add TestBrowserDialog tests for security descriptions.

Add TestBrowserDialog tests for PageInfoBubbleView to show HTTP / insecure,
HTTPS / secure, and internal Page Info states, plus an array of Safe Browsing
Page Info states for malware & unwanted software distributing sites, and upon
detecting social engineering and password reuse.

Bug:  654151 
Change-Id: I9cced46c33a37ed688993d9fcdf60a4fdb1c8f41
Reviewed-on: https://chromium-review.googlesource.com/743122
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513384}
[modify] https://crrev.com/248691eb9358643325f0d670ba2b3759c595d0ca/chrome/browser/ui/views/page_info/page_info_bubble_view.h
[modify] https://crrev.com/248691eb9358643325f0d670ba2b3759c595d0ca/chrome/browser/ui/views/page_info/page_info_bubble_view_browsertest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1539fe199e1212f6d9cb4d24e017ade66765b5a7

commit 1539fe199e1212f6d9cb4d24e017ade66765b5a7
Author: Patti <patricialor@chromium.org>
Date: Fri Nov 03 03:16:03 2017

Profile Chooser: Add TestBrowserDialog test for supervised users.

This is a follow up to r511740, which adds a number of TestBrowserDialog tests
to ProfileChooserViewBrowserTest, but does not include a test showing the
ProfileChooserView from the perspective of a supervised user. Add a test for
that case.

Bug:  654151 
Change-Id: Ib0858063ba51775a8c2697da23a92a2f575e783f
Reviewed-on: https://chromium-review.googlesource.com/749610
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513663}
[modify] https://crrev.com/1539fe199e1212f6d9cb4d24e017ade66765b5a7/chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f21c4b000691cd2580a7e5841a94ce3cafde9ae

commit 7f21c4b000691cd2580a7e5841a94ce3cafde9ae
Author: Patti <patricialor@chromium.org>
Date: Tue Nov 28 03:37:17 2017

Desktop Page Info/Views: Add TestBrowserDialog tests for permission icons.

Add TestBrowserDialog tests for PageInfoBubbleView to show all permissions
either allowed or blocked, regardless of their factory default setting. In
future, these tests can be used in automatic screenshot taking to track UI
changes, but currently it can be run with
--gtest_filter=BrowserDialogTest.Invoke --interactive
--dialog=PageInfoBubbleViewBrowserTest.InvokeDialog_AllowAllPermissions (or
BlockAllPermissions).

Bug:  654151 
Change-Id: I06aaf4f39f0d660acbd1024198692b35cacef8cb
Reviewed-on: https://chromium-review.googlesource.com/788638
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519548}
[modify] https://crrev.com/7f21c4b000691cd2580a7e5841a94ce3cafde9ae/chrome/browser/ui/page_info/page_info.cc
[modify] https://crrev.com/7f21c4b000691cd2580a7e5841a94ce3cafde9ae/chrome/browser/ui/page_info/page_info.h
[modify] https://crrev.com/7f21c4b000691cd2580a7e5841a94ce3cafde9ae/chrome/browser/ui/views/page_info/page_info_bubble_view_browsertest.cc