Bug 1680407 - [geckodriver] Remove --android-storage argument and always use external storage for profile folder. r=webdriver-reviewers,agi,jgraham

Differential Revision: https://phabricator.services.mozilla.com/D109407
This commit is contained in:
Henrik Skupin 2021-03-23 12:05:19 +00:00
Родитель 26156d0573
Коммит b14a5ba911
7 изменённых файлов: 55 добавлений и 298 удалений

Просмотреть файл

@ -1,52 +1,6 @@
Flags
=====
#### <code>&#x2D;&#x2D;android-storage <var>ANDROID_STORAGE</var></code>
Selects the test data location on the Android device, eg. the Firefox profile.
By default `auto` is used.
<style type="text/css">
table { width: 100%; margin-bottom: 2em; }
table, th, td { border: solid gray 1px; }
td, th { padding: 10px; text-align: left; vertical-align: middle; }
td:nth-child(1), th:nth-child(1) { width: 10em; text-align: center; }
</style>
<table>
<thead>
<tr>
<th>Value
<th>Description
</tr>
</thead>
<tr>
<td>auto
<td>Best suitable location based on whether the device is rooted.<br/>
If the device is rooted <code>internal</code> is used, otherwise <code>app</code>.
<tr>
<td>app
<td><p>Location: <code>/data/data/%androidPackage%/test_root</code></p>
Based on the <code>androidPackage</code> capability that is passed as part of
<code>moz:firefoxOptions</code> when creating a new session. Commands that
change data in the app's directory are executed using run-as. This requires
that the installed app is debuggable.
<tr>
<td>internal
<td><p>Location: <code>/data/local/tmp/test_root</code></p>
The device must be rooted since when the app runs, files that are created
in the profile, which is owned by the app user, cannot be changed by the
shell user. Commands will be executed via <code>su</code>.
<tr>
<td>sdcard
<td><p>Location: <code>/mnt/sdcard/test_root</code></p>
This location is not supported on Android 11+ due to the
<a href="https://developer.android.com/about/versions/11/privacy/storage">
changes related to scoped storage</a>.
</table>
#### <code>-b <var>BINARY</var></code> / <code>&#x2D;&#x2D;binary <var>BINARY</var></code>
Path to the Firefox binary to use. By default geckodriver tries to

Просмотреть файл

@ -1,5 +1,5 @@
use crate::capabilities::AndroidOptions;
use mozdevice::{AndroidStorage, Device, Host};
use mozdevice::{Device, Host};
use mozprofile::profile::Profile;
use serde::Serialize;
use serde_yaml::{Mapping, Value};
@ -141,11 +141,6 @@ impl Drop for AndroidHandler {
impl AndroidHandler {
pub fn new(options: &AndroidOptions, host_port: u16) -> Result<AndroidHandler> {
// We need to push profile.pathbuf to a safe space on the device.
// Make it per-Android package to avoid clashes and confusion.
// This naming scheme follows GeckoView's configuration file naming scheme,
// see bug 1533385.
let host = Host {
host: None,
port: None,
@ -153,7 +148,7 @@ impl AndroidHandler {
write_timeout: Some(time::Duration::from_millis(5000)),
};
let mut device = host.device_or_default(options.device_serial.as_ref(), options.storage)?;
let device = host.device_or_default(options.device_serial.as_ref())?;
// Set up port forward. Port forwarding will be torn down, if possible,
device.forward_port(host_port, TARGET_PORT)?;
@ -162,29 +157,6 @@ impl AndroidHandler {
host_port, TARGET_PORT
);
let test_root = match device.storage {
AndroidStorage::App => {
device.run_as_package = Some(options.package.to_owned());
let mut buf = PathBuf::from("/data/data");
buf.push(&options.package);
buf.push("test_root");
buf
}
AndroidStorage::Internal => PathBuf::from("/data/local/tmp/test_root"),
AndroidStorage::Sdcard => PathBuf::from("/mnt/sdcard/test_root"),
};
debug!(
"Connecting: options={:?}, storage={:?}) test_root={}, run_as_package={:?}",
options,
device.storage,
test_root.display(),
device.run_as_package
);
let mut profile = test_root.clone();
profile.push(format!("{}-geckodriver-profile", &options.package));
// Check if the specified package is installed
let response =
device.execute_host_shell_command(&format!("pm list packages {}", &options.package))?;
@ -198,6 +170,26 @@ impl AndroidHandler {
return Err(AndroidError::PackageNotFound(options.package.clone()));
}
// We need to push the profile to a location on the device that can also
// be read and write by the application, and works for unrooted devices.
// The only location that meets this criteria is under:
// $EXTERNAL_STORAGE/Android/data/%package_name%/files
let external_storage = device.execute_host_shell_command("echo $EXTERNAL_STORAGE")?;
let mut test_root = PathBuf::from(external_storage.trim_end_matches('\n'));
test_root.push("Android/data");
test_root.push(&options.package);
test_root.push("files/test_root");
debug!(
"Connecting: options={:?}, test_root={}, run_as_package={:?}",
options,
test_root.display(),
device.run_as_package
);
let mut profile = test_root.clone();
profile.push("profile");
let config = PathBuf::from(format!(
"/data/local/tmp/{}-geckoview-config.yaml",
&options.package
@ -398,73 +390,18 @@ mod test {
use crate::android::AndroidHandler;
use crate::capabilities::AndroidOptions;
use mozdevice::{AndroidStorage, AndroidStorageInput};
use std::path::PathBuf;
fn run_handler_storage_test(package: &str, storage: AndroidStorageInput) {
let options = AndroidOptions::new(package.to_owned(), storage);
#[test]
fn android_handler_test_root_and_profile_path() {
let package = "org.mozilla.geckoview_example";
let options = AndroidOptions::new(package.to_owned());
let handler = AndroidHandler::new(&options, 4242).expect("has valid Android handler");
assert_eq!(handler.options, options);
assert_eq!(handler.process.package, package);
let path = format!("/Android/data/{}/files/test_root", package);
assert!(handler.test_root.display().to_string().contains(&path));
let expected_config_path = PathBuf::from(format!(
"/data/local/tmp/{}-geckoview-config.yaml",
&package
));
assert_eq!(handler.config, expected_config_path);
if handler.process.device.storage == AndroidStorage::App {
assert_eq!(
handler.process.device.run_as_package,
Some(package.to_owned())
);
} else {
assert_eq!(handler.process.device.run_as_package, None);
}
let test_root = match handler.process.device.storage {
AndroidStorage::App => {
let mut buf = PathBuf::from("/data/data");
buf.push(&package);
buf.push("test_root");
buf
}
AndroidStorage::Internal => PathBuf::from("/data/local/tmp/test_root"),
AndroidStorage::Sdcard => PathBuf::from("/mnt/sdcard/test_root"),
};
assert_eq!(handler.test_root, test_root);
let mut profile = test_root.clone();
profile.push(format!("{}-geckodriver-profile", &package));
assert_eq!(handler.profile, profile);
}
#[test]
#[ignore]
fn android_handler_storage_as_app() {
let package = "org.mozilla.geckoview_example";
run_handler_storage_test(&package, AndroidStorageInput::App);
}
#[test]
#[ignore]
fn android_handler_storage_as_auto() {
let package = "org.mozilla.geckoview_example";
run_handler_storage_test(package, AndroidStorageInput::Auto);
}
#[test]
#[ignore]
fn android_handler_storage_as_internal() {
let package = "org.mozilla.geckoview_example";
run_handler_storage_test(package, AndroidStorageInput::Internal);
}
#[test]
#[ignore]
fn android_handler_storage_as_sdcard() {
let package = "org.mozilla.geckoview_example";
run_handler_storage_test(package, AndroidStorageInput::Sdcard);
let profile_path = format!("{}/profile", path);
assert!(handler.profile.display().to_string().ends_with(&profile_path));
}
}

Просмотреть файл

@ -5,7 +5,6 @@
use crate::command::LogOptions;
use crate::logging::Level;
use base64;
use mozdevice::AndroidStorageInput;
use mozprofile::preferences::Pref;
use mozprofile::profile::Profile;
use mozrunner::runner::platform::firefox_default_path;
@ -349,14 +348,12 @@ pub struct AndroidOptions {
pub device_serial: Option<String>,
pub intent_arguments: Option<Vec<String>>,
pub package: String,
pub storage: AndroidStorageInput,
}
impl AndroidOptions {
pub fn new(package: String, storage: AndroidStorageInput) -> AndroidOptions {
pub fn new(package: String) -> AndroidOptions {
AndroidOptions {
package,
storage,
..Default::default()
}
}
@ -386,7 +383,6 @@ impl FirefoxOptions {
pub fn from_capabilities(
binary_path: Option<PathBuf>,
android_storage: AndroidStorageInput,
matched: &mut Capabilities,
) -> WebDriverResult<FirefoxOptions> {
let mut rv = FirefoxOptions::new();
@ -401,7 +397,7 @@ impl FirefoxOptions {
)
})?;
rv.android = FirefoxOptions::load_android(android_storage, &options)?;
rv.android = FirefoxOptions::load_android(&options)?;
rv.args = FirefoxOptions::load_args(&options)?;
rv.env = FirefoxOptions::load_env(&options)?;
rv.log = FirefoxOptions::load_log(&options)?;
@ -559,7 +555,6 @@ impl FirefoxOptions {
}
pub fn load_android(
storage: AndroidStorageInput,
options: &Capabilities,
) -> WebDriverResult<Option<AndroidOptions>> {
if let Some(package_json) = options.get("androidPackage") {
@ -583,7 +578,7 @@ impl FirefoxOptions {
));
}
let mut android = AndroidOptions::new(package, storage);
let mut android = AndroidOptions::new(package);
android.activity = match options.get("androidActivity") {
Some(json) => {
@ -725,7 +720,6 @@ mod tests {
use self::mozprofile::preferences::Pref;
use super::*;
use crate::marionette::MarionetteHandler;
use mozdevice::AndroidStorageInput;
use serde_json::json;
use std::default::Default;
use std::fs::File;
@ -744,7 +738,7 @@ mod tests {
let mut caps = Capabilities::new();
caps.insert("moz:firefoxOptions".into(), Value::Object(firefox_opts));
FirefoxOptions::from_capabilities(None, AndroidStorageInput::Auto, &mut caps)
FirefoxOptions::from_capabilities(None, &mut caps)
}
#[test]
@ -764,7 +758,7 @@ mod tests {
let mut caps = Capabilities::new();
let opts =
FirefoxOptions::from_capabilities(None, AndroidStorageInput::Auto, &mut caps).unwrap();
FirefoxOptions::from_capabilities(None, &mut caps).unwrap();
assert_eq!(opts.android, None);
assert_eq!(opts.args, None);
assert_eq!(opts.binary, None);
@ -784,7 +778,6 @@ mod tests {
let opts = FirefoxOptions::from_capabilities(
Some(binary.clone()),
AndroidStorageInput::Auto,
&mut caps,
)
.unwrap();
@ -799,7 +792,7 @@ mod tests {
fn fx_options_from_capabilities_with_debugger_address_not_set() {
let mut caps = Capabilities::new();
let opts = FirefoxOptions::from_capabilities(None, AndroidStorageInput::Auto, &mut caps)
let opts = FirefoxOptions::from_capabilities(None, &mut caps)
.expect("Valid Firefox options");
assert!(
@ -813,7 +806,7 @@ mod tests {
let mut caps = Capabilities::new();
caps.insert("moz:debuggerAddress".into(), json!(false));
let opts = FirefoxOptions::from_capabilities(None, AndroidStorageInput::Auto, &mut caps)
let opts = FirefoxOptions::from_capabilities(None, &mut caps)
.expect("Valid Firefox options");
assert!(
@ -827,7 +820,7 @@ mod tests {
let mut caps = Capabilities::new();
caps.insert("moz:debuggerAddress".into(), json!(true));
let opts = FirefoxOptions::from_capabilities(None, AndroidStorageInput::Auto, &mut caps)
let opts = FirefoxOptions::from_capabilities(None, &mut caps)
.expect("Valid Firefox options");
if let Some(args) = opts.args {
@ -851,7 +844,7 @@ mod tests {
let mut caps = Capabilities::new();
caps.insert("moz:firefoxOptions".into(), json!(42));
FirefoxOptions::from_capabilities(None, AndroidStorageInput::Auto, &mut caps)
FirefoxOptions::from_capabilities(None, &mut caps)
.expect_err("Firefox options need to be of type object");
}
@ -873,10 +866,7 @@ mod tests {
let opts = make_options(firefox_opts).expect("valid firefox options");
assert_eq!(
opts.android,
Some(AndroidOptions::new(
value.to_string(),
AndroidStorageInput::Auto
))
Some(AndroidOptions::new(value.to_string()))
);
}
}

Просмотреть файл

@ -57,7 +57,6 @@ pub mod test;
use crate::command::extension_routes;
use crate::logging::Level;
use crate::marionette::{MarionetteHandler, MarionetteSettings};
use mozdevice::AndroidStorageInput;
const EXIT_SUCCESS: i32 = 0;
const EXIT_USAGE: i32 = 64;
@ -159,8 +158,6 @@ fn parse_args(app: &mut App) -> ProgramResult<Operation> {
Err(e) => usage!("{}: {}:{}", e, host, port),
};
let android_storage = value_t!(matches, "android_storage", AndroidStorageInput)?;
let binary = matches.value_of("binary").map(PathBuf::from);
let marionette_host = matches.value_of("marionette_host").unwrap();
@ -183,7 +180,6 @@ fn parse_args(app: &mut App) -> ProgramResult<Operation> {
binary,
connect_existing: matches.is_present("connect_existing"),
jsdebugger: matches.is_present("jsdebugger"),
android_storage,
};
Operation::Server {
log_level,
@ -321,14 +317,6 @@ fn make_app<'a, 'b>() -> App<'a, 'b> {
.long("version")
.help("Prints version and copying information"),
)
.arg(
Arg::with_name("android_storage")
.long("android-storage")
.possible_values(&["auto", "app", "internal", "sdcard"])
.default_value("auto")
.value_name("ANDROID_STORAGE")
.help("Selects storage location to be used for test data."),
)
}
fn get_program_name() -> String {

Просмотреть файл

@ -21,7 +21,6 @@ use marionette_rs::webdriver::{
ScreenshotOptions, Script as MarionetteScript, Selector as MarionetteSelector,
Url as MarionetteUrl, WindowRect as MarionetteWindowRect,
};
use mozdevice::AndroidStorageInput;
use mozprofile::preferences::Pref;
use mozprofile::profile::Profile;
use mozrunner::runner::{FirefoxProcess, FirefoxRunner, Runner, RunnerProcess};
@ -100,8 +99,6 @@ pub struct MarionetteSettings {
/// Brings up the Browser Toolbox when starting Firefox,
/// letting you debug internals.
pub jsdebugger: bool,
pub android_storage: AndroidStorageInput,
}
#[derive(Default)]
@ -138,7 +135,6 @@ impl MarionetteHandler {
let options = FirefoxOptions::from_capabilities(
fx_capabilities.chosen_binary,
self.settings.android_storage,
&mut capabilities,
)?;
(options, capabilities)

Просмотреть файл

@ -26,7 +26,7 @@ use std::iter::FromIterator;
use std::net::TcpStream;
use std::num::{ParseIntError, TryFromIntError};
use std::path::{Path, PathBuf};
use std::str::{FromStr, Utf8Error};
use std::str::Utf8Error;
use std::time::{Duration, SystemTime};
use uuid::Uuid;
use walkdir::WalkDir;
@ -37,46 +37,10 @@ pub type Result<T> = std::result::Result<T, DeviceError>;
static SYNC_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"[^A-Za-z0-9_@%+=:,./-]").unwrap());
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum AndroidStorageInput {
Auto,
App,
Internal,
Sdcard,
}
impl FromStr for AndroidStorageInput {
type Err = DeviceError;
fn from_str(s: &str) -> Result<Self> {
match s {
"auto" => Ok(AndroidStorageInput::Auto),
"app" => Ok(AndroidStorageInput::App),
"internal" => Ok(AndroidStorageInput::Internal),
"sdcard" => Ok(AndroidStorageInput::Sdcard),
_ => Err(DeviceError::InvalidStorage),
}
}
}
impl Default for AndroidStorageInput {
fn default() -> Self {
AndroidStorageInput::Auto
}
}
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum AndroidStorage {
App,
Internal,
Sdcard,
}
#[derive(Debug)]
pub enum DeviceError {
Adb(String),
FromInt(TryFromIntError),
InvalidStorage,
Io(io::Error),
MissingPackage,
MultipleDevices,
@ -91,7 +55,6 @@ impl fmt::Display for DeviceError {
match *self {
DeviceError::Adb(ref message) => message.fmt(f),
DeviceError::FromInt(ref int) => int.fmt(f),
DeviceError::InvalidStorage => write!(f, "Invalid storage"),
DeviceError::Io(ref error) => error.fmt(f),
DeviceError::MissingPackage => write!(f, "Missing package"),
DeviceError::MultipleDevices => write!(f, "Multiple Android devices online"),
@ -302,8 +265,7 @@ impl Host {
/// the `ANDROID_SERIAL` environment variable can be used to select one.
pub fn device_or_default<T: AsRef<str>>(
self,
device_serial: Option<&T>,
storage: AndroidStorageInput,
device_serial: Option<&T>
) -> Result<Device> {
let serials: Vec<String> = self
.devices::<Vec<_>>()?
@ -319,7 +281,7 @@ impl Host {
return Err(DeviceError::UnknownDevice(serial.clone()));
}
return Device::new(self, serial.to_owned(), storage);
return Device::new(self, serial.to_owned());
}
if serials.len() > 1 {
@ -327,7 +289,7 @@ impl Host {
}
if let Some(ref serial) = serials.first() {
return Device::new(self, serial.to_owned().to_string(), storage);
return Device::new(self, serial.to_owned().to_string());
}
Err(DeviceError::Adb("No Android devices are online".to_owned()))
@ -407,21 +369,18 @@ pub struct Device {
pub run_as_package: Option<String>,
pub storage: AndroidStorage,
/// Cache intermediate tempfile name used in pushing via run_as.
pub tempfile: PathBuf,
}
impl Device {
pub fn new(host: Host, serial: DeviceSerial, storage: AndroidStorageInput) -> Result<Device> {
pub fn new(host: Host, serial: DeviceSerial) -> Result<Device> {
let mut device = Device {
host,
serial,
adbd_root: false,
is_rooted: false,
run_as_package: None,
storage: AndroidStorage::App,
su_c_root: false,
su_0_root: false,
tempfile: PathBuf::from("/data/local/tmp"),
@ -443,19 +402,13 @@ impl Device {
.map_or(false, uid_check);
device.is_rooted = device.adbd_root || device.su_0_root || device.su_c_root;
device.storage = match storage {
AndroidStorageInput::App => AndroidStorage::App,
AndroidStorageInput::Internal => AndroidStorage::Internal,
AndroidStorageInput::Sdcard => AndroidStorage::Sdcard,
AndroidStorageInput::Auto => match device.is_rooted {
true => AndroidStorage::Internal,
false => AndroidStorage::App,
},
};
// Set Permissive=1 if we have root.
if device.is_rooted {
debug!("Device is rooted");
// Set Permissive=1 if we have root.
device.execute_host_shell_command("setenforce permissive")?;
} else {
debug!("Device is unrooted");
}
Ok(device)

Просмотреть файл

@ -13,7 +13,6 @@ use crate::*;
//
// $ cargo test -- --ignored --test-threads=1
use crate::{AndroidStorage, AndroidStorageInput};
use std::collections::BTreeSet;
use std::panic;
use tempfile::{tempdir, TempDir};
@ -81,7 +80,7 @@ where
..Default::default()
};
let device = host
.device_or_default::<String>(None, AndroidStorageInput::Auto)
.device_or_default::<String>(None)
.expect("device_or_default");
let tmp_dir = tempdir().expect("create temp dir");
@ -131,7 +130,7 @@ fn host_device_or_default() {
let expected_device = devices.first().expect("found a device");
let device = host
.device_or_default::<String>(Some(&expected_device.serial), AndroidStorageInput::App)
.device_or_default::<String>(Some(&expected_device.serial))
.expect("connected device with serial");
assert_eq!(device.run_as_package, None);
assert_eq!(device.serial, expected_device.serial);
@ -145,7 +144,7 @@ fn host_device_or_default_invalid_serial() {
..Default::default()
};
host.device_or_default::<String>(Some(&"foobar".to_owned()), AndroidStorageInput::Auto)
host.device_or_default::<String>(Some(&"foobar".to_owned()))
.expect_err("invalid serial");
}
@ -160,67 +159,11 @@ fn host_device_or_default_no_serial() {
let expected_device = devices.first().expect("found a device");
let device = host
.device_or_default::<String>(None, AndroidStorageInput::Auto)
.device_or_default::<String>(None)
.expect("connected device with serial");
assert_eq!(device.serial, expected_device.serial);
}
#[test]
#[ignore]
fn host_device_or_default_storage_as_app() {
let host = Host {
..Default::default()
};
let device = host
.device_or_default::<String>(None, AndroidStorageInput::App)
.expect("connected device");
assert_eq!(device.storage, AndroidStorage::App);
}
#[test]
#[ignore]
fn host_device_or_default_storage_as_auto() {
let host = Host {
..Default::default()
};
let device = host
.device_or_default::<String>(None, AndroidStorageInput::Auto)
.expect("connected device");
if device.is_rooted {
assert_eq!(device.storage, AndroidStorage::Internal);
} else {
assert_eq!(device.storage, AndroidStorage::App);
}
}
#[test]
#[ignore]
fn host_device_or_default_storage_as_internal() {
let host = Host {
..Default::default()
};
let device = host
.device_or_default::<String>(None, AndroidStorageInput::Internal)
.expect("connected device");
assert_eq!(device.storage, AndroidStorage::Internal);
}
#[test]
#[ignore]
fn host_device_or_default_storage_as_sdcard() {
let host = Host {
..Default::default()
};
let device = host
.device_or_default::<String>(None, AndroidStorageInput::Sdcard)
.expect("connected device");
assert_eq!(device.storage, AndroidStorage::Sdcard);
}
#[test]
#[ignore]
fn device_shell_command() {
@ -509,10 +452,6 @@ fn device_push_dir() {
#[test]
fn format_own_device_error_types() {
assert_eq!(
format!("{}", DeviceError::InvalidStorage),
"Invalid storage".to_string()
);
assert_eq!(
format!("{}", DeviceError::MissingPackage),
"Missing package".to_string()