1346171 - Fix stupid copy-paste error when deleting session files

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Jan Henning [:JanH]]

Affecting the synchronous code path on shutdown:
https://dxr.mozilla.org/mozilla-central/rev/34585620e529614c79ecc007705646de748e592d/mobile/android/components/SessionStore.js#150

Comment 1

[Jan Henning [:JanH]]

Attachment: Bug 1346171 - Part 1 - Check correct file before attempting do delete it.

Review commit: https://reviewboard.mozilla.org/r/119446/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119446/

Comment 2

[Jan Henning [:JanH]]

Attachment: Bug 1346171 - Part 2 - Test synchronous session file deletion on clearing history.

Review commit: https://reviewboard.mozilla.org/r/119448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119448/

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8846404 [details]
Bug 1346171 - Part 1 - Check correct file before attempting do delete it.

https://reviewboard.mozilla.org/r/119446/#review121468

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8846405 [details]
Bug 1346171 - Part 2 - Test synchronous session file deletion on clearing history.

https://reviewboard.mozilla.org/r/119448/#review121470

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/303b1628ce44
Part 1 - Check correct file before attempting do delete it. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/0be73c1886dd
Part 2 - Test synchronous session file deletion on clearing history. r=sebastian

Comment 6

[Jan Henning [:JanH]]

Comment on attachment 8846404 [details]
Bug 1346171 - Part 1 - Check correct file before attempting do delete it.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1266594
[User impact if declined]: When clearing history on shutdown, some sessions store files might not be cleaned up.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No, but verified locally.
[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?]: Just changed check whether file exists to use the correct file.
[String changes made/needed]: none

Comment 7

[Jan Henning [:JanH]]

Comment on attachment 8846405 [details]
Bug 1346171 - Part 2 - Test synchronous session file deletion on clearing history.

See Part 1 for the rest.
[List of other uplifts needed for the feature/fix]: Test modified here was introduced in bug 1343603 part 2, but that has already been uplifted to Aurora and Beta.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/303b1628ce44
https://hg.mozilla.org/mozilla-central/rev/0be73c1886dd

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8846404 [details]
Bug 1346171 - Part 1 - Check correct file before attempting do delete it.

Fix for regression from 52, let's uplift to beta.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8846405 [details]
Bug 1346171 - Part 2 - Test synchronous session file deletion on clearing history.

Thanks for the test fixes.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2cf57400546e
https://hg.mozilla.org/releases/mozilla-aurora/rev/b364c6b8169d

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/ffb57e932e83
https://hg.mozilla.org/releases/mozilla-beta/rev/a70aec4e81ce