Issue metadata
Sign in to add a comment
|
[MacViews] Wire up AccountChooserDialogView |
||||||||||||||||||||
Issue descriptionI think we can just add wiring here: https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/passwords/password_prompt_view_bridge.mm
,
Oct 4 2017
I'll figure out what this is.
,
Oct 4 2017
,
Oct 6 2017
The NextAction date has arrived: 2017-10-06
,
Jan 4 2018
This doesn't look wired. After digging again, I still have no idea how to summon this.
Next step should probably be to try to get
"../browser/ui/views/passwords/password_dialog_view_browsertest.cc",
in chrome/test/BUILD.gn compiling on Mac.
,
Jan 9 2018
The good news is that it builds already.
The bad news is:
3 tests failed:
PasswordDialogViewTest.InvokeUi_AutoSigninFirstRun (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:508)
PasswordDialogViewTest.InvokeUi_PopupAccountChooserWithMultipleCredentialClickSignIn (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:504)
PasswordDialogViewTest.InvokeUi_PopupAccountChooserWithSingleCredentialClickSignIn (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:498)
4 tests crashed:
PasswordDialogViewTest.PopupAccountChooserWithMultipleCredentialsReturnEmpty (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:172)
PasswordDialogViewTest.PopupAccountChooserWithSingleCredentialClickSignIn (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:259)
PasswordDialogViewTest.PopupAccountChooserWithSingleCredentialReturnEmpty (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:239)
PasswordDialogViewTest.PopupAutoSigninPrompt (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:369)
I'll look into why that is.
,
Jan 9 2018
BrowserTestBase received signal: Segmentation fault: 11. Backtrace:
0 libbase.dylib 0x000000010ec3170c base::debug::StackTrace::StackTrace(unsigned long) + 28
1 browser_tests 0x0000000107e43a62 content::(anonymous namespace)::DumpStackTraceSignalHandler(int) + 226
2 libsystem_platform.dylib 0x00007fff50f52f5a _sigtramp + 26
3 browser_tests 0x0000000108c75d92 ConstrainedWindowMac::ShowWebContentsModalDialog() + 146
4 browser_tests 0x0000000107e4378f content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() + 335
5 browser_tests 0x0000000107960a80 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() + 4048
6 browser_tests 0x000000010795f9ae ChromeBrowserMainParts::PreMainMessageLoopRun() + 62
7 libcontent.dylib 0x00000001129399d3 content::BrowserMainLoop::PreMainMessageLoopRun() + 67
8 libcontent.dylib 0x0000000112d1b6f7 content::StartupTaskRunner::RunAllTasksNow() + 39
9 libcontent.dylib 0x0000000112937fc7 content::BrowserMainLoop::CreateStartupTasks() + 583
10 libcontent.dylib 0x000000011293c4b5 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) + 613
11 libcontent.dylib 0x00000001129361a4 content::BrowserMain(content::MainFunctionParams const&) + 180
12 libcontent.dylib 0x00000001131b350f content::ContentMainRunnerImpl::Run() + 383
13 libembedder.dylib 0x0000000116c138f8 service_manager::Main(service_manager::MainParams const&) + 2344
14 libcontent.dylib 0x00000001131b29f4 content::ContentMain(content::ContentMainParams const&) + 68
15 browser_tests 0x0000000107e43456 content::BrowserTestBase::SetUp() + 2438
16 browser_tests 0x00000001078edb48 InProcessBrowserTest::SetUp() + 392
17 browser_tests 0x0000000107455831 testing::Test::Run() + 97
18 browser_tests 0x0000000107456450 testing::TestInfo::Run() + 288
19 browser_tests 0x00000001074569b7 testing::TestCase::Run() + 263
20 browser_tests 0x000000010745dfc7 testing::internal::UnitTestImpl::RunAllTests() + 903
21 browser_tests 0x000000010745dc13 testing::UnitTest::Run() + 163
22 browser_tests 0x0000000107901427 base::TestSuite::Run() + 167
23 browser_tests 0x00000001078e40c5 ChromeTestSuiteRunner::RunTestSuite(int, char**) + 37
24 browser_tests 0x0000000107e8bf93 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) + 387
25 browser_tests 0x00000001078e44d7 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) + 135
26 browser_tests 0x00000001078e403e main + 94
27 libdyld.dylib 0x00007fff50cd1115 start + 1
28 ??? 0x0000000000000006 0x0 + 6
[1/1] PasswordDialogViewTest.PopupAutoSigninPrompt (CRASHED)
1 test crashed:
PasswordDialogViewTest.PopupAutoSigninPrompt (../../chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc:369)
,
Feb 5 2018
I have finally nailed down what this is: it is the family of dialogs that appear when we want to use some creds to auto-signin, but need to prompt the user about it (there are multiple eligible sets, it's the first time, etc). The Cocoa version looks like the attached screenshot - it took quite some hacking to get it to show up via the browser test. I can't figure out yet how to get the Views one to show. Still, it's obvious where the wiring needs to go. Still to do: * Work out manual repro steps (probably need to ask the team about it) * Get the browser tests passing - this dialog seems to use a sheet which I think is messing with the existing browser tests.
,
Feb 6 2018
Okay, I have working repro steps \o/ 1) https://credential-management-sample.appspot.com/ 2) Enter some fake creds, hit register 3) Hit save on the bubble 4) Reload the page - the bubble that pops up is the auto signin prompt 5) Hit sign out 6) Reload again - the bubble that pops up is the account chooser prompt Note that it won't work if you are using a Chromium build and don't allow it to use your local keychain.
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f99c89339afca8d4358f528bbc7117cafb051d5 commit 5f99c89339afca8d4358f528bbc7117cafb051d5 Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Wed Feb 07 13:39:16 2018 macviews: wire account chooser dialogs This change wires the auto signin prompt and the account chooser prompt. Bug: 728129 Change-Id: Ibdf76a20642ec8e8734bf733facb9aec4dc69146 Reviewed-on: https://chromium-review.googlesource.com/905163 Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#534995} [modify] https://crrev.com/5f99c89339afca8d4358f528bbc7117cafb051d5/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/5f99c89339afca8d4358f528bbc7117cafb051d5/chrome/browser/ui/cocoa/passwords/password_prompt_view_bridge.mm [add] https://crrev.com/5f99c89339afca8d4358f528bbc7117cafb051d5/chrome/browser/ui/cocoa/passwords/password_prompt_views_mac.mm [modify] https://crrev.com/5f99c89339afca8d4358f528bbc7117cafb051d5/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc [modify] https://crrev.com/5f99c89339afca8d4358f528bbc7117cafb051d5/chrome/browser/ui/views/passwords/auto_signin_first_run_dialog_view.cc
,
Feb 7 2018
,
Feb 8 2018
Checked the issue on chrome version 66.0.3343.0 using Mac 10.12.6 with the steps mentioned in comment#10. In step 3 after hitting save on the pop-up, we reloaded the page. But we didn't see any pop-up after reloading. Note: We observed a pop-up before reloading(i.e., after hitting save bubble) Attaching the screen cast of the same. @ellyjones: Could you please check the screen cast and let us know if anything missed in the process of verification. Thanks!
,
Feb 8 2018
That's odd - those are the correct repro steps, but they work for me :) I'll ask the involved team about the repro steps and see if there's some way they could have broken. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by tapted@chromium.org
, Jun 8 2017Labels: Proj-MacViews