Closed Bug 1474143 Opened 6 years ago Closed 6 years ago

Don't load FormAutofillContent and FormValidationChild until they're needed

Categories

(Toolkit :: Form Autofill, enhancement, P3)

Product:
Toolkit
Component:
Form Autofill
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla67
Project Flags:
Fission Milestone M1
Tracking Flags:
Tracking Status
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed

People

(Reporter: kmag, Assigned: MattN)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [overhead:89k] )

Attachments

(10 files)

Bug 1474143 - Use sharedData instead of initialProcessData for autofillSavedFieldNames. r=felipe
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Use sharedData instead of initialProcessData for autofillEnabled. r=felipe
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Remove unused 'passwordmanager' category and gPasswordManagerInitialized. r=ehsan
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Remove unused 'firstformsubmit' category and gFirstFormSubmitted. r=ehsan
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Replace form submission observer notifications with events. r=ehsan
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Switch earlyformsubmit pwmgr observers to DOMFormBeforeSubmit listeners. r=felipe
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Rename FormSubmitChild.jsm to FormValidationChild.jsm. r=felipe
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Switch earlyformsubmit satchel observer to DOMFormBeforeSubmit listener. r=felipe
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Don't load FormAutofillContent.jsm until it's needed. r=felipe
(deleted), text/x-phabricator-request
Details
Bug 1474143 - Switch earlyformsubmit webNavigation observers to DOMFormBeforeSubmit listeners. r=felipe
(deleted), text/x-phabricator-request
Details
FormAutofillContent shouldn't be needed in a content process until we get an "earlyformsubmit" or its frame script gets a "focusin" event. In many content processes, such as ad iframes, this will never happen, and it need never be loaded it all. The JSM already delegates a lot of its event handling to its frame script. It should delegate the rest as well. If ProfileAutocomplete needs to be registered before this point (which seems unlikely), it should be registered in a chrome.manifest so it can be loaded lazily when necessary.
Priority: -- → P3
This is a pretty big Fission MemShrink [1] win for a low level of effort. Matt do you know who could look into this? Kris should be able to provide guidance on where to start for whoever ends up working on it. [1] https://groups.google.com/d/msg/mozilla.dev.platform/QbDgI4oM3oY/rTqap48BCgAJ
Flags: needinfo?(MattN+bmo)
Realistically it's unlikely to happen in the 63 cycle since we're trying to get PaymentRequest enabled by default for Nightly and I have a security bug to fix.
(In reply to Matthew N. [:MattN] (away until Sept. 9) from comment #2) > Realistically it's unlikely to happen in the 63 cycle since we're trying to > get PaymentRequest enabled by default for Nightly and I have a security bug > to fix. Marking as wontfix for 63
status-firefox63: affected → wontfix
status-firefox64: --- → affected

I started looking at this tonight… I have some test failures to work through

Depends on: 1519576
Depends on: 1520075

I have a large patch series that mostly works other than bug 1520075 and maybe one other issue.

Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(MattN+bmo)
Depends on: 1520383
Extend ActorChild for satchel's formSubmitListener in order to listen to the event. Depends on D16653
Switch to using DOMFormBeforeSubmit event instead of earlyformsubmit observer notifications. Depends on D16655

I confirmed that after my changes I was able to see

satchelFormListener: Form submit observer notified.

in the ADB logs and got the password manager save doorhanger as I would expect. m-c Form Autofill isn't on Android so there's nothing to test there.

This is still blocked on bug 1520075 and bug 1520383 (and the soft code freeze) for landing.

Blocks: 1519306
Fission Milestone: --- → M1
Whiteboard: [overhead:47k] → [2/14] from Comment 16: blocked on bug 1520075 and bug 1520383 for landing. [overhead:47k]
Fission Milestone: M1 → M2

Try looks good and the two dependencies have landed so I will land this once bug 1304001 is on autoland longer.

Whiteboard: [2/14] from Comment 16: blocked on bug 1520075 and bug 1520383 for landing. [overhead:47k] → [overhead:47k]
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/9fe55dd58cd8 Use sharedData instead of initialProcessData for autofillSavedFieldNames. r=Felipe https://hg.mozilla.org/integration/autoland/rev/3983d7b6d9ad Use sharedData instead of initialProcessData for autofillEnabled. r=Felipe https://hg.mozilla.org/integration/autoland/rev/2fc9b13171d0 Remove unused 'passwordmanager' category and gPasswordManagerInitialized. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/1a83be7a75ca Remove unused 'firstformsubmit' category and gFirstFormSubmitted. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/9fd0d7a7946f Replace form submission observer notifications with events. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/5d35599598bb Switch earlyformsubmit pwmgr observers to DOMFormBeforeSubmit listeners. r=Felipe https://hg.mozilla.org/integration/autoland/rev/5c112b77e489 Rename FormSubmitChild.jsm to FormValidationChild.jsm. r=Felipe https://hg.mozilla.org/integration/autoland/rev/85ec4f1f5f60 Switch earlyformsubmit satchel observer to DOMFormBeforeSubmit listener. r=Felipe https://hg.mozilla.org/integration/autoland/rev/78bfb4dd1f6a Don't load FormAutofillContent.jsm until it's needed. r=Felipe https://hg.mozilla.org/integration/autoland/rev/1b5d04866585 Switch earlyformsubmit webNavigation observers to DOMFormBeforeSubmit listeners. r=Felipe

The patch in bug 1520383 should fix the test failure in unofficial builds (asan or local).

Flags: needinfo?(MattN+bmo)
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/913a5a45b3c5 Use sharedData instead of initialProcessData for autofillSavedFieldNames. r=Felipe https://hg.mozilla.org/integration/autoland/rev/93aed64c54ca Use sharedData instead of initialProcessData for autofillEnabled. r=Felipe https://hg.mozilla.org/integration/autoland/rev/ffc0f96ff0b1 Remove unused 'passwordmanager' category and gPasswordManagerInitialized. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/edb3dd416083 Remove unused 'firstformsubmit' category and gFirstFormSubmitted. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/93b2262689a0 Replace form submission observer notifications with events. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/d13f92b248a2 Switch earlyformsubmit pwmgr observers to DOMFormBeforeSubmit listeners. r=Felipe https://hg.mozilla.org/integration/autoland/rev/bb1ecf178495 Rename FormSubmitChild.jsm to FormValidationChild.jsm. r=Felipe https://hg.mozilla.org/integration/autoland/rev/25f5c0d8531d Switch earlyformsubmit satchel observer to DOMFormBeforeSubmit listener. r=Felipe https://hg.mozilla.org/integration/autoland/rev/1eea6e958cca Don't load FormAutofillContent.jsm until it's needed. r=Felipe https://hg.mozilla.org/integration/autoland/rev/5a471d8ee829 Switch earlyformsubmit webNavigation observers to DOMFormBeforeSubmit listeners. r=Felipe

Because the scope of the bug included more than just Form Autofill, this ended up being a 89k (2.19%) "macOS Base JS After tabs open [+30s, forced GC] opt stylo" improvement.

Comparison: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=autoland&originalRevision=1c9d01a6fbb89c217ba02247f25183f41c6b1952&newProject=autoland&newRevision=5a471d8ee8290e2e791b74a13c2d1bff36777041&originalSignature=1684839&newSignature=1684839&framework=4

Graph: https://treeherder.mozilla.org/perf.html#/graphs?timerange=172800&series=autoland,1684839,1,4&highlightedRevisions=5a471d8ee829&selected=autoland,1684839,437886,732543494,4

Summary: Don't load FormAutofillContent until it's needed → Don't load FormAutofillContent and FormValidationChild until they're needed
Whiteboard: [overhead:47k] → [overhead:89k]

== Change summary for alert #19589 (as of Fri, 22 Feb 2019 22:27:30 GMT) ==

Improvements:

2% Base Content JS windows10-64-pgo-qr opt stylo 4,226,344.00 -> 4,126,941.33
2% Base Content JS osx-10-10 opt stylo 4,161,778.67 -> 4,066,890.67

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=19589

Fission Milestone: M2 → M1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: