Closed Bug 1343603 Opened 3 years ago Closed 3 years ago

Clearing history should clear cached session store history data as well

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox53 + fixed
firefox54 + fixed
firefox55 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(3 files, 1 obsolete file)

Discovered through bug 1342820 comment 2.
Sounds like 53 is also affected. Tracking since it's a privacy issue
Comment on attachment 8843024 [details]
Bug 1343603 - Part 1 - Immediately clear cached session store history data when clearing history.

https://reviewboard.mozilla.org/r/116720/#review119346
Attachment #8843024 - Flags: review?(ahunt) → review+
Comment on attachment 8843025 [details]
Bug 1343603 - Part 2 - Test that clearing history clears the cached session store data, too.

https://reviewboard.mozilla.org/r/116722/#review119348
Attachment #8843025 - Flags: review?(ahunt) → review+
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/4d31fb49a844
Part 1 - Immediately clear cached session store history data when clearing history. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/80934b637165
Part 2 - Test that clearing history clears the cached session store data, too. r=ahunt
https://hg.mozilla.org/mozilla-central/rev/4d31fb49a844
https://hg.mozilla.org/mozilla-central/rev/80934b637165
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Hi :JanH, 
Since this bug also affects 54, do you think it's worth uplifting to 54 if this patch is not too risky?
Flags: needinfo?(jh+bugzilla)
Comment on attachment 8843024 [details]
Bug 1343603 - Part 1 - Immediately clear cached session store history data when clearing history.

Approval Request Comment
[Feature/Bug causing the regression]: Clearing of private data
[User impact if declined]: Supposedly purged session history of open tabs might resurface if a tab/Firefox is closed after clearing history (with no navigation in between) and later restored.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Tested with Nightly build and found to work.
[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?]: No.
[Why is the change risky/not risky?]: All tabs have minimal session store data with session history available, so it should be safe to iterate over them and clear everything except the current session history entry.
[String changes made/needed]: none
Flags: needinfo?(jh+bugzilla)
Attachment #8843024 - Flags: approval-mozilla-beta?
Attachment #8843024 - Flags: approval-mozilla-aurora?
Comment on attachment 8843025 [details]
Bug 1343603 - Part 2 - Test that clearing history clears the cached session store data, too.

Test for Part 1
Attachment #8843025 - Flags: approval-mozilla-beta?
Attachment #8843025 - Flags: approval-mozilla-aurora?
Comment on attachment 8843024 [details]
Bug 1343603 - Part 1 - Immediately clear cached session store history data when clearing history.

Fix a private issue related to session store history data and include tests. Aurora54+.
Attachment #8843024 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8843025 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8843024 [details]
Bug 1343603 - Part 1 - Immediately clear cached session store history data when clearing history.

The user impact seems pretty severe, Beta53+
Attachment #8843024 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8843025 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Backed out for test_session_clear_history.html failures. Looks like the new test is going to need some tweaking for Beta.
https://hg.mozilla.org/releases/mozilla-beta/rev/a1d99033e1f7

https://treeherder.mozilla.org/logviewer.html#?job_id=82904410&repo=mozilla-beta&lineNumber=2891
Flags: needinfo?(jh+bugzilla)
Ah right, because of bug 1333046 zombification needs to be handles differently. I'll look into it.
Fixed tab zombification to use the MemoryObserver on Beta
Flags: needinfo?(jh+bugzilla)
Fixed a= for Beta in commit message.
Attachment #8846055 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.