Closed Bug 1007850 Opened 6 years ago Closed 6 years ago
Don't do any work in Seer::Reset if disabled
Running quickly through try now: https://tbpl.mozilla.org/?tree=Try&rev=a8c7b9957f67
Attachment #8419618 - Flags: review?(mcmanus)
Attachment #8419618 - Flags: review?(mcmanus) → review+
Tracking this so we can evaluate on channels and see if this addresses some of the shutdown issues that are still presenting from bug 1005958
Welp, that failed on try, thanks to the lack of a try block in ForgetAboutSite.jsm around the call to reset the seer (which now throws). New patch incoming.
So this just makes the seer return NS_OK if it's disabled when Reset is called. The other option is to wrap the call in ForgetAboutSite.jsm in a try block to silence the error. I (slightly) prefer this version, however. New try run to make sure I really, for reals, fixed it this time: https://tbpl.mozilla.org/?tree=Try&rev=cce26723eaa8
Attachment #8419732 - Flags: review?(mcmanus) → review+
Comment on attachment 8419732 [details] [diff] [review] patch v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): 881804 User impact if declined: Seer does some work on shutdown if disabled and sanitize is enabled, causing an empty sqlite file to magically appear on disk if it wasn't there before. Testing completed (on m-c, etc.): on m-c, locally Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none
Does this fix need manual QA verification ?
Not sure that I'm the best arbiter of whether or not a particular fix needs manual QA verification, but it seems like a waste of time for QA to spend any time verifying this. That said, if you want to, here are steps to reproduce/verify: (1) Set network.seer.enabled to false (2) Set privacy.sanitize.sanitizeOnShutdown to true (3) Quit firefox (4) Remove <profile>/netpredictions.sqlite (5) Start firefox (maybe browse around a bit, if you're having a boring day) (6) Quit firefox (7) See if <profile>/netpredictions.sqlite exists In builds from before this fix landed, the file *will* exist in step 7. In builds from after this fix landed, the file should *not* exist in step 7.
Verified on Windows 7 64bit using Beta 30.0b4, latest Nightly 32.0a1 and latest Aurora 31.0a2. Based on comment #10 I will mark this issue as verified. If you consider otherwise or want me to verify on other OSs please let me know.
You need to log in before you can comment on or make changes to this bug.