Make ChromeBrowserTestsActivity inherit from ChromeTabbedActivity
We want android_browsertests to run a full Chromium browser. The piece
of Java code that owns the various top level UI is the
ChromeTabbedActivity, so we want to use that. We make our test activity
inherit from that, and thus instantiate its own NativeTest and run it
when Chromium Java startup is complete.
We copy a number of services and activities over from the Chromium
AndroidManifest.xml which are referred to during initialization and
cause exceptions to be thrown when not present. Long-term we should
merge the browser test activity into the main Chromium manifest file
and optionally use it when building android_browsertests.
We add some dependencies for various pieces needed to run a full
Chromium browser, that fail to load at runtime otherwise.
We remove the initChromiumBrowserProcessForTests() method which is
now unused as browser tests inject a replacement for ContentMain()
but otherwise use the regular startup paths.
We also remove the handlePostNativeStartupSynchronously() paths as
they are not used anymore. This is done as part of pumping the Java
startup tasks.
R=jbudorick@chromium.org
Bug: 961849
Change-Id: I4c59c450c2ad2da39473331e82a3f4959f33897c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1672197
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672263}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/NativeStartupBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/init/NativeStartupBridge.java
index dda29fe..d1e012d2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/init/NativeStartupBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/init/NativeStartupBridge.java
@@ -39,16 +39,4 @@
}
});
}
-
- @CalledByNative
- private static void handlePostNativeStartupSynchronously() {
- final BrowserParts parts = new EmptyBrowserParts() {};
- try {
- ChromeBrowserInitializer.getInstance().handlePostNativeStartup(
- /*isAsync=*/false, parts);
- } catch (ProcessInitException e) {
- Log.e(TAG, "Cannot handlePostNativeStartup synchronously.", e);
- System.exit(-1);
- }
- }
}
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/background_task_scheduler/NativeBackgroundTaskTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/background_task_scheduler/NativeBackgroundTaskTest.java
index 8c7def2..dd5d60a 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/background_task_scheduler/NativeBackgroundTaskTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/background_task_scheduler/NativeBackgroundTaskTest.java
@@ -88,8 +88,6 @@
public void addStartupCompletedObserver(StartupCallback callback) {}
@Override
- public void initChromiumBrowserProcessForTests() {}
- @Override
public void setContentMainCallbackForTests(Runnable r) {}
public void setIsStartupSuccessfullyCompleted(boolean flag) {
diff --git a/chrome/browser/android/startup_bridge.cc b/chrome/browser/android/startup_bridge.cc
index 12af6b0..a88bba9 100644
--- a/chrome/browser/android/startup_bridge.cc
+++ b/chrome/browser/android/startup_bridge.cc
@@ -22,11 +22,4 @@
Java_NativeStartupBridge_loadFullBrowser(env);
}
-void HandlePostNativeStartupSynchronously() {
- // The C++ initialization of the browser has already been done.
- DCHECK(g_browser_process);
- JNIEnv* env = base::android::AttachCurrentThread();
- Java_NativeStartupBridge_handlePostNativeStartupSynchronously(env);
-}
-
} // namespace android_startup
diff --git a/chrome/browser/android/startup_bridge.h b/chrome/browser/android/startup_bridge.h
index 0c1b40b..913cef9 100644
--- a/chrome/browser/android/startup_bridge.h
+++ b/chrome/browser/android/startup_bridge.h
@@ -7,11 +7,6 @@
namespace android_startup {
extern void LoadFullBrowser();
-
-// Called to start up java bits of chrome synchronously after the C++ bits have
-// been initialized. Used for browser tests which must run the browser
-// synchronously before running the test.
-extern void HandlePostNativeStartupSynchronously();
}
#endif // CHROME_BROWSER_ANDROID_STARTUP_BRIDGE_H_
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index a704a29c..c405780 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -471,6 +471,7 @@
":test_support_ui_android",
"//chrome:chrome_android_core",
"//chrome/android:app_hooks_java",
+ "//components/crash/android:crashpad_main",
# TODO(crbug.com/961849): This is needed for ShellManager which is what
# the ChromeBrowserTestsActivity is using to build the java UI. It's
@@ -544,7 +545,11 @@
deps = [
"//base:base_java",
"//base:base_java_test_support",
+ "//build/android/buildhooks:build_hooks_android_java",
"//chrome/android:chrome_all_java",
+ "//components/crash/android:handler_java",
+ "//components/crash/android:java",
+ "//components/module_installer/android:module_installer_impl_java",
"//content/public/android:content_java",
"//content/public/test/android:android_test_message_pump_support_java",
"//content/shell/android:content_shell_browsertests_java",
diff --git a/chrome/test/android/browsertests_apk/AndroidManifest.xml.jinja2 b/chrome/test/android/browsertests_apk/AndroidManifest.xml.jinja2
index 1af39f6..2c15484 100644
--- a/chrome/test/android/browsertests_apk/AndroidManifest.xml.jinja2
+++ b/chrome/test/android/browsertests_apk/AndroidManifest.xml.jinja2
@@ -7,15 +7,19 @@
-->
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ xmlns:tools="http://schemas.android.com/tools"
package="org.chromium.android_browsertests_apk">
<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="23" />
+ <uses-feature android:glEsVersion="0x00020000" />
+
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/>
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
<uses-permission android:name="android.permission.CAMERA" />
<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.MODIFY_AUDIO_SETTINGS"/>
<uses-permission android:name="android.permission.READ_SYNC_SETTINGS"/>
+ <uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED"/>
<uses-permission android:name="android.permission.RECORD_AUDIO"/>
<uses-permission android:name="android.permission.VIBRATE"/>
<uses-permission android:name="android.permission.WAKE_LOCK"/>
@@ -25,7 +29,7 @@
android:label="ChromeBrowserTests">
<activity android:name="ChromeBrowserTestsActivity"
android:launchMode="singleTask"
- android:theme="@android:style/Theme.Holo.Light.NoActionBar"
+ android:theme="@style/Theme.Chromium.TabbedMode"
android:configChanges="orientation|keyboardHidden|keyboard|screenSize"
android:hardwareAccelerated="true"
android:process=":test_process">
@@ -34,8 +38,61 @@
<category android:name="android.intent.category.LAUNCHER"/>
</intent-filter>
</activity>
- <!-- The following service entries exist in order to allow us to
- start more than one sandboxed process. -->
+
+ <activity android:name="org.chromium.chrome.browser.media.MediaLauncherActivity"
+ android:theme="@android:style/Theme.NoDisplay"
+ android:excludeFromRecents="true"
+ android:exported="true"
+ android:enabled="false"><!-- This will be selectively enabled at runtime. -->
+ <intent-filter tools:ignore="AppLinkUrlError">
+ <action android:name="android.intent.action.VIEW" />
+ <category android:name="android.intent.category.DEFAULT" />
+ <!-- TODO(https://crbug.com/800875): Limit these to
+ supported MIME types. -->
+ <data android:mimeType="image/*" />
+ <data android:mimeType="video/*" />
+ <data android:scheme="file" />
+ <data android:scheme="content" />
+ </intent-filter>
+ </activity>
+ <activity-alias android:name="org.chromium.chrome.browser.media.AudioLauncherActivity"
+ android:targetActivity="org.chromium.chrome.browser.media.MediaLauncherActivity"
+ android:theme="@android:style/Theme.NoDisplay"
+ android:excludeFromRecents="true"
+ android:exported="true"
+ android:enabled="false"><!-- This will be selectively enabled at runtime. -->
+ <intent-filter tools:ignore="AppLinkUrlError">
+ <action android:name="android.intent.action.VIEW" />
+ <category android:name="android.intent.category.DEFAULT" />
+ <!-- TODO(https://crbug.com/800875): Limit these to supported MIME types. -->
+ <data android:mimeType="audio/*" />
+ <data android:scheme="file" />
+ <data android:scheme="content" />
+ </intent-filter>
+ </activity-alias>
+
+ <activity android:name="org.chromium.chrome.browser.incognito.IncognitoTabLauncher"
+ android:theme="@android:style/Theme.NoDisplay"
+ android:taskAffinity=""
+ android:enabled="false"
+ android:excludeFromRecents="true"
+ android:exported="true">
+ <intent-filter>
+ <action android:name="org.chromium.chrome.browser.incognito.OPEN_PRIVATE_TAB" />
+ <category android:name="android.intent.category.DEFAULT" />
+ </intent-filter>
+ </activity>
+
+ <service android:name="org.chromium.components.background_task_scheduler.BackgroundTaskJobService"
+ android:exported="false"
+ android:permission="android.permission.BIND_JOB_SERVICE"/>
+
+ <service android:name="org.chromium.chrome.browser.crash.ChromeMinidumpUploadJobService"
+ android:permission="android.permission.BIND_JOB_SERVICE"
+ android:exported="false"/>
+
+ <!-- The following service entries exist in order to allow us
+ to start more than one sandboxed process. -->
<!-- NOTE: If you change the values of "android:process" for any of the below services,
you also need to update kHelperProcessExecutableName in chrome_constants.cc. -->
diff --git a/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsActivity.java b/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsActivity.java
index e598bf6..87281dd 100644
--- a/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsActivity.java
+++ b/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsActivity.java
@@ -4,135 +4,76 @@
package org.chromium.android_browsertests_apk;
-import android.os.Bundle;
-import android.view.Window;
-import android.view.WindowManager;
+import android.content.Intent;
-import org.chromium.base.Log;
-import org.chromium.base.StrictModeContext;
-import org.chromium.base.ThreadUtils;
-import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.library_loader.LibraryProcessType;
-import org.chromium.base.library_loader.ProcessInitException;
import org.chromium.base.test.util.UrlUtils;
-import org.chromium.chrome.browser.init.BrowserParts;
-import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
-import org.chromium.chrome.browser.init.EmptyBrowserParts;
+import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.content_public.browser.BrowserStartupController;
import org.chromium.native_test.NativeBrowserTest;
-import org.chromium.native_test.NativeBrowserTestActivity;
+import org.chromium.native_test.NativeTest;
import java.io.File;
/**
* Android activity for running chrome browser tests.
*/
-public class ChromeBrowserTestsActivity extends NativeBrowserTestActivity {
+public class ChromeBrowserTestsActivity extends ChromeTabbedActivity {
private static final String TAG = "cr_browser_test";
- @Override
- public void onCreate(Bundle savedInstanceState) {
- super.onCreate(savedInstanceState);
- appendCommandLineFlags(
- "--remote-debugging-socket-name android_browsertests_devtools_remote");
- }
+ private NativeTest mTest = new NativeTest();
@Override
- protected void initializeBrowserProcess() {
- // TODO(ajwong): This is forked from ContentShellBrowserTestActivity. Should it be pulled
- // out into a static?
- try (StrictModeContext unused = StrictModeContext.allowDiskReads()) {
- LibraryLoader.getInstance().ensureInitialized(LibraryProcessType.PROCESS_BROWSER);
- } catch (ProcessInitException e) {
- Log.e(TAG, "Cannot load android_browsertests.", e);
- System.exit(-1);
+ public void performPreInflationStartup() {
+ // These steps for NativeTest are usually performed in onCreate, but we can not
+ // override onCreate in this class since a super class marks it as final. The
+ // performPreInflationStartup() steps is another early step in initialization of the
+ // activity so we do that here.
+ mTest.preCreate(this);
+ super.performPreInflationStartup();
+ // Sets up the command line for tests.
+ mTest.postCreate(this);
+ // Append things we want for Android-based browser tests. C++ will also append things.
+ for (String flag : NativeBrowserTest.BROWSER_TESTS_FLAGS) {
+ mTest.appendCommandLineFlags(flag);
}
+ mTest.appendCommandLineFlags(
+ "--remote-debugging-socket-name android_browsertests_devtools_remote");
- // Don't use the production BrowserStartupController, as we want to replace it with
- // one that doesn't actually run ContentMain(). The BrowserTestBase class does the
- // work of ContentMain() itself once we pass control to C++ to run tests. That occurs
- // after this current method runs.
- // This replacement startup controller will be used when the C++ code calls into
- // ChromeBrowserTestsActivity.handlePostNativeStartup() to modify that behaviour.
- BrowserStartupController startupController = new BrowserStartupController() {
- @Override
- public void startBrowserProcessesAsync(boolean startGpuProcess,
- boolean startServiceManagerOnly, final StartupCallback callback) {
- assert false; // Browser tests do a sync startup.
- }
+ NativeBrowserTest.deletePrivateDataDirectory(getPrivateDataDirectory());
- @Override
- public void startBrowserProcessesSync(boolean singleProcess) {
- ThreadUtils.assertOnUiThread();
- mStartupCompleted = true;
- // Runs the stuff that BrowserStartupController wants to do, without actually
- // running a chrome process.
- BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
- .initChromiumBrowserProcessForTests();
- }
-
- @Override
- public boolean isStartupSuccessfullyCompleted() {
- ThreadUtils.assertOnUiThread();
- // Technically C++ code should call through and set this after starting the browser
- // process but it will have done so by the time it goes through
- // handlePostNativeStartupSynchronously() which will call
- // startBrowserProcessesSync() in this class.
- return mStartupCompleted;
- }
-
- @Override
- public boolean isServiceManagerSuccessfullyStarted() {
- ThreadUtils.assertOnUiThread();
- // Technically C++ code should call through and set this after starting the service
- // manager but it will have done so by the time it goes through
- // handlePostNativeStartupSynchronously() which will call
- // startBrowserProcessesSync() in this class.
- return mStartupCompleted;
- }
-
- @Override
- public void addStartupCompletedObserver(StartupCallback callback) {
- ThreadUtils.assertOnUiThread();
- // Pass the callback through to the "real" BrowserStartupController because many
- // pieces of code will do the same, and we want them all in one place. The
- // initChromiumBrowserProcessForTests() will run them all so that none are missed.
- BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
- .addStartupCompletedObserver(callback);
- }
-
- @Override
- public void initChromiumBrowserProcessForTests() {
- assert false;
- }
-
- @Override
- public void setContentMainCallbackForTests(Runnable completionCallback) {
- assert false;
- }
-
- private boolean mStartupCompleted;
- };
- ChromeBrowserInitializer.setBrowserStartupControllerForTesting(startupController);
-
- // This does the pre-native stuff before handing control to C++ with runTests. Then
- // C++ will call back through handlePostNativeStartupSynchronously() to do the
- // post-native startup in ChromeBrowserInitializer.
- final BrowserParts parts = new EmptyBrowserParts() {};
- ChromeBrowserInitializer.getInstance().handlePreNativeStartup(parts);
-
- Window wind = this.getWindow();
- wind.addFlags(WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD);
- wind.addFlags(WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED);
- wind.addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON);
-
- // TODO(danakj): Make startup async and inherit from ChromeTabbedActivity.
- NativeBrowserTest.javaStartupTasksComplete();
- runTests();
+ // Replace ContentMain() with running our NativeTest suite.
+ BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
+ .setContentMainCallbackForTests(() -> {
+ // This jumps into C++ to set up and run the test harness. The test harness runs
+ // ContentMain()-equivalent code, and then waits for javaStartupTasksComplete()
+ // to be called. We delay that until finishNativeInitialization() is done which
+ // marks the end of the startup tasks posted from C++ in ContentMain() and then
+ // by Java in BrowserStartupControllerImpl::browserStartupComplete().
+ mTest.postStart(this, false);
+ });
}
+ /**
+ * Tests should not go through the first run process every time.
+ */
@Override
- protected File getPrivateDataDirectory() {
+ protected boolean requiresFirstRunToBeCompleted(Intent intent) {
+ return false;
+ }
+
+ /**
+ * This is the point at which Java initialization tasks are done and tests can be run.
+ * While mTest.postStart() runs the test harness, it waits for Java initialization
+ * tasks, and this signals that they are done.
+ */
+ @Override
+ public void finishNativeInitialization() {
+ super.finishNativeInitialization();
+ NativeBrowserTest.javaStartupTasksComplete();
+ }
+
+ private File getPrivateDataDirectory() {
// TODO(agrieve): We should not be touching the side-loaded test data directory.
// https://crbug.com/617734
return new File(UrlUtils.getIsolatedTestRoot(),
diff --git a/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsApplication.java b/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsApplication.java
index 0c8bb59..8cde625 100644
--- a/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsApplication.java
+++ b/chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsApplication.java
@@ -12,11 +12,6 @@
/**
* A basic chrome.browser.tests {@link android.app.Application}.
- *
- * TODO(danakj): This class sets up some of the things that ChromeApplication
- * does but we should maybe subclass ChromeApplication or share code. There are
- * many things missing from attachBaseContext() that ChromeApplication sets up
- * and it's not clear yet if they are needed or not.
*/
public class ChromeBrowserTestsApplication extends NativeBrowserTestApplication {
static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "chrome";
diff --git a/chrome/test/base/android/android_browser_test.cc b/chrome/test/base/android/android_browser_test.cc
index 08882b5..61b67c7 100644
--- a/chrome/test/base/android/android_browser_test.cc
+++ b/chrome/test/base/android/android_browser_test.cc
@@ -4,7 +4,8 @@
#include "chrome/test/base/android/android_browser_test.h"
-#include "chrome/browser/android/startup_bridge.h"
+#include "chrome/browser/ui/android/tab_model/tab_model.h"
+#include "chrome/browser/ui/android/tab_model/tab_model_list.h"
#include "chrome/test/base/test_launcher_utils.h"
#include "content/public/test/test_utils.h"
@@ -27,16 +28,14 @@
}
void AndroidBrowserTest::PreRunTestOnMainThread() {
- android_startup::HandlePostNativeStartupSynchronously();
-
- // Pump startup related events.
- content::RunAllPendingInMessageLoop();
}
void AndroidBrowserTest::PostRunTestOnMainThread() {
- // Sometimes tests leave Quit tasks in the MessageLoop (for shame), so let's
- // run all pending messages here to avoid preempting the QuitBrowsers tasks.
- // TODO(https://crbug.com/922118): Remove this once it is no longer possible
- // to post QuitCurrent* tasks.
+ for (size_t i = 0; i < TabModelList::size(); ++i) {
+ while (TabModelList::get(i)->GetTabCount())
+ TabModelList::get(i)->CloseTabAt(0);
+ }
+
+ // Run any shutdown events from closing tabs.
content::RunAllPendingInMessageLoop();
}
diff --git a/chrome/test/base/android/android_browser_test_browsertest_android.cc b/chrome/test/base/android/android_browser_test_browsertest_android.cc
index cf2b90f..51d6ad9 100644
--- a/chrome/test/base/android/android_browser_test_browsertest_android.cc
+++ b/chrome/test/base/android/android_browser_test_browsertest_android.cc
@@ -2,7 +2,17 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "chrome/browser/ui/android/tab_model/tab_model.h"
+#include "chrome/browser/ui/android/tab_model/tab_model_list.h"
#include "chrome/test/base/android/android_browser_test.h"
#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
-IN_PROC_BROWSER_TEST_F(AndroidBrowserTest, Smoke) {}
+IN_PROC_BROWSER_TEST_F(AndroidBrowserTest, Smoke) {
+ ASSERT_EQ(TabModelList::size(), 1u);
+
+ // Grab a tab an navigate its contents
+ TabModel* tab_model = TabModelList::get(0);
+ EXPECT_TRUE(content::NavigateToURL(tab_model->GetActiveWebContents(),
+ GURL("about:blank")));
+}
diff --git a/content/public/android/java/src/org/chromium/content/browser/BrowserStartupControllerImpl.java b/content/public/android/java/src/org/chromium/content/browser/BrowserStartupControllerImpl.java
index 7a219cb..c0fc4d1 100644
--- a/content/public/android/java/src/org/chromium/content/browser/BrowserStartupControllerImpl.java
+++ b/content/public/android/java/src/org/chromium/content/browser/BrowserStartupControllerImpl.java
@@ -463,25 +463,6 @@
ServicificationStartupUma.getInstance().commit();
}
- /**
- * Initialization needed for tests. Mainly used by content browsertests.
- */
- @Override
- public void initChromiumBrowserProcessForTests() {
- ResourceExtractor resourceExtractor = ResourceExtractor.get();
- resourceExtractor.setResultTraits(UiThreadTaskTraits.BOOTSTRAP);
- resourceExtractor.startExtractingResources("en");
- resourceExtractor.waitForCompletion();
- nativeSetCommandLineFlags(false);
-
- mFullBrowserStartupDone = true;
- mStartupSuccess = true;
- for (StartupCallback asyncStartupCallback : mAsyncStartupCallbacks) {
- asyncStartupCallback.onSuccess();
- }
- mAsyncStartupCallbacks.clear();
- }
-
private static native void nativeSetCommandLineFlags(boolean singleProcess);
// Is this an official build of Chrome? Only native code knows for sure. Official build
diff --git a/content/public/android/java/src/org/chromium/content_public/browser/BrowserStartupController.java b/content/public/android/java/src/org/chromium/content_public/browser/BrowserStartupController.java
index be2bc04..1b130af 100644
--- a/content/public/android/java/src/org/chromium/content_public/browser/BrowserStartupController.java
+++ b/content/public/android/java/src/org/chromium/content_public/browser/BrowserStartupController.java
@@ -83,11 +83,6 @@
void addStartupCompletedObserver(StartupCallback callback);
/**
- * Initialization needed for tests. Mainly used by browsertests.
- */
- void initChromiumBrowserProcessForTests();
-
- /**
* Set a callback that will be run in place of calling ContentMain(). For tests to
* define their own way of initializing the C++ system.
*/
