Open Bug 1357818 Opened 7 years ago Updated 2 years ago

Tidy up nsOSHelperAppService::GetMIMEInfoFromOS to do fewer registry lookups

Categories

(Firefox :: File Handling, enhancement, P3)

53 Branch
enhancement

Tracking

()

People

(Reporter: Gijs, Unassigned)

References

Details

(Whiteboard: [fxperf:p5])

There are 3 ways, in theory, that this method gets called:

- with an extension and no mimetype
- with no extension and a mimetype
- with both an extension and a mimetype

We currently do something like this:

1) if we get a mimetype that's not application/octet-stream, look up an extension for that mimetype. If we can't find one, look for one based on the Netscape 4x registry input (bug 1357788).
2) if this resulted in an extension (irrespective of whether we were passed one), use that extension to try to get a mime info object
3) if we have a mime info object, but we were also passed an extension, do another registry lookup in typeFromExtEquals to check that the registry lookup of a mime type for that extension matches our mime type. *If* that succeeds, check that the mime type doesn't already list our extension (if the extension we were passed is simply equal to the extension the mimetype looks up, this will be the case and we will have done all this work for no reason), and if not, add it.
4) if we don't have a mime info object *or* it doesn't have a default handler app (?), create a new, separate mime info object from the extension we were passed (not related to the mime type)
5) if that didn't work but we had a mime object earlier (that isn't the default), return the earlier object
6) if that did work and we didn't have a mime object earlier, return the new object (without caring if it's the default)
7) if neither mime object creation step worked, we create a new one using the passed-in-mime-type (even if that's empty-string) and add the extension iff that's non-empty-string
8) if both mime objects were created, declare an unused default app variable (???) and copy the description of the default for the extension-based mimetype object to the mimetype-based mimetype object (still with me?) and return the latter.


This is overly complex and there's a number of redundant registry lookups / code. We should tidy it up. It might make sense to first carefully check the callers and what they actually need - maybe we should just change some consumers and that might allow simplifying the functionality in this method beyond what's already mentioned above.
Blocks: 1354570
Priority: -- → P3
See Also: → 1363425
I wonder how often we hit this code when browsing, and if it's something we can optimize for perf. Ni me to look at this.
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(gijskruitbosch+bugs)
Whiteboard: [fxperf]
Unless I'm mistaken, the user-perceivable perf wins here seem pretty speculative, so I'm setting this to fxperf:p3.
Whiteboard: [fxperf] → [fxperf:p3]
I just checked by adding some logging and opening a few random "heavy" webpages (yahoo.com, nytimes.com) and I don't think this is exposed almost at all, at this point. So going to downgrade the fxperf tag given that, even if the code is pretty inefficient as written.
Flags: needinfo?(gijskruitbosch+bugs)
Whiteboard: [fxperf:p3] → [fxperf:p5]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.