Closed Bug 1373635 Opened 6 years ago Closed 6 years ago

start_session inappropriately creates a new profile if not requested on shutdown with clean=True


(Remote Protocol :: Marionette, defect)

52 Branch
Not set


(firefox-esr52 wontfix, firefox54 wontfix, firefox55 fixed, firefox56 fixed)

Tracking Status
firefox-esr52 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed
firefox56 --- fixed


(Reporter: whimboo, Assigned: whimboo)



(Keywords: regression)


(1 file)

Even the instance is not running we call `restart()` on it to get the process running again:

Providing no arguments will assume clean=True, and as such creates a new profile. Also it would try to close the application again, which at this stage is even not running. 

We should simply change this to just `start()`.

This behavior is present since Firefox 41 and was introduced by bug 1166033. Interestingly no single unit test catches it.
Also for a forced quit (without in_app=True argument) we invalidate the profile. I think we really should make this opt-in, and by default keep the current profile as we do for forced restarts. The API here is just broken.
The try build shows failures with the initial safebrowsing test, and I'm not sure if this is related to artifact builds I have choosen on try, and bug 1371923 which is about to get fixed. I pushed another try for those failing test by not requesting artifact builds:
As I thought non-artifact builds do not cause the safebrowsing test to fail. All green. So I think we are good in getting this patch reviewed.
Comment on attachment 8878488 [details]
Bug 1373635 - Application profile has only be reset if explicitly requested.
Attachment #8878488 - Flags: review?(ato) → review+
Pushed by
Application profile has only be reset if explicitly requested. r=ato
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
A bad regression we shipped for a while for the marionette harness package, and which prevents a test from using this method for testing. Given that it will be needed on beta, I request uplift of this test-only patch.
Whiteboard: [checkin-needed-beta]
Flags: in-testsuite+
Whiteboard: [checkin-needed-beta]
Were you thinking you might want this on ESR52 still? It'll need a rebased patch if the answer is yes.
Flags: needinfo?(hskupin)
Nothing actually utilizes it on esr52, so I don't think it's actually necessary to spend the time on a rebased patch.
Flags: needinfo?(hskupin)
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.