Commit 75438b2a authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

[Android] Add support for overriding user data dir in Java browsertests

On Android, the user data dir in Chrome and WebLayer by default is
ANDROID_APP_DATA_DIR. In the context of Java browsertests, this
directory is set to the isolated test root, which is problematic as it
can't be cleared between runs (see linked bug). This bleedthrough of
state can result in cryptic test failures.

//chrome's android_browsertests handles this issue by overriding the
user data directory in C++ at the //chrome level
(https://chromium-review.googlesource.com/c/chromium/src/+/1677591).
However, we are now facing this issue for weblayer_browsertests.

This CL introduces a general solution for this problem at the level of
NativeBrowserTestsActivity.java by allowing subclasses of this class
to specify the command-line switch for the user data directory. Doing
this specification will result in the user data directory getting
overridden to the embedder-specified private data directory, which
*is* cleared between each run by NativeBrowserTestsActivity. We do
this specification for //weblayer as well as Content Shell-based
browsertests. The latter solves the problem for the existing
content_browsertests and components_browsertests suites, which are
run on Android.

//chrome's ChromeBrowserTestActivity.java does not subclass
NativeBrowserTestsActivity as it subclasses ChromeTabbedActivity.
Changing //chrome's android_browsertests to use this mechanism rather
than its current one is left as future work.

Finally, we note that this solution is a stopgap, as it leaves open the
possibility that the C++ and Java sides have different views of the
conceptual "user data dir". The proper solution for this problem would
be for ANDROID_APP_DATA_DIR to *not* be set to the isolated test root
but rather to a directory that is cleared between each run (e.g., a
temp directory). That solution is however not easily accessible for
reasons detailed in the bug. In the meantime, the stopgap solution of
changing the user data dir on the C++ side only has been working in the
context of android_browsertests for ~18 months.

Bug: 617734
Change-Id: I8dd2fe25595f01a540af9a6c6c4787bb6570b62b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504262Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822575}
parent d49bdbc1
...@@ -89,6 +89,16 @@ public abstract class ContentShellBrowserTestActivity extends NativeBrowserTestA ...@@ -89,6 +89,16 @@ public abstract class ContentShellBrowserTestActivity extends NativeBrowserTestA
}); });
} }
@Override
/**
* Ensure that the user data directory gets overridden to getPrivateDataDirectory() (which is
* cleared at the start of every run); the directory that ANDROID_APP_DATA_DIR is set to in the
* context of Java browsertests is not cleared as it also holds persistent state, which
* causes test failures due to state bleedthrough. See crbug.com/617734 for details.
*/
protected String getUserDataDirectoryCommandLineSwitch() {
return "data-path";
}
protected abstract int getTestActivityViewId(); protected abstract int getTestActivityViewId();
protected abstract int getShellManagerViewId(); protected abstract int getShellManagerViewId();
......
...@@ -9,6 +9,9 @@ ...@@ -9,6 +9,9 @@
namespace switches { namespace switches {
// Makes Content Shell use the given path for its data directory. // Makes Content Shell use the given path for its data directory.
// NOTE: If changing this value, change the corresponding Java-side value in
// ContentShellBrowserTestActivity.java#getUserDataDirectoryCommandLineSwitch()
// to match.
const char kContentShellDataPath[] = "data-path"; const char kContentShellDataPath[] = "data-path";
// The directory breakpad should store minidumps in. // The directory breakpad should store minidumps in.
......
...@@ -27,6 +27,12 @@ public abstract class NativeBrowserTestActivity extends FragmentActivity { ...@@ -27,6 +27,12 @@ public abstract class NativeBrowserTestActivity extends FragmentActivity {
for (String flag : NativeBrowserTest.BROWSER_TESTS_FLAGS) { for (String flag : NativeBrowserTest.BROWSER_TESTS_FLAGS) {
appendCommandLineFlags(flag); appendCommandLineFlags(flag);
} }
String userDataDirSwitch = getUserDataDirectoryCommandLineSwitch();
if (!userDataDirSwitch.isEmpty()) {
String userDataDirFlag = "--" + userDataDirSwitch + "=" + getPrivateDataDirectory();
appendCommandLineFlags(userDataDirFlag);
}
} }
@Override @Override
...@@ -55,6 +61,21 @@ public abstract class NativeBrowserTestActivity extends FragmentActivity { ...@@ -55,6 +61,21 @@ public abstract class NativeBrowserTestActivity extends FragmentActivity {
/** Returns the test suite's private data directory. */ /** Returns the test suite's private data directory. */
protected abstract File getPrivateDataDirectory(); protected abstract File getPrivateDataDirectory();
/**
* Returns the command line switch used to specify the user data directory.
*
* The default implementation returns an empty string, which means no user
* data directory.
* If this method returns a non-empty value, the user data directory will be overridden to be
* the private data directory, which is cleared at the beginning of each test run.
* NOTE: The switch should not start with "--".
* TODO(crbug.com/617734): Solve this problem holistically for Java and C++ at the level of
* DIR_ANDROID_APP_DATA and eliminate the need for this solution.
*/
protected String getUserDataDirectoryCommandLineSwitch() {
return "";
}
/** Initializes the browser process. /** Initializes the browser process.
* *
* This generally includes loading native libraries and switching to the native command line, * This generally includes loading native libraries and switching to the native command line,
......
...@@ -14,6 +14,9 @@ const char kDisableAutoReload[] = "disable-auto-reload"; ...@@ -14,6 +14,9 @@ const char kDisableAutoReload[] = "disable-auto-reload";
const char kEnableAutoReload[] = "enable-auto-reload"; const char kEnableAutoReload[] = "enable-auto-reload";
// Makes WebLayer Shell use the given path for its data directory. // Makes WebLayer Shell use the given path for its data directory.
// NOTE: If changing this value, change the corresponding Java-side value in
// WebLayerBrowserTestsActivity.java#getUserDataDirectoryCommandLineSwitch() to
// match.
const char kWebLayerUserDataDir[] = "weblayer-user-data-dir"; const char kWebLayerUserDataDir[] = "weblayer-user-data-dir";
} // namespace switches } // namespace switches
......
...@@ -119,4 +119,15 @@ public class WebLayerBrowserTestsActivity extends NativeBrowserTestActivity { ...@@ -119,4 +119,15 @@ public class WebLayerBrowserTestsActivity extends NativeBrowserTestActivity {
return new File(UrlUtils.getIsolatedTestRoot(), return new File(UrlUtils.getIsolatedTestRoot(),
WebLayerBrowserTestsApplication.PRIVATE_DATA_DIRECTORY_SUFFIX); WebLayerBrowserTestsApplication.PRIVATE_DATA_DIRECTORY_SUFFIX);
} }
@Override
/**
* Ensure that the user data directory gets overridden to getPrivateDataDirectory() (which is
* cleared at the start of every run); the directory that ANDROID_APP_DATA_DIR is set to in the
* context of Java browsertests is not cleared as it also holds persistent state, which
* causes test failures due to state bleedthrough. See crbug.com/617734 for details.
*/
protected String getUserDataDirectoryCommandLineSwitch() {
return "weblayer-user-data-dir";
}
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment