1237465 - Work around Android 5.1+ permission restrictions in remote reftest and mochitests by changing pushed directory permissions to rwx

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: remotetests-dir-perms.patch

While investigating some issues esawin was having with the mochitest dom media tests on Autophone, I found that one of the reasons my Nexus 6 Android / AOSP 5.1 devices failed was due to permission problems with the files and directories created in the profile and other files/directories pushed to the device.

When the unit test runners push the profile to the device, the files and directories are owned by the shell's process owner which is typically root if adbd root is supported or by shell if not. When Firefox starts and attempts to initialize the remaining portions of the profile, it will create files and directories with its user as the owner and will fail to create or modify files in directories it does not own since they are typically created with rwx------. This prevents the extensions from being installed from staging and prevents access to several other files such as browser.db etc.

The way to work around this is to call chmodDir for directories that are pushed to the device. devicemanager's chmodDir will attempt to set the permissions for files to 777 by default and always set permissions for directories to 755.

This will still result in warnings that Firefox can not set the permissions of the extensions to 755 but it does not cause errors. We can not set the files permissions to 755 rwxr-xr-x since that would still prevent a non-owner from writing to the files.

I did a try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c7a1227d0b6&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3

I'm not sure what is up with the emulator Android 2.3 Mochitest-chrome tests, but it looks good otherwise.

I just kicked off https://treeherder.allizom.org/#/jobs?filter-searchStr=autophone&exclusion_profile=false&repo=try&author=[e-mail]&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=15664302 with my local devices.

The Nexus 9 will suffer from the issue where Tablets are excluded from the android version detection by https://dxr.mozilla.org/mozilla-central/source/dom/media/test/manifest.js#510

The Nexus 6 will still suffer from the issue where the seek.webm video will not be decoded but tests such as https://dxr.mozilla.org/mozilla-central/source/dom/media/test/test_VideoPlaybackQuality.html do not check for decoding errors when attempting to load the video. I'll file another bug on that.

Comment 1

[Geoff Brown [:gbrown]]

Comment on attachment 8704885 [details] [diff] [review]
remotetests-dir-perms.patch

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

r+ if you leave out the remoteChromeTestDir change.

fwiw, I can run mochitest-plain on a 6.0 emulator without your patch.

::: testing/mochitest/runtestsremote.py
@@ +223,5 @@
>          remote = self.remoteChromeTestDir
>          if options.chrome:
>              self.log.info("pushing %s to %s on device..." % (local, remote))
>              self._dm.pushDir(local, remote)
> +            self._dm.chmodDir(remote)

I think you should not add a chmodDir here. This one should only affect mochitest-chrome tests, so you should not need it for autophone. There are a lot of files in this directory. On Android 4.3, it looks like this takes about 10 minutes (from your try run)! And on 2.3, it times out!!

Comment 2

[Geoff Brown [:gbrown]]

Could we avoid the chmod'ing with a well-placed umask?

Comment 3

[Bob Clary [:bc] (inactive)]

(In reply to Geoff Brown [:gbrown] from comment #1)
> Comment on attachment 8704885 [details] [diff] [review]
> remotetests-dir-perms.patch
> 
> Review of attachment 8704885 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ if you leave out the remoteChromeTestDir change.
> 
> fwiw, I can run mochitest-plain on a 6.0 emulator without your patch.
> 

I wonder why? In the created profile, who owns the directory/files that are pushed by devicemanagerADB ? And once Firefox starts, who owns the new directories and files and what are their permissions?

> ::: testing/mochitest/runtestsremote.py
> @@ +223,5 @@
> >          remote = self.remoteChromeTestDir
> >          if options.chrome:
> >              self.log.info("pushing %s to %s on device..." % (local, remote))
> >              self._dm.pushDir(local, remote)
> > +            self._dm.chmodDir(remote)
> 
> I think you should not add a chmodDir here. This one should only affect
> mochitest-chrome tests, so you should not need it for autophone. There are a
> lot of files in this directory. On Android 4.3, it looks like this takes
> about 10 minutes (from your try run)! And on 2.3, it times out!!

Heh. Ok.

(In reply to Geoff Brown [:gbrown] from comment #2)
> Could we avoid the chmod'ing with a well-placed umask?

On my Nexus 6 umask is 0 for the shell user and 0 for the root user.

Comment 4

[Geoff Brown [:gbrown]]

root@generic:/mnt/sdcard/tests # ls -l profile                                 
-rwxrwx--x root     sdcard_rw     4096 2016-01-07 11:56 browser.db
-rwxrwx--x root     sdcard_rw    32768 2016-01-07 11:56 browser.db-shm
-rwxrwx--x root     sdcard_rw   177192 2016-01-07 11:56 browser.db-wal
-rwxrwx--x root     sdcard_rw   229376 2016-01-07 11:56 cert9.db
-rwxrwx--x root     sdcard_rw      222 2016-01-07 11:56 compatibility.ini
-rwxrwx--x root     sdcard_rw   131072 2016-01-07 11:56 cookies.sqlite
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 crashes
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 extensions
-rwxrwx--x root     sdcard_rw      298 2016-01-07 11:56 extensions.ini
-rwxrwx--x root     sdcard_rw   196608 2016-01-07 11:56 formhistory.sqlite
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 gmp
-rwxrwx--x root     sdcard_rw    77824 2016-01-07 11:56 health.db
-rwxrwx--x root     sdcard_rw    45656 2016-01-07 11:56 health.db-journal
-rwxrwx--x root     sdcard_rw   294912 2016-01-07 11:56 key4.db
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 minidumps
-rwxrwx--x root     sdcard_rw    10240 2016-01-07 11:55 permissions.sqlite
-rwxrwx--x root     sdcard_rw      431 2016-01-07 11:55 pkcs11.txt
-rwxrwx--x root     sdcard_rw      551 2016-01-07 11:56 pluginreg.dat
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 plugins
-rwxrwx--x root     sdcard_rw        0 2016-01-07 11:55 prefs.js
-rwxrwx--x root     sdcard_rw     1741 2016-01-07 11:56 profile_info_cache.json
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 safebrowsing
-rwxrwx--x root     sdcard_rw       11 2016-01-07 11:56 server_alive.txt
-rwxrwx--x root     sdcard_rw   327680 2016-01-07 11:56 signons.sqlite
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 startupCache
-rwxrwx--x root     sdcard_rw      306 2016-01-07 11:55 tests.manifest
-rwxrwx--x root     sdcard_rw       27 2016-01-07 11:56 times.json
-rwxrwx--x root     sdcard_rw    17604 2016-01-07 11:55 user.js
-rwxrwx--x root     sdcard_rw      253 2016-01-07 11:55 userChrome.css
drwxrwx--x root     sdcard_rw          2016-01-07 11:56 webapps
-rwxrwx--x root     sdcard_rw    32768 2016-01-07 11:56 webappsstore.sqlite
-rwxrwx--x root     sdcard_rw    32768 2016-01-07 11:56 webappsstore.sqlite-shm
-rwxrwx--x root     sdcard_rw    98408 2016-01-07 11:56 webappsstore.sqlite-wa

My umask is also 0.

Comment 5

[Bob Clary [:bc] (inactive)]

Firefox and devicemanagerADB appear to be creating the files/directories with owner:group root:sdcard_rw. In my case devicemanagerADB with adbd running as root created them as root:root but Firefox created them with its user id which didn't have write access since it wasn't in the same group. I wonder why.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/825f10e3b518

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/825f10e3b518