patricialor@, can you have a look at this? This dialog has been a pain the entire time we've been working on MacViews because it's so difficult to summon.

Sure, having this would be kinda cool (and useful)! Did we want like a views_examples type thing (like, a separate build target entirely?) or a flag (I'm assuming you mean a command line flag?). To be honest, the latter seems a bit weird as a long term solution to me - do either of you have opinions?

A views_examples-style system seems like it would be great but I image you can't just put a dialog onscreen that's not tied to a real browser window?

Oh, I have no idea - it might be possible to mock up dummy versions of all the things each dialog needs, but I don't know how difficult that is, especially with the more complicated ones.. CCing tapted@ who will probably know more?

There's hopefully a browser_test for this dialog.. but I wasn't involved in the UI for it. If there is, we could do something like
 - add a testcase like CardUnmaskPrompt.DISABLED_Invoke
 - Insert base::RunLoop().Run() so that the test never exits.

This would allow us to take screenshots with a reference Browser window to help UI review.

Putting dummy credit card information and/or a way to invoke a security-sensitive dialog into the Official/Canary build will probably be met with objections.

And a new "examples" target that depends on Chrome Profile information could be a lot of work to maintain.

Oh, that's a good idea, will take a look at adding the test case then :) Thanks for the advice!

So, adding a test case to do this is easy - there's already one called CardUnmaskPromptViewBrowserTest.DisplayUI, so it was just a matter of adding the base::RunLoop().Run() that tapted@ mentioned. However, there are some issues with this approach:

- The test has to be run with --ui-test-action-max-timeout=1000000 --test-launcher-timeout=1000000 to get gtest not to kill it after the default timeout limit. There was no 'never time out' option that I could find, so it'll eventually die even if you're not done with it.
 - While you can interact with the card unmask prompt dialog with the mouse, it doesn't accept keyboard focus, so typing into its fields won't work. This might be annoying if you're trying to say, test some error messages with text input fields or whatever.
- Since the test isn't designed to accept user interactions, if you close the dialog while playing with it it'll still wait until the timeout finishes to close the browser. You can still kill it manually with Ctrl-C though.
- The test is always going to fail since it's always going to time out (or be killed manually). This probably doesn't matter since it's DISABLED, but might be confusing to others.

Basically, we need to decide how scrappy of a solution we want. The options we could try, ascending by amount of effort required:

a) This is good enough for us, just add a test similar to this one for each dialog we're interested in. See https://crrev.com/2382963002 for what this would look like.
b) Try to fix some of the issues described above by making a 'special' test type that isn't really a test - not sure how difficult this is.
c) Add new examples target that knows about all the dialogs, as tapted@ described in #c5.

Check out option a) by patching in the above CL in and running it with ./out/Debug/browser_tests --gtest_filter="CardUnmaskPromptViewBrowserTest.DISABLED_Invoke" --gtest_also_run_disabled_tests --ui-test-action-max-timeout=1000000 --test-launcher-timeout=1000000

As much functionality as we can get, without too much effort, and within the bounds of acceptability - that's my vote. I will leave it to you and tapted@ to determine what that means.

I notice that the dialog has month and year popups but they should not be visible? See the screenshot in  Issue 605677 .

I need to pass the flag --secondary-ui-md to turn on Harmony styling - any ideas there?

I also need a similar solution for some other ccard dialogs:

* sync first run - https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview#%2FP%20-%20payments_01.png%3Fz=width

* sync steady state - https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview#%2FP%20-%20payments_02.png%3Fz=width

* local saved steady state - https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview#%2FP%20-%20payments_03.png%3Fz=width

To fix the keyboard focus thing, I think you need something similar to what ShowAndFocusNativeWindow() does in interactive_test_utils_mac.mm. That is,

  ProcessSerialNumber psn = { 0, kCurrentProcess };
  TransformProcessType(&psn,kProcessTransformToForegroundApplication);
  SetFrontProcess(&psn);

Or, the less gross way of doing that,

  [NSApp setActivationPolicy:NSApplicationActivationPolicyRegular];

which is what views_content_client_main_parts_mac.mm does.


The second one is also called upon instantiating a views::ViewsTestHelper[Mac] (e.g. with a ScopedViewsTestHelper), but I think other things will conflict if you try that in a browser_test.

The first option has the advantage that you might be able to just call it in a .cc file behind #ifdefs.

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

commit 24ac066a9656e83b071cca34cbfe15c3f29a1539
Author: patricialor <patricialor@chromium.org>
Date: Fri Oct 21 00:51:31 2016

Views: Add two disabled tests for invoking the CC unmask prompt dialog.

The card unmask prompt is quite difficult to invoke (a signed-in official build
is needed with saved credit cards stored), so add two disabled tests which only
invoke the dialog UI, one with an expired and one with a valid credit card. This
is needed for easier testing and UI review. Note that these tests need to be
disabled because of a call to base::RunLoop().Run() which will make them always
time out if enabled.

Using disabled browser tests instead of another approach means that the mock
models/data used in the pre-existing tests for these dialogs can be re-used,
which will be a smaller maintenance burden. Run them with the browser_tests
target: ./browser_tests --gtest_filter=
"CardUnmaskPromptViewBrowserTest.DISABLED_Invoke*"
--gtest_also_run_disabled_tests --ui-test-action-max-timeout=10000000
--test-launcher-timeout=10000000

Use --secondary-ui-md for new Harmony styling.

BUG= 649908 

Review-Url: https://chromiumcodereview.appspot.com/2382963002
Cr-Commit-Position: refs/heads/master@{#426663}

[modify] https://crrev.com/24ac066a9656e83b071cca34cbfe15c3f29a1539/chrome/browser/ui/autofill/card_unmask_prompt_view_browsertest.cc
[modify] https://crrev.com/24ac066a9656e83b071cca34cbfe15c3f29a1539/ui/base/BUILD.gn
[add] https://crrev.com/24ac066a9656e83b071cca34cbfe15c3f29a1539/ui/base/test/user_interactive_test_case.cc
[add] https://crrev.com/24ac066a9656e83b071cca34cbfe15c3f29a1539/ui/base/test/user_interactive_test_case.h
[add] https://crrev.com/24ac066a9656e83b071cca34cbfe15c3f29a1539/ui/base/test/user_interactive_test_case_mac.mm

Great!

Closing this now because there are specific Proj-HarmonyDialogs-HardToSummon labels on each dialog we want to do something similar to this for.