So it turns out that bug 1266594 helped by slightly delaying the UI shutdown, but in fact it didn't establish a clear dependency between sanitizing having finished and the start of shutdown, which is why some people might still experience this bug. That is because BrowserApp expects to receive a promise for each sanitization item and then waits for all of them to resolve before proceeding with shutdown (https://dxr.mozilla.org/mozilla-central/rev/e91de6fb2b3dce9c932428265b0fdb630ea470d7/mobile/android/chrome/content/browser.js#1500). That was my assumption when I wrote bug 1266594, however upon closer inspection, it turns out that Sanitizer.jsm wraps each sanitization handler within a promise as expected (which is why I assumed bug 1266594 would work as expected), but clearItem fails to actually return that promise (https://dxr.mozilla.org/mozilla-central/rev/e91de6fb2b3dce9c932428265b0fdb630ea470d7/mobile/android/modules/Sanitizer.jsm#41).

Comment on attachment 8843029 [details] Bug 1343995 - Wait for sanitizing to really finish before shutting down. https://reviewboard.mozilla.org/r/116774/#review118776

Pushed by mozilla@buttercookie.de: https://hg.mozilla.org/integration/autoland/rev/dc8b32d3d5c6 Wait for sanitizing to really finish before shutting down. r=jchen

Changing the component to Settings and Preferences, if anyone feels like this is not the right component please feel free to change it back.

Once Aurora updates to 54, could you confirm that this is working as expected?

First off - sorry im a noob. I'd like to test aurora 54, will this be updated via googleplay or can i get a pre-version in here (like in the old bugreport)?

You can find Aurora at https://play.google.com/store/apps/details?id=org.mozilla.fennec_aurora however it will be Friday or later before it is updated to v54.0a2. You can check the version by visiting about:support.

(In reply to Jan Henning [:JanH] from comment #6) > Once Aurora updates to 54, could you confirm that this is working as > expected? Working fine - it takes its time to close the app but it doesnt matter to me.

(In reply to black.snowman from comment #9) > [...] it takes its time to close the app but it doesnt matter to me. Filed bug 1345460 for that, which should give us an idea on how big of a problem this turns out to be.

Comment on attachment 8843029 [details] Bug 1343995 - Wait for sanitizing to really finish before shutting down. Approval Request Comment [Feature/Bug causing the regression]: History clearing on exit, bug 1001309 [User impact if declined]: Private data clearing on quitting could randomly fail. [Is this code covered by automated tests?]: Partially. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: From a code perspective no. From a performance perspective a bit, since quitting the app might appear to take a bit longer than it used to, although it should probably still be preferable to randomly failing to clear data. [Why is the change risky/not risky?]: All the infrastructure was already in place, except we failed to actually return the promises we wanted to wait on from the Sanitizer. The biggest unknown is how much time sanitization is actually taking across our user base. [String changes made/needed]: none

oh i forgot to thank you for fixing - can i help somehow to check / log the app shutdown?

Holding off on beta uplift till next week so we can follow up with data from bug 1345460.

liuche, can you help out here (and in bug 1345460)?

Does this also need a privacy review? And, without the telemetry feedback to measure performance, do you still want to uplift this?

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #15) > Does this also need a privacy review? I'd tend to say no, but since I'm not really familiar with that, is there any reason why you'd think this might need one, respectively what would having one achieve here? > And, without the telemetry feedback to measure performance, do you still > want to uplift this? I've tested a few times on a somewhat slower phone in my family (S3 Mini) and on a simple shutdown with clearing history the delay (1 - 2 s) didn't seem unbearably long, although of course the phone wasn't particularly busy at the time, so the worst case could still be considerably longer. I'd still think that reliably clearing user data is the more important thing in the short term at least.

Ah, I meant for the new telemetry added in the other bug.

Yeah, it would be nice to get this fix out. Although .. > I've tested a few times on a somewhat slower phone in my family (S3 Mini) > and on a simple shutdown with clearing history the delay (1 - 2 s) didn't > seem unbearably long, although of course the phone wasn't particularly busy > at the time, so the worst case could still be considerably longer. I'd still > think that reliably clearing user data is the more important thing in the > short term at least. .. how does this look like. Do you press back and then 1-2 seconds nothing happens? Maybe that's something for UX to look at in a follow-up bug?

I thought about that, although of course anything involving new strings unfortunately cannot be uplifted together with this.

Comment on attachment 8843029 [details] Bug 1343995 - Wait for sanitizing to really finish before shutting down. Fix for clearing private data, let's get it on beta. It should show up for the beta 7 fennec build next week.

Release Note Request (optional, but appreciated) [Why is this notable]: Long-standing problem with clearing user data on shutdown fixed. [Affects Firefox for Android]: Only. [Suggested wording]: FIXED: "Clear private data on exit" could fail to clear data. [Links (documentation, blog post, etc)]: none

Verified on Beta 53.0b7 with two devices: -Asus ZenPad 8.0 Z380KL (Android 6.0.1) -Huawei Honor 5X (Android 5.1.1) The browsing history, logins, etc are all cleared, but I've found downloads remaining after quitting on 7/10 tries. Logged a new issue for this: Bug 1351308

I'm not sure calling attention to this in release notes is what we want here -unless it was a broadly known problem for a long time (which I don't think it was).

Still does not work in 54.0 Google Search history is not deleted.

That means cookies

It should work when using the "Quit" menu.

Yes, it should, but it doesn't. Steps to reproduce found in bug 1269159

What was fixed here is only that our shutdown process didn't use to wait for the promises returned by each sanitisation handler to actually resolve. There could still be some bugs in the individual sanitisation handlers, though, which would result in them resolving their promise even though some sort of async deletion/file writing/... operation was in fact still going on.