Closed Bug 521906 Opened 12 years ago Closed 12 years ago

Breakpad should report if extension compatibility overrides are being used


(Toolkit :: Crash Reporting, defect)

Not set



Tracking Status
status1.9.2 --- beta2-fixed


(Reporter: Dolske, Assigned: Dolske)




(1 file)

A suspicion on some crashes is that extensions (esp. with binary components) are crashing as a result of the user having overridden compatibility checking. (eg, by setting extensions.checkCompatibility = false, though I think Nightly Tester Tools might to it a different way).

It would be good to have the crash reporter report if this is happening (at least the pref, maybe more), so that by looking at the report we can know if the user dug their own hole, and how big of a problem this is. [The suggestion was raised of resetting this pref after a major update, so data would help with that decision.]
Needs some EM expertise.
One cheap way would be just to include the value of extensions.checkCompatibility in the crash report. A slightly better way would be to actually check whether any add-ons were installed that should be incompatible in that case, though this would be a perf hit unless you were happy to defer gathering that information until after startup. Neither of these though can detect what Nightly Tester Tools does.

A more all encompassing way would be to do lookups on AMO based on the extension ID and version already in the crash report and see what compatibility version the extension really should have, though that wouldn't work for non-AMO hosted add-ons of course.
Just the pref would be a good start, to allow correlating crashes with users who have the pref enabled.

Though for accurately knowing if a compatibility override actually resulted in loading an outdated extension, we'd probably want the client to report that explicitly. If not "incompatible Extension X was loaded anyway", at least "loaded at least 1 incompatible extension".
Actually thinking about the way we currently load up the list of add-ons it probably wouldn't make for a perf hit to include whether it should be incompatible at that point.
Attached patch Patch v.1Splinter Review
So, maybe just do this? (subject to any changes bug 521905 ends up introducing).

It occurs to me that since we get the list of installed extensions, we can just look to see if Nightly Tester Tools is installed. That can be a cue to look closer at the possibility of forcibly-installed extensions. [It might be possible to use NTT, uninstall it, and still have an incompatible addon used, but I'd imagine that case is rare (if even possible?).] Not perfect, but probably good enough for the purposes of topcrash analysis.

[n.b. didn't test this yet]
Assignee: nobody → dolske
Attachment #409838 - Flags: review?(dtownsend)
Comment on attachment 409838 [details] [diff] [review]
Patch v.1

I'm ok with this, but if you want to mark specific items as being forcibly enabled then I think that can be done inside of _updateExtensionsManifest without really incurring a penalty
Attachment #409838 - Flags: review?(dtownsend) → review+
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Attachment #409838 - Flags: approval1.9.2?
I presume the information of the pref's status is not exposed to the public Crash Report (or its JSON)?
I'm asking because testing a recent Hourly build including this checkin ( + the "Crash Me" Extension + setting "extensions.checkCompatibility;false" results in a Crash Report (-> bp-6a3af55e-2083-4e12-b331-dedc52091107) where i cannot see anything what mentions the pref's status?!
We will need to file a separate bug report to get Socorro to collect and store this information.
Comment on attachment 409838 [details] [diff] [review]
Patch v.1

Attachment #409838 - Flags: approval1.9.2? → approval1.9.2+
Attachment #409838 - Flags: approval1.9.1.6?
Attachment #409838 - Flags: approval1.9.1.6? → approval1.9.1.6-
Comment on attachment 409838 [details] [diff] [review]
Patch v.1

Is this a 1.9.1-applicable patch? With "LightweightThemeManager" in the context I have my doubts.

In any case we want more testing/baking of this so it's not one we want to approve with hours to go until the code-freeze. Next time.
(And where's the Socorro bug to actually start collecting this information?)
Blocks: 528018
(bug 521906 for the Socorro followup)
You need to log in before you can comment on or make changes to this bug.