Try to avoid the RemoteLogins:findRecipes sync IPC

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Ehsan, Assigned: perry)

Tracking

(Blocks 1 bug)

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [qf:p1])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
There are three call sites in LoginManagerContent.jsm, one looks pretty async, one is sending an async message shortly after so it may be possible to convert to an asynchronous message, the getFieldContext() one is trickier.

The mean time for this probe is 2.81ms which is fairly high.  It's sent by 86% of sessions.
(Reporter)

Updated

2 years ago
Whiteboard: [qf]
Whiteboard: [qf] → [qf:p1]
Assignee: nobody → jiangperry
Status: NEW → ASSIGNED
One of the reasons this is a sync message is because the return value provided by the listener is then sent back and set as the return value of the sendSyncMessage function. e.g.:

> // --- in the parent process ---
> 
> addMessageListener("foo", function() {
>   return 3;
> });

> // --- in the child process ---
> 
> let bar = messageManager.sendSyncMessage("foo");
> // bar is 3

With an async message this is not possible.. So you need to do this manually with some msg handshake.

Looking it up on searchfox, it looks like there are three locations where we use this sync message: http://searchfox.org/mozilla-central/search?q=RemoteLogins%3AfindRecipes&path=

Matt suggested that we can convert them one by one (in separate bugs), while leaving the others untouched (still using a sync message). That way we can start by the easier or most important first, and not care too much about the others.

Here's what Matt said:

The one on _onFormSubmit happens for every form submission (unless the password manager is disabled)
The one on getFieldContext happens on every page load _if there is a saved login for that domain_
(The other one we didn't talk about)

I think we can start by the _onFormSubmit one which looks simpler, and then work on the other two.
(Assignee)

Updated

2 years ago
Depends on: 1389595
(Assignee)

Updated

2 years ago
Depends on: 1389596
(Assignee)

Updated

2 years ago
Depends on: 1390347
Perry did all the hard work in the three dependencies, thanks to him for that! I'll leave this assigned to him as a result as it's kinda a meta bug.

After those three bugs, we will have removed all uses of the sync RemoteLogins:findRecipes message except for edge cases where we need a synchronous answer (e.g. _onFormSubmit) but the cache hasn't been populated asynchronously. We can watch the sync message JS telemetry to ensure that the occurrence goes down to single-digit % of sessions.

Comment 4

2 years ago
mozreview-review
Comment on attachment 8903817 [details]
Bug 1371525 - Warn when sending a synchronous RemoteLogins:findRecipes message.

https://reviewboard.mozilla.org/r/175574/#review180678
Attachment #8903817 - Flags: review?(felipc) → review+

Comment 5

2 years ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 7708c1e3f5bf -d 512562477a1e: rebasing 417500:7708c1e3f5bf "Bug 1371525 - Warn when sending a synchronous RemoteLogins:findRecipes message. r=Felipe" (tip)
merging toolkit/components/passwordmgr/LoginRecipes.jsm
warning: conflicts while merging toolkit/components/passwordmgr/LoginRecipes.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 6

2 years ago
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/2d4783c5bf18
Warn when sending a synchronous RemoteLogins:findRecipes message. r=Felipe
https://hg.mozilla.org/mozilla-central/rev/2d4783c5bf18
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.