1197118 - Clearing all current history in permanent Private Browsing Mode leads to uncaught exception

Status: RESOLVED WORKSFORME

(Firefox :: Session Restore, defect)

Description

[Georg Koppen]

If one starts in permanent Private Browsing Mode and tries to clear all current state an uncaught exception is thrown:

A coding exception was thrown and uncaught in a Task.

Full message: TypeError: this.Paths is null
Full stack: Agent.wipe@resource:///modules/sessionstore/SessionWorker.js:296:7
worker.dispatch@resource:///modules/sessionstore/SessionWorker.js:21:24
anonymous/AbstractWorker.prototype.handleMessage@resource://gre/modules/workers/PromiseWorker.js:122:16
@resource:///modules/sessionstore/SessionWorker.js:30:41
EventListener.handleEvent*@resource:///modules/sessionstore/SessionWorker.js:30:1

https://bugzilla.mozilla.org/show_bug.cgi?id=1151356#c1 mentions this as well and we have it in our bugtracker, too: https://trac.torproject.org/projects/tor/ticket/16862

Comment 1

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1197118-In-Permanent-PBM-don-t-attempt-to-wipe-s.patch

Normally, when permanent private browsing mode is not activated, then 
SessionFile.read() is called:
https://dxr.mozilla.org/mozilla-central/rev/f2518b8a7b97b5bb477e94bc9131584007aac887/browser/components/sessionstore/nsSessionStartup.js#92
which calls SessionFileInternal.read():
https://dxr.mozilla.org/mozilla-central/rev/f2518b8a7b97b5bb477e94bc9131584007aac887/browser/components/sessionstore/SessionFile.jsm#65
which posts "init" to SessionWorker.jsm with a value for `this.Paths`:
https://dxr.mozilla.org/mozilla-central/rev/f2518b8a7b97b5bb477e94bc9131584007aac887/browser/components/sessionstore/SessionFile.jsm#250

On the other hand, if Permanent PBM is active, then `this.Paths` remains null, and the wipe function at
https://dxr.mozilla.org/mozilla-central/rev/f2518b8a7b97b5bb477e94bc9131584007aac887/browser/components/sessionstore/SessionWorker.js#289
throws the "Full message: TypeError: this.Paths is null" exception Georg reported.

Given
https://dxr.mozilla.org/mozilla-central/rev/f2518b8a7b97b5bb477e94bc9131584007aac887/browser/components/sessionstore/nsSessionStartup.js#92
and
https://dxr.mozilla.org/mozilla-central/rev/f2518b8a7b97b5bb477e94bc9131584007aac887/browser/components/sessionstore/SessionSaver.jsm#175
it's clear that Permanent PBM is not supposed to be touching the session store. So this patch prevents SessionFile.wipe() from being called when Permanent PBM is active.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8655257 [details] [diff] [review]
0001-Bug-1197118-In-Permanent-PBM-don-t-attempt-to-wipe-s.patch

Review of attachment 8655257 [details] [diff] [review]:
-----------------------------------------------------------------

The code in question here gets only called (and only fails) when the browser is instructed to sanitize on shutdown but failed to do so for whatever reason - maybe the browser crashed shortly after setting that option. So in theory there still might be a sessionstore file on disk that we'd want to remove. If we simply ignore sanitize request in permanent private browsing then this would leave such a file around. Not an issue on a new profile but this might hit existing users that turn off navigation history.

A solution here might be to move the SessionWorker.post("init", ...) call to its own function and ensure this is called once before we do anything with the worker. But that call also expects the origin... not a big issue because that might as well just be "empty".

OTOH we could also always pass "this.Paths" to wipe(), a function that shouldn't be called very often anyway.

Comment 3

[Georg Koppen]

This is no issue longer with ESR45.