Currently we don't do so, but we should.

This seems to work.

Comment on attachment 761095 [details] [diff] [review] Proposed patch. v1 Review of attachment 761095 [details] [diff] [review]: ----------------------------------------------------------------- r+ if your claim about error states is correct. ::: mobile/android/base/health/BrowserHealthRecorder.java @@ +142,5 @@ > + final boolean wasOOM = false; > + final boolean wasStopped = true; > + final long wallStartTime = System.currentTimeMillis(); > + final long realStartTime = android.os.SystemClock.elapsedRealtime(); > + Log.d(LOG_TAG, "Recording runtime session transition: " + -1 on log spew. @@ +341,5 @@ > this.state = State.INITIALIZATION_FAILED; > return; > } > + > + final int env = ensureEnvironment(); nit: shadowing this.env is lame. newEnv? next? @@ +342,5 @@ > return; > } > + > + final int env = ensureEnvironment(); > + nit: whitespace. @@ +597,5 @@ > + /** > + * Invoked in the background whenever the environment transitions between > + * two valid values. > + * > + * This will never be called without a valid storage handle, so it does not Why can't this race with storage shutdown? Runnable gets pushed to background thread, lots of things happen, storage shutdown starts (and finishes), then runnable executes? @@ +603,5 @@ > + */ > + protected void onEnvironmentTransition(int prev, int env) { > + final SharedPreferences prefs = GeckoApp.getAppSharedPreferences(); > + final SharedPreferences.Editor editor = prefs.edit(); > + nit: less newlines and less this.

(In reply to Nick Alexander :nalexander from comment #2) > Why can't this race with storage shutdown? Runnable gets pushed to > background thread, lots of things happen, storage shutdown starts (and > finishes), then runnable executes? I think my logic is: BrowserHealthRecorder.close is only ever called on the background thread. If our runnable starts and it's INITIALIZED, it'll continue to be valid until the next runnable. I'm checking on this, though, and might add the extra safety regardless.

https://hg.mozilla.org/integration/fx-team/rev/5fd1ed6740f8 Note for verification: with this patch you'll see stuff like 08-01 14:03:54.022 D/GeckoHealthRec(30441): Add-on changed: {0fa2149e-bb2c-4ac2-a8d3-479599819475} 08-01 14:03:54.082 D/GeckoHealthRec(30441): Recording session end: E 08-01 14:03:54.122 D/GeckoSessInfo(30441): Recording session done: 1375390877883 08-01 14:03:54.122 D/GeckoSessInfo(30441): Recording runtime session transition: 1375391034126, 406917130 08-01 14:03:54.122 D/GeckoSessInfo(30441): Recording start of session: 1375391034126 when you install an add-on -- that is, we'll end the current session and start another. Flagging for tracking 24, because it's a little interrelated with Bug 900694, and it will improve FHR's accuracy with little risk, but needs to bake for a brief period on Nightly before I request uplift.

Comment on attachment 761095 [details] [diff] [review] Proposed patch. v1 Drat, forgot to flag this. Dependency for Part 2 of Bug 900694. Should be safe, and well-tested by hand.

Comment on attachment 761095 [details] [diff] [review] Proposed patch. v1 [Triage Comment] a=bajaj for beta as Fx24 is beta now