Mystery solved: this is actually caused by the old parser!

The old parser ignores disabled fields. The new one does not, by design [1].

However, there is interference as follows:
In renderer, HTML forms are translated into FormData.
Further, the old parser translates that FormData into PasswordForm.
PasswordForm contains FormData and this whole package is sent to the browser.
There the new parser ignores the PasswordForm except for its FormData payload and re-parses it to create a new PasswordForm.
(The idea is that once the old parser is gone, only FormData is transferred to browser process, but we kept the bundle for now to minimize code changes.)

Now, the password field on typing.com is disabled. The old parser refuses to create a PasswordForm, so nothing is sent over to the browser. Therefore also the new parser won't be able to parse the FormData, even though that would otherwise succeed (I verified).


[1] https://docs.google.com/document/d/1KxWnt3-Pykz4ut4P1IHxNhn6Ef64v3VMyGobUWyLnDg/edit#heading=h.azxb6xacxjq2

We have some alternatives to fix that.

My favourite is: decouple sending FormData to browser from parsing PasswordForms in the renderer, i.e., send it over even if the old parser fails.

Another option is: make the old parser ignore the "disabled" status.

The drawback of the latter is that we don't know the impact. The former does not impact the old parser, only the new one, which is desired.

I talked to dvadym@ about the first proposal from #2 and we both think it is worth doing. If we keep only running the new parser on forms which the old parser could parsed, then we might get a surprise once the old parser is gone, because some forms will not have been seen by the new parser before.

Also, the current situation prevents efficient evaluation of cases when the parsing looks broken.

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

commit d446be5b4ceca4d3a42395a5094075026c674af8
Author: Vaclav Brozek <vabr@chromium.org>
Date: Mon Nov 19 10:11:56 2018

Run new PasswordForm parser even if old one fails

The old FormData -> PasswordForm parser runs in renderer process,
before the data about forms is passed on to the browser process.

The new parser runs in browser, after it receives the data from the
renderer.

For legacy reasons, the FormData (parsing input) is enclosed inside
PasswordForm (output) during the inter-process transport.

As a result, if the old parser fails to parse completely, the new one
has no chance to run at all, because there is no valid PasswordForm to
encapsulate its input FormData during the inter-process transport.

The important case when this matters is if there are password fields
present, but none of them is enabled. The old parser gives up, the
new does not care about enabled vs. disabled, so can parse.

This CL allows the old parser to produce a minimal PasswordForm
if only disabled password fields are present, to allow transporting
the FormData to the new parser. In PasswordManager, all these
minimal PasswordForms are managed by at most one PasswordFormManager
and only used as a fallback, so the functionality of Chrome under
the old parser is not affected.

Bug:  904908 
Change-Id: Idaf569d39884375dc5942dea4349aeec41355c76
Reviewed-on: https://chromium-review.googlesource.com/c/1337497
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609225}
[modify] https://crrev.com/d446be5b4ceca4d3a42395a5094075026c674af8/components/autofill/content/renderer/password_form_conversion_utils.cc
[modify] https://crrev.com/d446be5b4ceca4d3a42395a5094075026c674af8/components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc