New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 802163 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression

Blocking:
issue 714618



Sign in to add a comment

Regression:Password remains encrypted in Windows 10 touch-device.

Reported by shruti.j...@etouch.net, Jan 16 2018

Issue description

Chrome Version: 65.0.3322.3 (Official Build)Revision 8758ca55b13d4f2082b2ed9269fce8f37f37c577-refs/branch-heads/3322@{#6}(32/64-bit) 
OS: Win(10 Touch Device).
   
Pre-conditions:Use Touch for tapping on eye icon of password in chrome:settings/password.

Steps to reproduce:
1.Launch Chrome, navigate to gmail.com.
2.Login with valid credentials ,click on save button of save password bubble.
3.Navigate to chrome://settings/passwords.
3.Using touch, tap on eye icon and enter valid password for authentication of system and observe.

Actual Result: Password remains encrypted even after entering valid password of system.
Expected Result: Password should not remain encrypted after entering valid password of system.

This is regression issue broken in M-64, Using the per-revision bisect providing the bisect results,
Good Build: 64.0.3249.0 (Revision:511319)
Bad Build:  64.0.3250.0 (Revision:511680)

You are probably looking for a change made after 511496 (known good), but no later than 511497 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/19dcf4b1006f2b3623816b0326352b27704331e0..7786300c3ba9ffbc00ccad5ee558675a9df0c094

Suspect: https://chromium.googlesource.com/chromium/src/+/7786300c3ba9ffbc00ccad5ee558675a9df0c094

@Adrienne Walker: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note:
1.Issue is reproducible on dev #65.0.3315.3.
2.Issue is reproducible on beta #64.0.3282.85.

Thank You!




 
Expectedpwd.mp4
1.3 MB View Download
Actualpwd.mp4
1.3 MB View Download
Labels: RegressedIn-64 ReleaseBlock-Beta Target-65 FoundIn-64 Target-64
issue existing in current Dev 65.0.3315.3 & beta 64.0.3282.85, marking as RBB please change if required.

Comment 2 by enne@chromium.org, Jan 16 2018

Is this only Windows 10 touch device and not other platforms?

My change modified the way content is rasterized on screen, so it's possible that maybe the password screen is just drawing incorrectly after my change.


Cc: abdulsyed@chromium.org manoranj...@chromium.org
Labels: -ReleaseBlock-Beta ReleaseBlock-Stable
I think we can mark it as 'RBS' instead of 'RBB' for M64.
This is not a beta blocker. Marking it as rb-stable.

Comment 6 by battre@chromium.org, Jan 17 2018

Cc: rogerta@chromium.org
Could you run the bisect once more? I am pretty sure this is not related to the CL mentioned.
Cc: enne@chromium.org
Labels: Needs-Bisect
Owner: shruti.j...@etouch.net
shruti.jadhav@, can you please re-run the bisect?
Labels: -Needs-Bisect
Owner: gab@chromium.org
With respect to comment#7:
Kindly refer the re-Bisect info:
You are probably looking for a change made after 511672 (known good), but no later than 511673 (first known bad).
CHANGE-LOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/4ba0bf602b08a76355ca978bb6c19492729cfc54..b030a4a0884a87640ad65e51b20847c6213b8eab

Suspect:https://chromium.googlesource.com/chromium/src/+/b030a4a0884a87640ad65e51b20847c6213b8eab.

@Gabriel Charette:Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thank you!


Comment 9 by gab@chromium.org, Jan 18 2018

Labels: Needs-Bisect
I tried to repro on Win10 (non-touch) and could not. Is this specific to touch devices?

How confident are we about the above bisect, is the repro 100%? It's not impossible that this is it but I'd like to be confident before digging in because if so that'll be a tough one...

Also, this CL was merged to M63 (https://chromium.googlesource.com/chromium/src/+/8661c6c3eacf31fbbc5c202a844b06bd0381e571) so it should already be a bug on Stable if it is indeed the culprit..?
Cc: pnangunoori@chromium.org
Labels: -Needs-Bisect RegressedIn-63 FoundIn-63
Tested on latest Chrome Stable #63.0.3239.132, Canary # 65.0.3325.0 on Windows 7 and Windows 10 and able to reproduce the issue.

Using the per-revision bisect providing the bisect results,

Good build: 64.0.3249.0 (511318)
Bad build: 64.0.3250.0 (511680)

You are probably looking for a change made after 511672 (known good), but no later than 511673 (first known bad).

CHANGELOG URL:
The script might not always return single CL as suspectas some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/4ba0bf602b08a76355ca978bb6c19492729cfc54..b030a4a0884a87640ad65e51b20847c6213b8eab
https://chromium.googlesource.com/chromium/src/+/b030a4a0884a87640ad65e51b20847c6213b8eab

From the CL above, assigning the issue to the owner concerned.

@gab:  Could you please look into the issue as the above change seems to be the suspect.

Reply to C#3:

1. Issue looks to be observed only in touch devices. Because, as mentioned in Step-3&4 in original comment, we need to touch on the eye icon in chrome://settings/passwords page.
2. Issue is also seen in latest M-63 stable build #63.0.2129.132 and following are the details of good and builds in M-63 builds:
63.0.3239.32 - 3239@{#341}
63.0.3239.34 - 3239@{#359}

However, version #64.0.3241.0 don't have this issue.

The below CL can be found in the above range: https://chromium.googlesource.com/chromium/src/+/8661c6c3eacf31fbbc5c202a844b06bd0381e571

Removing Needs-Bisect label.

Thanks!
M-63 build details:
Good build: 63.0.3239.32 - 3239@{#341}
Bad build: 63.0.3239.34 - 3239@{#359}

Comment 12 by gab@chromium.org, Jan 19 2018

Thanks for the additional bisect. With the M63 bisect as well it's pretty clear that my CL is at fault (in some sense[1]). As mentioned below the real regression range is [2] but my CL enabled the regression.

For further bisecting here's what we can do with access to a local build bisect:
 - Bisect between 3ff403eecdd23a39853a4ebca52023fbba6c5d00 and b030a4a0884a87640ad65e51b20847c6213b8eab
 - But... revert b030a4a0884a87640ad65e51b20847c6213b8eab before building each time (hopefully it applies...).

Is this possible? If not I'll have to get my hands on a Win10 touch device.

[1] That CL re-enabled preventing application tasks from running in nested RunLoops by default. This restriction was accidently lifted in r492263. With the restriction one requires a ScopedNestableTaskAllower in scope of a nested RunLoop which should also process Chrome application tasks, but a developer making a change in this range [2] would not have had to for it to work in that range...

[2] https://chromium.googlesource.com/chromium/src/+log/3ff403eecdd23a39853a4ebca52023fbba6c5d00..b030a4a0884a87640ad65e51b20847c6213b8eab?pretty=fuller&n=10000

Comment 13 by gab@chromium.org, Jan 19 2018

Wait... no, wrong instructions...

For further bisecting here's what we can do with access to a local build bisect:
 - Bisect between 3ff403eecdd23a39853a4ebca52023fbba6c5d00 and 4ba0bf602b08a76355ca978bb6c19492729cfc54
 - But... **cherry-pick** b030a4a0884a87640ad65e51b20847c6213b8eab to your branch before building each time (hopefully it applies...).
Praphulla@, you can use this command to run bisect per c#13.

$ python bisect-builds.py -a win64 -g 492263 -b 511672 -o
Labels: -M-64 -Target-64 M-65
Since this is present in M63 as well, let's target fix for M65. Removing RBS for M64. 
Labels: -RegressedIn-64 FoundIn-65

Comment 17 by kolos@chromium.org, Jan 26 2018

Blocking: 714618

Comment 18 by gab@chromium.org, Jan 26 2018

 manoranjanr@/praphulla@ : I took #14 as an assumption that you'll be performing further bisect per instructions in #13?

That would be great, thank you!

Comment 19 by gab@chromium.org, Jan 26 2018

Labels: Needs-Bisect
Adding Needs-Bisect again, see #13 for extra bisect instructions. Please reply if infeasible.
Cc: gab@chromium.org
Owner: ajha@chromium.org
+ Amit who is looking into it.
gab@, from c#13 **cherry-pick** won't work from TE's end since most of us do not have specific git repository access to checkout and cherry-pick (At the same time we are eager to know if there is a feasible way of doing it).

We are still able to perform c#14 alone if that makes sense. Please let us know!

PS: We've tried the bisect already (per c#14), however it was not consistent. So will give a try one more time and update here.

Thank you!

Comment 22 by gab@chromium.org, Jan 26 2018

No,#14 is not useful, it will lead to my CL. I'd like to pinpoint in between, but requires Cherry-Picking.

Comment 23 Deleted

Comment 24 by gab@chromium.org, Jan 28 2018

Cc: ajha@chromium.org robliao@chromium.org wfh@chromium.org
Owner: rogerta@chromium.org
Ok, I've done the manual bisect and played around with variants of the build.

What started happening as of r511673 is that the browser process gets stuck in an infinite system-induced loop when calling CredUIInternalPromptForWindowsCredentialsW() from password_manager_util_win.cc's AuthenticateUserNew() [1]

r511673 is blamed because it re-introduced an old MessageLoop rule accidently dropped in r492263 that prevents execution of application (chrome) tasks in nested message loops (to prevent unexpected re-entrancy). In this case it looks like CredUIInternalPromptForWindowsCredentialsW() is waiting on a response from chrome (e.g. a system hook/message that ends up posting a chrome task to answer a question -- to which chrome never replies per not running application tasks in this nested loop).

I've confirmed that if I sync to r511673, I can reproduce this infinite loop by tapping on the "show password eye" button with a touch screen and that if I add a MessageLoop::ScopedNestableTaskAllower in the scope of that call, the problem goes away.

While this illustrates the issue, I would however not recommend adding a MessageLoop::ScopedNestableTaskAllower here if we can avoid it as the stack is fairly deep and goes through unwary components which could have subtle bugs if called reentrantly by an unrelated task in the MessageLoop queue that happens to go through a similar stack (this being subtle is the very reason why we block application tasks in nested loops by default).

As mentioned in #12 the true blame range (in which the problem was unfortunately hidden in local testing per missing r511673) is https://chromium.googlesource.com/chromium/src/+log/3ff403eecdd23a39853a4ebca52023fbba6c5d00..4ba0bf602b08a76355ca978bb6c19492729cfc54?pretty=fuller&n=10000

This range contains 5 changes to password_manager_util_win.cc, all by rogerta@, one of which (c225ceb0cad1) introduced the identified problematic call to CredUIPromptForWindowsCredentials().

08e161eecbdb Don't use a pseudo process token when calling GetTokenInformation().
b462fa892c8e Fix password reveal for OS accounts created from Microsoft accounts.
1d9cca6f19ae Remove all hard coded strings length when calling CredUnPackAuthenticationBuffer() and use the function itself to get the required sizes.
839bf36b9860 Fix unable to reveal saved passwords on Windows 10 version 1703 (build 15063, "Creators Update") with new credential prompt.
c225ceb0cad1 Upgrade the credential dialog for password protection in Chrome.

On trunk however I am not able to fully reproduce this bug (i.e. there is no infinite loop waiting for system induced nested task in my repro attempts). I do however see a problem showing the password immediately after taping the eye icon and entering system password (contrary to a mouse click: a touch requires touching the "eye" again to reveal the password (without being prompted for password)). This problem seems to have been present before r511673 as well however so might require an additional bisect (and a different bug?).

An additional bisect between r511673 (64.0.3250.0) and r532204 (66.0.3334.0) to determine when the infinite loop issue went away would be nice to have a better understanding of this issue.


Final note: I'm now having trouble repro'ing the above @ r511673 in my attempts to confirm everything... I suspect the infinite loop might have had to do with a combination of me using a touch screen laptop over RDP. Different touch drivers may drive this in different ways (e.g. opt to register a hook which requires chrome re-entrancy)... But anyways, the root cause sure is related to a nested system loop per r511673 having been bisected as initial blame CL and I'm therefore 99.9% certain that system calls in password_manager_util_win.cc introduced in the aforementioned CLs are causing it.


@rogerta for further assessment based on the above information.


Cheers!
Gab


[1] 
USER32!PeekMessageW+0x1b7
USER32!PeekMessageW+0x14d
combase!CCliModalLoop::MyPeekMessage+0x31 [d:\rs1\onecore\com\combase\dcomrem\callctrl.cxx @ 3085] 
combase!CCliModalLoop::PeekRPCAndDDEMessage+0x31 [d:\rs1\onecore\com\combase\dcomrem\callctrl.cxx @ 2787] 
combase!CCliModalLoop::BlockFn+0x195 [d:\rs1\onecore\com\combase\dcomrem\callctrl.cxx @ 2421] 
combase!ClassicSTAThreadWaitForHandles+0xb4 [d:\rs1\onecore\com\combase\dcomrem\classicsta.cpp @ 53] 
combase!CoWaitForMultipleHandles+0x8e [d:\rs1\onecore\com\combase\dcomrem\sync.cxx @ 123] 
windows_ui_creddialogcontroller!WaitForCompletion<Windows::Foundation::IAsyncOperationCompletedHandler<Windows::Internal::UI::Credentials::Controller::RequestCredentialsData *>,Windows::Foundation::IAsyncOperation<Windows::Internal::UI::Credentials::Controller::RequestCredentialsData *> >+0x93
windows_ui_creddialogcontroller!PromptInternal+0xe2
windows_ui_creddialogcontroller!PromptAndWaitForCredsWithStyle+0x54
windows_ui_creddialogcontroller!CredUXController::Prompt+0x13e
wincredui!CredUIPromptForWindowsCredentialsWorkerInternal+0x4ee
wincredui!CredUIInternalPromptForWindowsCredentialsWorker+0x2d
wincredui!CredUIInternalPromptForWindowsCredentialsW+0xa1
chrome!password_manager_util_win::`anonymous namespace'::AuthenticateUserNew+0x5cf [D:\src\chrome\src\chrome\browser\password_manager\password_manager_util_win.cc @ 484] 
chrome!password_manager_util_win::AuthenticateUser+0x600 [D:\src\chrome\src\chrome\browser\password_manager\password_manager_util_win.cc @ 513] 
chrome!PasswordManagerPresenter::OsReauthCall+0x14 [D:\src\chrome\src\chrome\browser\ui\passwords\password_manager_presenter.cc @ 468] 
chrome!base::RepeatingCallback<bool ()>::Run+0xb [D:\src\chrome\src\base\callback.h @ 92] 
chrome!PasswordAccessAuthenticator::EnsureUserIsAuthenticated+0xcf [D:\src\chrome\src\chrome\browser\ui\passwords\password_access_authenticator.cc @ 35] 
chrome!PasswordManagerPresenter::RequestShowPassword+0x38 [D:\src\chrome\src\chrome\browser\ui\passwords\password_manager_presenter.cc @ 316] 
chrome!extensions::PasswordsPrivateDelegateImpl::RequestShowPasswordInternal+0x15 [D:\src\chrome\src\chrome\browser\extensions\api\passwords_private\passwords_private_delegate_impl.cc @ 126] 
chrome!base::internal::FunctorTraits<void (extensions::PasswordsPrivateDelegateImpl::*)(unsigned int, content::WebContents *) __attribute__((thiscall)),void>::Invoke+0xc [D:\src\chrome\src\base\bind_internal.h @ 194] 
chrome!base::internal::InvokeHelper<0,void>::MakeItSo+0xc [D:\src\chrome\src\base\bind_internal.h @ 277] 
chrome!base::internal::Invoker<base::internal::BindState<void (extensions::PasswordsPrivateDelegateImpl::*)(unsigned int, content::WebContents *) __attribute__((thiscall)),base::internal::UnretainedWrapper<extensions::PasswordsPrivateDelegateImpl>,unsigned int,content::WebContents *>,void ()>::RunImpl+0xf [D:\src\chrome\src\base\bind_internal.h @ 349] 
chrome!base::internal::Invoker<base::internal::BindState<void (extensions::PasswordsPrivateDelegateImpl::*)(unsigned int, content::WebContents *) __attribute__((thiscall)),base::internal::UnretainedWrapper<extensions::PasswordsPrivateDelegateImpl>,unsigned int,content::WebContents *>,void ()>::Run+0x15 [D:\src\chrome\src\base\bind_internal.h @ 334] 
chrome!base::RepeatingCallback<void ()>::Run+0xc [D:\src\chrome\src\base\callback.h @ 92] 
chrome!extensions::PasswordsPrivateDelegateImpl::ExecuteFunction+0x19 [D:\src\chrome\src\chrome\browser\extensions\api\passwords_private\passwords_private_delegate_impl.cc @ 234] 
chrome!extensions::PasswordsPrivateDelegateImpl::RequestShowPassword+0x8d [D:\src\chrome\src\chrome\browser\extensions\api\passwords_private\passwords_private_delegate_impl.cc @ 108] 
chrome!extensions::PasswordsPrivateRequestPlaintextPasswordFunction::Run+0x52 [D:\src\chrome\src\chrome\browser\extensions\api\passwords_private\passwords_private_api.cc @ 103] 
chrome!ExtensionFunction::RunWithValidation+0x4a [D:\src\chrome\src\extensions\browser\extension_function.cc @ 452] 
chrome!extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal+0x2c4 [D:\src\chrome\src\extensions\browser\extension_function_dispatcher.cc @ 464] 
chrome!extensions::ExtensionFunctionDispatcher::Dispatch+0xd9 [D:\src\chrome\src\extensions\browser\extension_function_dispatcher.cc @ 380] 
chrome!extensions::ExtensionWebContentsObserver::OnRequest+0x29 [D:\src\chrome\src\extensions\browser\extension_web_contents_observer.cc @ 244] 
chrome!IPC::DispatchToMethodImpl+0xb
chrome!IPC::DispatchToMethod+0xb
chrome!IPC::MessageT<ExtensionHostMsg_Request_Meta,std::tuple<ExtensionHostMsg_Request_Params>,void>::Dispatch<extensions::ExtensionWebContentsObserver,extensions::ExtensionWebContentsObserver,content::RenderFrameHost,void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost *, const ExtensionHostMsg_Request_Params &) __attribute__((thiscall))>+0x60 [D:\src\chrome\src\ipc\ipc_message_templates.h @ 145] 
chrome!extensions::ExtensionWebContentsObserver::OnMessageReceived+0x59 [D:\src\chrome\src\extensions\browser\extension_web_contents_observer.cc @ 172] 
chrome!extensions::ChromeExtensionWebContentsObserver::OnMessageReceived+0x27 [D:\src\chrome\src\chrome\browser\extensions\chrome_extension_web_contents_observer.cc @ 90] 
content!content::WebContentsImpl::OnMessageReceived+0x9f [D:\src\chrome\src\content\browser\web_contents\web_contents_impl.cc @ 822] 
content!content::RenderFrameHostImpl::OnMessageReceived+0x7f [D:\src\chrome\src\content\browser\frame_host\render_frame_host_impl.cc @ 859] 
content!content::RenderProcessHostImpl::OnMessageReceived+0x102 [D:\src\chrome\src\content\browser\renderer_host\render_process_host_impl.cc @ 2897] 
ipc!IPC::ChannelProxy::Context::OnDispatchMessage+0x1f [D:\src\chrome\src\ipc\ipc_channel_proxy.cc @ 320] 
ipc!base::internal::FunctorTraits<void (IPC::ChannelProxy::Context::*)(const IPC::Message &) __attribute__((thiscall)),void>::Invoke+0xc [D:\src\chrome\src\base\bind_internal.h @ 194] 
ipc!base::internal::InvokeHelper<0,void>::MakeItSo+0x18 [D:\src\chrome\src\base\bind_internal.h @ 277] 
ipc!base::internal::Invoker<base::internal::BindState<void (IPC::ChannelProxy::Context::*)(const IPC::Message &) __attribute__((thiscall)),scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message>,void ()>::RunImpl+0x18 [D:\src\chrome\src\base\bind_internal.h @ 349] 
ipc!base::internal::Invoker<base::internal::BindState<void (IPC::ChannelProxy::Context::*)(const IPC::Message &) __attribute__((thiscall)),scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message>,void ()>::Run+0x21 [D:\src\chrome\src\base\bind_internal.h @ 334] 
base!base::OnceCallback<void ()>::Run+0x46 [D:\src\chrome\src\base\callback.h @ 64] 
base!base::debug::TaskAnnotator::RunTask+0xe5 [D:\src\chrome\src\base\debug\task_annotator.cc @ 55] 
base!base::internal::IncomingTaskQueue::RunTask+0x6a [D:\src\chrome\src\base\message_loop\incoming_task_queue.cc @ 130] 
base!base::MessageLoop::RunTask+0x1a7 [D:\src\chrome\src\base\message_loop\message_loop.cc @ 394] 
base!base::MessageLoop::DeferOrRunPendingTask+0xa4 [D:\src\chrome\src\base\message_loop\message_loop.cc @ 406] 
base!base::MessageLoop::DoWork+0x1f8 [D:\src\chrome\src\base\message_loop\message_loop.cc @ 450] 
base!base::MessagePumpForUI::DoRunLoop+0x7d [D:\src\chrome\src\base\message_loop\message_pump_win.cc @ 174] 
base!base::MessagePumpWin::Run+0x6c [D:\src\chrome\src\base\message_loop\message_pump_win.cc @ 58] 
base!base::MessageLoop::Run+0xa7 [D:\src\chrome\src\base\message_loop\message_loop.cc @ 347] 
base!base::RunLoop::Run+0xce [D:\src\chrome\src\base\run_loop.cc @ 117] 
chrome!ChromeBrowserMainParts::MainMessageLoopRun+0xde [D:\src\chrome\src\chrome\browser\chrome_browser_main.cc @ 1921] 
content!content::BrowserMainLoop::RunMainMessageLoopParts+0x39 [D:\src\chrome\src\content\browser\browser_main_loop.cc @ 1208] 
content!content::BrowserMainRunnerImpl::Run+0x90 [D:\src\chrome\src\content\browser\browser_main_runner.cc @ 140] 
content!content::BrowserMain+0x76 [D:\src\chrome\src\content\browser\browser_main.cc @ 48] 
content!content::RunNamedProcessTypeMain+0x92 [D:\src\chrome\src\content\app\content_main_runner.cc @ 429] 
content!content::ContentMainRunnerImpl::Run+0x11c [D:\src\chrome\src\content\app\content_main_runner.cc @ 707] 
embedder!service_manager::Main+0x2cd [D:\src\chrome\src\services\service_manager\embedder\main.cc @ 456] 
content!content::ContentMain+0x33 [D:\src\chrome\src\content\app\content_main.cc @ 19] 
chrome!ChromeMain+0x14c [D:\src\chrome\src\chrome\app\chrome_main.cc @ 126] 
chrome_exe!MainDllLoader::Launch+0x235 [D:\src\chrome\src\chrome\app\main_dll_loader_win.cc @ 199] 
chrome_exe!wWinMain+0x57e [D:\src\chrome\src\chrome\app\chrome_exe_main_win.cc @ 231] 
chrome_exe!invoke_main+0x1a [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 118] 
chrome_exe!__scrt_common_main_seh+0xf6 [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 283] 
KERNEL32!BaseThreadInitThunk+0x24
ntdll!__RtlUserThreadStart+0x2f
ntdll!_RtlUserThreadStart+0x1b

Comment 25 by gab@chromium.org, Jan 28 2018

Issue 805717 has been merged into this issue.

Comment 26 by gab@chromium.org, Jan 28 2018

And indeed the recently re-enabled hang reports caught this in issue 805717 :) (well re-enabled in  issue 804345  but shortly after re-disabled on UI thread per issue 806174).

Comment 27 by ajha@chromium.org, Jan 29 2018

Labels: -Needs-Bisect
Removing Needs-Bisect as per C#24.

Comment 28 by gab@chromium.org, Jan 29 2018

Labels: Needs-Bisect
Sorry, it was in there but lots of text:

"An additional bisect between r511673 (64.0.3250.0) and r532204 (66.0.3334.0) to determine when the infinite loop issue went away would be nice to have a better understanding of this issue."

(if it's still present on trunk then it's just that I didn't manage to repro after my initial repro)
I am not able to repro on Win10 version 1709 (OS Build 16299.192) with chrome m66 and a touch screen.  I am able to repro with m64 on the same machine.  I am not able to repro with m64 and a mouse on the same machine, so does seem related to touch as mentioned in OP.

I cannot repro an infinite loop with either chrome version, with mouse or touch.  I'll look into nested loops.
Even with m64 and touch, the problem seems to happen only with I try to unlock with the account password.  If I switch to use the PIN to unlock, the problem does not occur.  I'm not sure why this happens yet.
Cc: ekaramad@chromium.org
With respected to command #28:

Just to update, tried bisect using the revisions mentioned in comment 28.

Getting error 32 while running per-revision bisect hence tried chromium bisect (pyhton bisects-builds.py -a win -g 511673 -b 532204) and below is the CL:
https://chromium.googlesource.com/chromium/src/+log/e348427745f1578f43e5c6660ba2b17e778ee60c..baae088436460d3c04fde7e801b435f7a0d3f294

Suspecting: https://chromium.googlesource.com/chromium/src/+/9144d372762c2017dd2cc43eaf733300b83fc873.

Thank You!
Labels: -Needs-Bisect
As per comment #31 removing Needs-Bisect as of now.Please add if its required.
Labels: ET-MUM-Reported
Friendly ping to get an update on this issue as it is marked as stable blocker for M65.
Thanks..!
Status: Started (was: Assigned)
After some more investigation, I am able to repro with 66.0.3342.0 on Win10 version 1709 (OS Build 16299.192).  The problem is that onShowPasswordButtonTap_() is being called twice when the user touches the eye icon, but only on the first touch of the screen.  All other touches only call this function once.

I can repro this behaviour in both the settings page as well as the password edit dialog.

This function is set as the on-tap handler for the eye button:

https://cs.chromium.org/search/?q=onShowPasswordButtonTap_&sq=package:chromium&type=cs

Not sure why chrome is calling the event listener more than once on the first screen touch.  Still investigating.
@rogerta: Are you able to reproduce this in any other platform using the DevTools device mode, to simulate touch events?

Also, is the problem fixed if you change on-tap= to on-click= ?
M65 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge  into the release branch ASAP. Thank you.
Not sure I would call this a release blocker.  The workaround is to just tap again, no need to enter password a second time.

Demetrios: changing to on-click fixes the issue.  Not clear to me why this is the correct fix though.  Apart from windows touch devices, what other touch platforms do we support this page on, besides chromeos?  I do not repro on chromeos touch devices.
I am not aware of an official list of touch devices we support on. Theoretically someone could have a Linux device with touch screen.

My suggestion on how to proceed is the following:
 - Review existing Polymer bugs and see if you find anything related, see [1].
 - Try to find a minimal repro example outside the context of Settings. You can use the template at [2] as a starting point.
 - Hopefully previous step succeeds, so you can file a bug on the Polymer side (assuming the bug is not on the Settings side).

Having said that, changing on-tap to on-click might be sufficient anyway without causing any other issues, given the Note from the WC3 spec at [3], also pasting below:

"Though the click event type has its origins in pointer	devices (e.g., a mouse), subsequent implementation enhancements have extended it beyond that association, and it can be considered a device-independent event type for element activation."

Theoretically we could transition the entire WebUI codebase to use on-click instead of the synthetic on-tap, without any problems, but that has not been attempted (or even proposed) so far.

[1] https://github.com/Polymer/polymer/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+%27tap%27
[2] https://jsfiddle.net/7h07cono/7/
[3] https://www.w3.org/TR/uievents/#event-type-click
FYI, filed issue 812035 to migrate Polymer WebUI to use 'click' instead. Feel free to send a CL that only fixes the chrome://settings/passwords in the meantime.
Labels: -M-65 -Target-65 M-66 Target-66
As this bug exists on M63/64 and per comment #39, not considering this as blocker for M65 stable. Please target fix for M66. Pls do let me know if there is any concern here. Thank you.
Status: Fixed (was: Started)
Fixed as part of issue 812035.

Sign in to add a comment