1898745 - Move section classification heuristics from the child process to the parent process

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, task, P2)

Description

[Dimi Lee [:dimi]]

In order to support reference field information across multiple iframe, we need to run our section classification heuristics in the parent process

Comment 1

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P1. Store DOM content reference id in FieldDetail structure r=#credential-management-reviewers

In order to run the field detection heuristics in the parent process,
the parent requires a unique identifier for DOM elements to facilitate
communication with the child.

Therefore, we utilize the id from ContentDOMReference as the unique identifier.
This identifier is stored in the FieldDetail structure, which the formautofill
module uses to maintain all information related to an identified field.

Comment 2

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P2. Move classify section from the child to the parent r=#credential-management-reviewers

Depends on D211550

Comment 3

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P3. Record form interactio telemetry in the parent process r=#credential-management-reviewers

We have moved most of the logic from the child to the parent. This patch updates
the locations where telemetry is now recorded in the parent. Additionally,
it updates the API of AutofillTelemetry to comply with the new architecture.

Depends on D211551

Comment 4

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P4. Remove active section and active field related code in formautofill r=#credential-management-reviewers

The new architecture no longer has the notion of an "active" section.
When autocomplete is triggered, we pass the "element id" of the triggered element and
use this id to indicate which form we are currently processing.

Because of this change, we no longer need the code that deals with an active section, field, etc.

Depends on D211552

Comment 5

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P5. Update Testcases r=#credential-management-reviewers

This patch makes the following changes:

1. Converts the xpcshell tests test_clearPopulatedForm.js and test_previewFormFields.js to browser tests.
   This change is necessary because the logic is moved from the child to the parent, requiring integration
   tests instead.
2. Replaces the custom popup listener in FormAutofillChild with BrowserTestUtils.waitForPopupEvent
3. Updates add_heuristic_tests to align with the new architecture.
4. Updates unit tests that use the 'active' section because there is no "active" section anymore.

Depends on D211553

Comment 6

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P6. Update formautofill iOS library to accommodate the new architecture r=#credential-management-reviewers

Depends on D211554

Comment 7

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P7. Do not delay identifying autofill fields after an input is focused r=#credential-management-reviewers

This patch removes setTimeout in doIdentifyAutofillFields because it causes the
formautofill module to mark a field as autocompletable AFTER the autocomplete controller
has already started searching for autocomplete entries. The issue with calling
markAsAutoCompletable after searching has begun is that it resets the autocomplete
search state, which results in the autocomplete popup not being displayed.

Upon reviewing why we added the setTimeout in the first place (see Bug 1364818),
I believe it can be removed. Listening for the DOMContentLoaded event should
be sufficient for handling the autofocus case.

Depends on D212239

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b7441408b33
P1. Store DOM content reference id in FieldDetail structure r=NeilDeakin,credential-management-reviewers
https://hg.mozilla.org/integration/autoland/rev/5481131bb31a
P2. Move classify section from the child to the parent r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/576634ec09aa
P3. Record form interactio telemetry in the parent process r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/d0d65cb44bc7
P4. Remove active section and active field related code in formautofill r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/db306113f8e8
P5. Update Testcases r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/aa4188b16097
P6. Update formautofill iOS library to accommodate the new architecture r=credential-management-reviewers,issammani
https://hg.mozilla.org/integration/autoland/rev/8daa01aff9d2
P7. Do not delay identifying autofill fields after an input is focused r=credential-management-reviewers,NeilDeakin,issammani

Comment 10

[Serban Stanca [:SerbanS]]

Backed out for causing AccessibilityTest related junit failures.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.AccessibilityTest#testTextEntry | java.lang.AssertionError: onTextSelectionChanged call count should be within limit

Comment 11

[Dimi Lee [:dimi]]

Attachment: Bug 1898745 - P8. Update GeckoView to adopt the new formautofill architecture.

The patch includes the following changes:

1. Implements a workaround for a test failure (Bug 1905040). This issue occurs because,
   in the current architecture, the child process calls markAsAutoCompletableField regardless of
   whether the field is in a valid section.
2. profile is now stored in fillMessageData. Accordingly, updates where we use profile in
   GeckoViewAutocomplete.sys.mjs.
3. Autofill is triggered from the parent process, so the autofillFields API is called directly
   instead of sending a FormAutofill:FillForm message to the child process.

Comment 12

[Dimi Lee [:dimi]]

Hi owlish, Is it possible to get someone from your team to help review this patch? Really appreciate for your help!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93022697e00d
P1. Store DOM content reference id in FieldDetail structure r=NeilDeakin,credential-management-reviewers
https://hg.mozilla.org/integration/autoland/rev/dd94baef2c71
P2. Move classify section from the child to the parent r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/38e902614b74
P3. Record form interactio telemetry in the parent process r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/9f0148b3b49f
P4. Remove active section and active field related code in formautofill r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/157fbfa2ed56
P5. Update Testcases r=credential-management-reviewers,NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/8c8d4cb4a785
P6. Update formautofill iOS library to accommodate the new architecture r=credential-management-reviewers,issammani
https://hg.mozilla.org/integration/autoland/rev/38803a225a03
P7. Do not delay identifying autofill fields after an input is focused r=credential-management-reviewers,NeilDeakin,issammani
https://hg.mozilla.org/integration/autoland/rev/21237438a530
P8. Update GeckoView to adopt the new formautofill architecture. r=geckoview-reviewers,m_kato

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/93022697e00d
https://hg.mozilla.org/mozilla-central/rev/dd94baef2c71
https://hg.mozilla.org/mozilla-central/rev/38e902614b74
https://hg.mozilla.org/mozilla-central/rev/9f0148b3b49f
https://hg.mozilla.org/mozilla-central/rev/157fbfa2ed56
https://hg.mozilla.org/mozilla-central/rev/8c8d4cb4a785
https://hg.mozilla.org/mozilla-central/rev/38803a225a03
https://hg.mozilla.org/mozilla-central/rev/21237438a530