Add-on state from extensions.installCache is compared using JSON.stringify

RESOLVED FIXED in mozilla28

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Irving, Assigned: Irving)

Tracking

unspecified
mozilla28
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

In XPIProvider.jsm, we try to determine whether add-ons have changed on disk by scanning the state of add-on directories into a JS data structure, and then comparing JSON.stringify() of that structure to a previous copy stored in a preference.

JSON.stringify() makes no guarantee of the order that object fields will render in the string version, so it's possible that this comparison is giving us false positives (detecting changes when the on-disk state didn't actually change), which would slow down startup.

We should do a structured comparison of this data rather than stringify.
Created attachment 832363 [details] [diff] [review]
WIP: Structured comparison of add-on directory state

This does the comparison both ways and flags in Telemetry if the old test gives the wrong answer, so we can see whether it actually mattered.
Attachment #832363 - Flags: feedback?(bmcbride)
Good to do the telemetry here, when I first wrote this I did try a few times to see if there was a difference but it seemed to be stable. I also assume that JSON.stringify is less expensive than JSON.parse so the current method should be fastest.
Based on some quick experiments I did, our JSON.stringify() appears to serialize attributes in the order they were first added to the object, so it's possible that we're getting away with this because we always build the objects in the same order.
Attachment #832363 - Flags: feedback?(bmcbride) → review+
https://hg.mozilla.org/integration/fx-team/rev/77ea1269f0e7
https://hg.mozilla.org/mozilla-central/rev/77ea1269f0e7
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Early results from telemetry are that this hasn't made any difference - I have yet to see any of the telemetry events I added to show where the new code returns a different result than the string compare.

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.