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/+/1672197Reviewed-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}

chrome/android/java/src/org/chromium/chrome/browser/init/NativeStartupBridge.java

@@ -39,16 +39,4 @@ public class NativeStartupBridge {
             }
         });
     }
-
-    @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);
-        }
-    }
 }

chrome/android/junit/src/org/chromium/chrome/browser/background_task_scheduler/NativeBackgroundTaskTest.java

@@ -87,8 +87,6 @@ public class NativeBackgroundTaskTest {
         @Override
         public void addStartupCompletedObserver(StartupCallback callback) {}
 
-        @Override
-        public void initChromiumBrowserProcessForTests() {}
         @Override
         public void setContentMainCallbackForTests(Runnable r) {}
 

chrome/browser/android/startup_bridge.cc

@@ -22,11 +22,4 @@ void LoadFullBrowser() {
   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

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_

chrome/test/BUILD.gn

@@ -471,6 +471,7 @@ if (is_android) {
       ":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 @@ if (is_android) {
     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",

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. -->

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.
-            }
-
-            @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);
+        NativeBrowserTest.deletePrivateDataDirectory(getPrivateDataDirectory());
+
+        // 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);
+                });
+    }
 
-        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);
+    /**
+     * Tests should not go through the first run process every time.
+     */
+    @Override
+    protected boolean requiresFirstRunToBeCompleted(Intent intent) {
+        return false;
+    }
 
-        // TODO(danakj): Make startup async and inherit from ChromeTabbedActivity.
+    /**
+     * 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();
-        runTests();
     }
 
-    @Override
-    protected File getPrivateDataDirectory() {
+    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(),

chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsApplication.java

@@ -12,11 +12,6 @@ import org.chromium.native_test.NativeBrowserTestApplication;
 
 /**
  * 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";

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::SetUpDefaultCommandLine(
 }
 
 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();
 }

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")));
+}

content/public/android/java/src/org/chromium/content/browser/BrowserStartupControllerImpl.java

@@ -463,25 +463,6 @@ public class BrowserStartupControllerImpl implements BrowserStartupController {
         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

content/public/android/java/src/org/chromium/content_public/browser/BrowserStartupController.java

@@ -82,11 +82,6 @@ public interface BrowserStartupController {
      */
     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.