Open Bug 1699988 Opened 4 years ago Updated 2 years ago

GeckoView config file on Android is not deleted when application is force-stopped

Categories

(Testing :: geckodriver, defect, P2)

Default
defect

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Here an excerpt from such a geckodriver session:

1616342759599	geckodriver::android	DEBUG	Pushing GeckoView configuration file to /data/local/tmp/org.mozilla.fenix-geckoview-config.yaml
1616342759600	mozdevice	DEBUG	execute_host_command: >> "host:transport:ZY226X76LJ"
1616342759600	mozdevice	DEBUG	execute_host_command: << []
1616342759600	mozdevice	DEBUG	execute_host_command: >> "shell:ls /data/local/tmp"
1616342759647	mozdevice	DEBUG	execute_host_command: << "mozdevice\n"
1616342759657	mozdevice	DEBUG	execute_host_command: >> "host:transport:ZY226X76LJ"
1616342759657	mozdevice	DEBUG	execute_host_command: << []
1616342759657	mozdevice	DEBUG	execute_host_command: >> "shell:am set-debug-app --persistent org.mozilla.fenix"
1616342759732	mozdevice	DEBUG	execute_host_command: << ""
1616342759732	geckodriver::android	DEBUG	Launching org.mozilla.fenix/.App
1616342759736	mozdevice	DEBUG	execute_host_command: >> "host:transport:ZY226X76LJ"
1616342759736	mozdevice	DEBUG	execute_host_command: << []
1616342759736	mozdevice	DEBUG	execute_host_command: >> "shell:am start -W -n org.mozilla.fenix/.App --es args \"--marionette --profile /mnt/sdcard/test_root/org.mozilla.fenix-geckodriver-profile\""
1616342763711	mozdevice	DEBUG	execute_host_command: << "Starting: Intent { cmp=org.mozilla.fenix/.App (has extras) }\nStatus: ok\nLaunchState: UNKNOWN (-1)\nActivity: org.mozilla.fenix/.App\nWaitTime: 3889\nComplete\n"
1616342763712	geckodriver::marionette	DEBUG	Waiting 60s to connect to browser on 127.0.0.1:2828
1616342823792	geckodriver::android	DEBUG	Force stopping the Android package: org.mozilla.fenix
1616342823792	mozdevice	DEBUG	Force stopping Android package: org.mozilla.fenix
1616342823793	mozdevice	DEBUG	execute_host_command: >> "host:transport:ZY226X76LJ"
1616342823793	mozdevice	DEBUG	execute_host_command: << []
1616342823793	mozdevice	DEBUG	execute_host_command: >> "shell:am force-stop org.mozilla.fenix"
1616342823913	mozdevice	DEBUG	execute_host_command: << ""
1616342823913	webdriver::server	DEBUG	<- 500 Internal Server Error {"value":{"error":"timeout","message":"Socket timeout reading Marionette handshake data: EOF reading marionette message","stacktrace":""}}

Checking for the config file reveals:

➜ adb shell ls /data/local/tmp/
org.mozilla.fenix-geckoview-config.yaml

The config file needs to be always deleted to not cause any side-effects. See also bug 1636985.

Blocks: 1700557
No longer blocks: 1686110

After some investigation yesterday I really have the feeling that there is always a high chance of leaving around customizations for the application under test on the device. This is pretty bad when it comes to the YAML config because manually starting the same application manually will not use the users profile, but the testing one. Similar it's bad to leave the app in debuggable state.

A situation as described above could come up due to bug 1430064. If a WebDriver:NewSession fails, and the consumer of geckodriver does not explicitly call WebDriver:DeleteSession, all the customizations will be left around on the device.

So what could we do:

  1. Keep pushing the profile once to the device at the very beginning
  2. Prepare the app (debuggable, YAML config, permissions) when launching the app
  3. Remove customizations from step 2 when force stopping the app
  4. Keep the usual clean-up when deleting the session.

Agi, would it be possible to immediately remove the YAML config file when the app has been started? This would drastically reduce the chance of leaving the file around due to possible side-effects as mentioned above. The same question would also apply to the am set-debug-app --persistent setting.

Flags: needinfo?(agi)
Assignee: nobody → hskupin
Status: NEW → ASSIGNED

Note that because geckodriver 0.29.1 will be a bug fix release, I would minimize the changes for that particular issue. Whatever Agi will suggest by next week we can implement it for 0.30.0. If that could be done, it looks like a larger refactoring.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #2)

Agi, would it be possible to immediately remove the YAML config file when the app has been started? This would drastically reduce the chance of leaving the file around due to possible side-effects as mentioned above. The same question would also apply to the am set-debug-app --persistent setting.

You mean the app would read it and then delete it immediately? I think It would be pretty confusing for users that create their own YAML config files. geckodriver is not the only use case.

Flags: needinfo?(agi)

(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #5)

You mean the app would read it and then delete it immediately? I think It would be pretty confusing for users that create their own YAML config files. geckodriver is not the only use case.

No, not the app by itself but geckodriver, so that it's really only for the geckodriver specific clean-up logic. But if the app would still need it at a later time we would indeed have to wait until it closes.

Flags: needinfo?(agi)

Thanks Agi! This is good to hear, and we will get it implemented that way.

Given that the other changes for 0.29.1 makes this issue way lesser to occur we decided to post-pone it and ship it with the next feature release.

Blocks: 1686110
No longer blocks: 1700557
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Blocks: 1723202
No longer blocks: 1686110
Blocks: 1750691
No longer blocks: 1723202

(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #5)

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #2)

Agi, would it be possible to immediately remove the YAML config file when the app has been started? This would drastically reduce the chance of leaving the file around due to possible side-effects as mentioned above. The same question would also apply to the am set-debug-app --persistent setting.

You mean the app would read it and then delete it immediately? I think It would be pretty confusing for users that create their own YAML config files. geckodriver is not the only use case.

We could add top-level YAML, or a Gecko pref, controlling whether to delete the YAML file. Something like:

one-shot: true

or delete-this-file-after-use: true.

Blocks: 1794560
No longer blocks: 1750691
No longer blocks: 1794560
Blocks: 1814050
Blocks: 1824713
No longer blocks: 1814050
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: