Closed Bug 1373635 Opened 5 years ago Closed 5 years ago
_session inappropriately creates a new profile if not requested on shutdown with clean=True
59 bytes, text/x-review-board-request
Even the instance is not running we call `restart()` on it to get the process running again: https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette_driver/marionette.py#1186-1190 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: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bb2ae032322c8579023a43f1e89bd7320d55b2e
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.
Attachment #8878488 - Flags: review?(ato)
Comment on attachment 8878488 [details] Bug 1373635 - Application profile has only be reset if explicitly requested. https://reviewboard.mozilla.org/r/149852/#review154684
Attachment #8878488 - Flags: review?(ato) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/274e6b6d1399 Application profile has only be reset if explicitly requested. r=ato
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.
Were you thinking you might want this on ESR52 still? It'll need a rebased patch if the answer is yes.
Nothing actually utilizes it on esr52, so I don't think it's actually necessary to spend the time on a rebased patch.
You need to log in before you can comment on or make changes to this bug.