New issue
Advanced search Search tips

Issue 617893 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

PasswordGenerationAgent doesn't see some forms, but PasswordManager does

Project Member Reported by kolos@chromium.org, Jun 7 2016

Issue description

PasswordGenerationAgent doesn't see some signup forms on allegro.pl and i.360.cn, but PasswordManager does (i.e. they appear on chrome://password-manager-internals/). 
 

Comment 1 by kolos@chromium.org, Jun 7 2016

PasswordGenerationAgent is initialized on DidFinishDocumentLoad event. DidFinishDocumentLoad event happens too early, i.e. some forms might be still unavailable. For example, if some Javascript stuff is involved. 

See https://cs.chromium.org/chromium/src/third_party/WebKit/public/web/WebFrameClient.h. DidFinishDocumentLoad is triggered before Javascript work.

Comment 2 by kolos@chromium.org, Jun 7 2016

The solution is to replace DidFinishDocumentLoad with DidFinishLoad.

The reverse replacement was made in this CL (https://bugs.chromium.org/p/chromium/issues/detail?id=452741). Javascript might change the form and Chrome might not recognize it. So, it is why the forms were processed before Javascript work, i.e. in DidFinishDocumentLoad.

Comment 3 by kolos@chromium.org, Jun 7 2016

Another solution is to process both events DidFinishDocumentLoad and DidFinishLoad
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 8 2016

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

commit c2fd8ecef7c96e060d601ef71894846164581c9a
Author: kolos <kolos@chromium.org>
Date: Wed Jun 08 09:39:46 2016

[Password Generation] Handle not only DidFinishDocumentLoad in PasswordGenerationAgent, but also DidFinishLoad

DidFinishDocumentLoad event happens too early, i.e. some forms might be still unavailable. For example, if some Javascript stuff is involved. See https://cs.chromium.org/chromium/src/third_party/WebKit/public/web/WebFrameClient.h. DidFinishDocumentLoad is triggered before Javascript work. The solution is to handle DidFinishLoad that is triggered after Javascript work.

This bug ( crbug.com/452741 ) points that the processing DidFinishDocumentLoad is also necessary. So, this CL introduces handling both events (DidFinishDocumentLoad and DidFinishLoad)

There are no tests for the given change, because it is very hard to catch the difference between DidFinishDocumentLoad and DidFinishLoad. Even if it is possible, the test will be probably flaky.

BUG= 617893 

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

[modify] https://crrev.com/c2fd8ecef7c96e060d601ef71894846164581c9a/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/c2fd8ecef7c96e060d601ef71894846164581c9a/components/autofill/content/renderer/password_generation_agent.h

Comment 5 by kolos@chromium.org, Jun 22 2016

Status: Fixed (was: Assigned)

Sign in to add a comment