We originally saved the last payload to disk to support needs of the initial version of about:healthreport and to potentially make Android FHR's life easier. Neither of which are needed any more. We can stop saving the payload to disk.

rnewman is out, so mconnor gets review. An issue with this patch is some users may have an orphaned lastpayload.json file sitting in their profile for eternity. If this gets uplifted to Beta (which I'd recommend since the payload saving just adds unnecessary performance overhead), then the only users affected are the {Beta, Aurora, and Nightly} channels. Honestly, I'm not too concerned about that relatively small population. We can always land cleanup code later if this bothers us.

Do we care about eliminating the ability to see what was uploaded, rather than a live snapshot? Do we care that about:healthreport now needs to (a) keep the document in-memory across refreshes, (b) needs to do a collection each time it's displayed, or (c) needs to somehow query real-time info from the DB? Not sure what the current state of a:hr is, so maybe it's moved beyond "display some JSON"…

(In reply to Richard Newman [:rnewman] from comment #2) > Do we care about eliminating the ability to see what was uploaded, rather > than a live snapshot? We aren't using it for anything now. We only care about the document ID. > Do we care that about:healthreport now needs to (a) keep the document > in-memory across refreshes, (b) needs to do a collection each time it's > displayed, or (c) needs to somehow query real-time info from the DB? Not > sure what the current state of a:hr is, so maybe it's moved beyond "display > some JSON"… a:hr currently collects and obtains a JSON payload with every load. It needs to collect to ensure there is *some* data to provide. Otherwise, we never generate a document if upload is enabled (we just perform a daily collection). We decided it was easier for a:hr to just perform the whole enchilada rather than bake "do I need to do X or Y" logic into a:hr. About the only downside is a:hr may take a few seconds to perform the collection before data is displayed. If this turns out to be a problem, we can always change this behavior and start caching documents again.

(In reply to Gregory Szorc [:gps] from comment #3) > We aren't using it for anything now. We only care about the document ID. Yeah, I kinda meant from a legal or privacy standpoint; I know *we* only care about the ID. I'm hesitant to remove functionality that might have been baked in as a requirement without checking. > a:hr currently collects and obtains a JSON payload with every load. Gotcha.

Rebased on top of bug 854018.

Comment on attachment 743308 [details] [diff] [review] Don't save payload to disk, v2 Review of attachment 743308 [details] [diff] [review]: ----------------------------------------------------------------- Kinda want this in beta, so we don't create files in a user's profile that will stick around, stale, fo'eva. ::: services/healthreport/healthreporter.jsm @@ +101,5 @@ > this._dbOpenHistogram = hasFirstRun ? TELEMETRY_DB_OPEN : TELEMETRY_DB_OPEN_FIRSTRUN; > > TelemetryStopwatch.start(this._initHistogram, this); > > + // As soon as we have could storage, we need to register cleanup or could have

https://hg.mozilla.org/services/services-central/rev/3a7209e3c338 Perhaps we should consider a follow-up to delete the payload file from disk?

Comment on attachment 743308 [details] [diff] [review] Don't save payload to disk, v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): FHR User impact if declined: Slightly worse FHR perf, makes uplift of bug 860094 a little harder. Testing completed (on m-c, etc.): It's baked for a few weeks. No known issues. Risk to taking this patch (and alternatives if risky): This patch is low risk since it mostly removes code unneeded code. String or IDL/UUID changes made by this patch: None

Comment on attachment 743308 [details] [diff] [review] Don't save payload to disk, v2 In support of bug 860094's success in FF22. Risk profile of the overall uplift will not change according to gps.

Is anything QA can verify here?

(In reply to Simona B [QA] from comment #12) > Is anything QA can verify here? Nah.