Closed Bug 1429108 Opened 7 years ago Closed 2 years ago

Switch FormHistory consumers to the Promise interface instead of the callback one

Categories

(Toolkit :: Form Manager, enhancement, P3)

Product:
Toolkit
Component:
Form Manager
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
108 Branch
Tracking Flags:
Tracking Status
firefox108 --- fixed

People

(Reporter: mconley, Assigned: mak)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(5 files)

Bug 1429108 - Switch FormHistory::updateFormHistoryWrite to promises. r=sgalich,mconley
(deleted), text/x-phabricator-request
Details
Bug 1429108 - Switch FormHistory::search to promises. r=sgalich,mconley
(deleted), text/x-phabricator-request
Details
Bug 1429108 - Switch FormHistory::count to promises. r=sgalich,mconley
(deleted), text/x-phabricator-request
Details
Bug 1429108 - Switch FormHistory::update to promises. r=sgalich,mconley
(deleted), text/x-phabricator-request
Details
Bug 1429108 - Switch FormHistory::getAutocompleteResults to promises. r=sgalich,mconley
(deleted), text/x-phabricator-request
Details
In bug 888784, FormHistory.jsm was modified so that the FormHistory API could be queried asynchronously either via the old system (callbacks), or via Promises. What with async functions and await, it seems safe to assume that Promises are the current future for asynchronous JS, and we should probably migrate over all users of the old callback interface to use the new Promise-based one. After migrating all consumers of the FormHistory API, we can then remove the old callback mechanism.
Priority: -- → P3
Severity: normal → S3

I noticed most crashes on shutdown show formhistory: blocking, I don't think it's really that slow, I suspect the problem is sometimes the callbacks are not invoked, so it makes sense to convert to promises and see if it helps.

Assignee: nobody → mak
Blocks: 1236455, 1700584
Status: NEW → ASSIGNED
Depends on: 1798156

I just realized that we already started this switch in Bug 1798156

Depends on D160921

Depends on D160922

Depends on D160923

Duplicate of this bug: 1798210

There is an intermittent failure in TV with toolkit/components/search/tests/xpcshell/test_searchSuggest.js, though it's in the remote part of the SearchSuggestionController module, and at a maximum my changes would affect the local part... I tried to track it down but I suspect it's xhr failing silently or some other network thing, because a few lines above the remote result is there, then we remove a local result and the remote result is not fetched.
It's the same as Bug 1597923, Bug 1599381, Bug 1702550, that were closed as incomplete, but the problem is still there.
The best path forward would probably be to add logging to SearchSuggestionController and flip it on by default in tests, there's nothing I can log atm, so I'll file a bug for that.

Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/6f14fb2c1e17 Switch FormHistory::updateFormHistoryWrite to promises. r=sgalich,mconley https://hg.mozilla.org/integration/autoland/rev/3720c10532ae Switch FormHistory::search to promises. r=sgalich,mconley https://hg.mozilla.org/integration/autoland/rev/33ad97b705a0 Switch FormHistory::count to promises. r=sgalich,mconley https://hg.mozilla.org/integration/autoland/rev/677bfadbe5c3 Switch FormHistory::update to promises. r=sgalich,mconley https://hg.mozilla.org/integration/autoland/rev/9d0633c39587 Switch FormHistory::getAutocompleteResults to promises. r=sgalich,mconley
Blocks: 1798861

(In reply to Marco Bonardo [:mak] from comment #10)

The best path forward would probably be to add logging to SearchSuggestionController and flip it on by default in tests, there's nothing I can log atm, so I'll file a bug for that.

I filed bug 1798869

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: