Closed Bug 938689 Opened 11 years ago Closed 11 years ago

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

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: Irving, Assigned: Irving)

Details

(Whiteboard: [qa-])

Attachments

(1 file)

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.
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/mozilla-central/rev/77ea1269f0e7
Status: ASSIGNED → RESOLVED
Closed: 11 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.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: