Move ContentMain() replacement code to BrowserTestBase.

BrowserTestBase decides to run ContentMain() or BrowserMain()
based on the OS. On Android it uses BrowserMain() instead. When
using BrowserMain() on Android, the test harness reimplements
some of ContentMain() before calling BrowserMain(). This is done
in the ContentBrowserTestSuite currently, but that can not be
shared with chrome browser tests, which inherit
ContentTestSuiteBase but not ContentBrowserTestSuite. To promote
code sharing and move the logic to the same place where the
ContentMain() decision is made, we move this setup code over
to BrowserTestBase.

While doing so, we update the code to more closely match what
ContentMain() does. This makes Android existing browser test
suites act more like desktop browser tests, which go through
ContentMain() already:
- Instead of hardcoding using ShellContentClient, we use the
ContentMainDelegate to create the ContentClient (which will be
a ShellContentClient in existing cases, but will differ in
chrome browser tests).
- Ordering is adjusted to match.
- The ContentMainDelegate is used and called throughout the
setup process the same way that ContentMain() would, including
calls to ShouldCreateFeatureList(), PostFieldTrialInitialization(),
PreCreateMainMessageLoop(), PostEarlyInitialization(),
PostTaskSchedulerStart(), and BasicStartupComplete().

Using the ContentMainDelegate doesn't change existing behaviour for
the most part though, as the ShellMainDelegate overrides largely set
up for web test mode, which browser tests do not run in (content shell
is run separately for that).

The InitMessagePumpForUIFactory() call from ContentBrowserTestSuite
must be made before the test suite is initialized, however, so it can
not move to BrowserTestBase. Instead we put it in the JNI_Onload method
of each browser test target. This ensures it is run first, and we
DCHECK that it succeeded.

In single process mode, we also must initialize a ContentRendererClient
which was being done in ContentBrowserTest, so moved this up to
the BrowserTestBase.

R=avi@chromium.org

Bug: 961849
Change-Id: Iefc105e3dc9f142774331aa97b0f1006bd701c0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1606424
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659128}

components/test/android/browsertests_apk/components_browser_tests_jni_onload.cc

@@ -2,11 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
 #include "base/android/jni_android.h"
 #include "base/android/library_loader/library_loader_hooks.h"
 #include "base/bind.h"
+#include "base/message_loop/message_loop.h"
 #include "content/public/app/content_jni_onload.h"
 #include "content/public/app/content_main.h"
+#include "content/public/test/nested_message_pump_android.h"
 #include "content/shell/app/shell_main_delegate.h"
 #include "testing/android/native_test/native_test_launcher.h"
 
@@ -15,6 +19,17 @@ JNI_EXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) {
   base::android::InitVM(vm);
   if (!content::android::OnJNIOnLoadInit())
     return -1;
+
+  // This needs to be done before base::TestSuite::Initialize() is called,
+  // as it also tries to set MessagePumpForUIFactory.
+  bool success = base::MessageLoop::InitMessagePumpForUIFactory(
+      []() -> std::unique_ptr<base::MessagePump> {
+        return std::make_unique<content::NestedMessagePumpAndroid>();
+      });
+  // If this fails, MessagePumpForUIFactory is already set, and we're unable to
+  // override.
+  DCHECK(success);
+
   content::SetContentMainDelegate(new content::ShellMainDelegate());
   return JNI_VERSION_1_4;
 }

content/app/android/content_main.cc

@@ -57,4 +57,9 @@ void SetContentMainDelegate(ContentMainDelegate* delegate) {
   g_content_main_delegate.Get().reset(delegate);
 }
 
+ContentMainDelegate* GetContentMainDelegateForTesting() {
+  DCHECK(g_content_main_delegate.Get().get());
+  return g_content_main_delegate.Get().get();
+}
+
 }  // namespace content

content/public/app/content_main.h

@@ -70,6 +70,11 @@ struct ContentMainParams {
 // This should only be called once before ContentMainRunner actually running.
 // The ownership of |delegate| is transferred.
 CONTENT_EXPORT void SetContentMainDelegate(ContentMainDelegate* delegate);
+
+// In browser tests, ContentMain.java is not run either, and the browser test
+// harness does not run ContentMain() at all. It does need to make use of the
+// delegate though while replacing ContentMain().
+CONTENT_EXPORT ContentMainDelegate* GetContentMainDelegateForTesting();
 #else
 // ContentMain should be called from the embedder's main() function to do the
 // initial setup for every process. The embedder has a chance to customize

content/public/app/content_main_delegate.h

@@ -140,6 +140,7 @@ class CONTENT_EXPORT ContentMainDelegate {
 
  protected:
   friend class ContentClientInitializer;
+  friend class BrowserTestBase;
 
   // Called once per relevant process type to allow the embedder to customize
   // content. If an embedder wants the default (empty) implementation, don't

content/public/test/browser_test_base.cc

@@ -58,6 +58,18 @@
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_switches.h"
 
+#if defined(OS_ANDROID)
+#include "content/app/mojo/mojo_init.h"
+#include "content/common/url_schemes.h"
+#include "content/public/app/content_main_delegate.h"
+#include "content/public/common/content_paths.h"
+#include "ui/base/ui_base_paths.h"
+
+#ifdef V8_USE_EXTERNAL_STARTUP_DATA
+#include "gin/v8_initializer.h"
+#endif
+#endif
+
 #if defined(OS_MACOSX)
 #include "ui/events/test/event_generator.h"
 #include "ui/views/test/event_generator_delegate_mac.h"
@@ -330,17 +342,65 @@ void BrowserTestBase::SetUp() {
           &BrowserTestBase::CreatedBrowserMainParts, base::Unretained(this)));
 
 #if defined(OS_ANDROID)
-  MainFunctionParams params(*command_line);
-  params.ui_task = ui_task.release();
-  params.created_main_parts_closure = created_main_parts_closure.release();
-  base::ThreadPool::Create("Browser");
-  DCHECK(!field_trial_list_);
-  field_trial_list_ = SetUpFieldTrialsAndFeatureList();
-  StartBrowserThreadPool();
-  BrowserTaskExecutor::Create();
-  BrowserTaskExecutor::PostFeatureListSetup();
-  // TODO(phajdan.jr): Check return code, http://crbug.com/374738 .
-  BrowserMain(params);
+  // For all other platforms, we call ContentMain for browser tests which goes
+  // through the normal browser initialization paths. For Android, we must set
+  // things up manually. A meager re-implementation of ContentMainRunnerImpl
+  // follows.
+
+  base::i18n::AllowMultipleInitializeCallsForTesting();
+  base::i18n::InitializeICU();
+
+#ifdef V8_USE_EXTERNAL_STARTUP_DATA
+  gin::V8Initializer::LoadV8Snapshot();
+  gin::V8Initializer::LoadV8Natives();
+#endif
+
+  ContentMainDelegate* delegate = GetContentMainDelegateForTesting();
+  // The delegate should have been set by JNI_OnLoad for the test target.
+  DCHECK(delegate);
+
+  bool startup_error = delegate->BasicStartupComplete(/*exit_code=*/nullptr);
+  DCHECK(!startup_error);
+
+  InitializeMojo();
+
+  {
+    SetBrowserClientForTesting(delegate->CreateContentBrowserClient());
+    if (command_line->HasSwitch(switches::kSingleProcess))
+      SetRendererClientForTesting(delegate->CreateContentRendererClient());
+
+    content::RegisterPathProvider();
+    content::RegisterContentSchemes(false);
+    ui::RegisterPathProvider();
+
+    delegate->PreSandboxStartup();
+
+    DCHECK(!field_trial_list_);
+    if (delegate->ShouldCreateFeatureList()) {
+      field_trial_list_ = SetUpFieldTrialsAndFeatureList();
+      delegate->PostFieldTrialInitialization();
+    }
+
+    base::ThreadPool::Create("Browser");
+
+    delegate->PreCreateMainMessageLoop();
+    BrowserTaskExecutor::Create();
+    delegate->PostEarlyInitialization(/*is_running_tests=*/true);
+
+    StartBrowserThreadPool();
+    BrowserTaskExecutor::PostFeatureListSetup();
+    delegate->PostTaskSchedulerStart();
+  }
+
+  // Run BrowserMain which ContentMain would normally run.
+  {
+    MainFunctionParams params(*command_line);
+    params.ui_task = ui_task.release();
+    params.created_main_parts_closure = created_main_parts_closure.release();
+    int exit_code = BrowserMain(params);
+    DCHECK_EQ(exit_code, 0);
+  }
+
   BrowserTaskExecutor::ResetForTesting();
 #else
   GetContentMainParams()->ui_task = ui_task.release();

content/public/test/content_browser_test.cc

@@ -22,10 +22,6 @@
 #include "content/test/test_content_client.h"
 #include "ui/events/platform/platform_event_source.h"
 
-#if defined(OS_ANDROID)
-#include "content/shell/app/shell_main_delegate.h"
-#endif
-
 #if defined(OS_MACOSX)
 #include "base/mac/foundation_util.h"
 #endif
@@ -66,21 +62,7 @@ void ContentBrowserTest::SetUp() {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   SetUpCommandLine(command_line);
 
-#if defined(OS_ANDROID)
-  shell_main_delegate_.reset(new ShellMainDelegate);
-  shell_main_delegate_->PreSandboxStartup();
-  if (command_line->HasSwitch(switches::kSingleProcess)) {
-    // We explicitly leak the new ContentRendererClient as we're
-    // setting a global that may be used after ContentBrowserTest is
-    // destroyed.
-    ContentRendererClient* old_client =
-        switches::IsRunWebTestsSwitchPresent()
-            ? SetRendererClientForTesting(new WebTestContentRendererClient)
-            : SetRendererClientForTesting(new ShellContentRendererClient);
-    // No-one should have set this value before we did.
-    DCHECK(!old_client);
-  }
-#elif defined(OS_MACOSX)
+#if defined(OS_MACOSX)
   // See InProcessBrowserTest::PrepareTestCommandLine().
   base::FilePath subprocess_path;
   base::PathService::Get(base::FILE_EXE, &subprocess_path);
@@ -116,10 +98,6 @@ void ContentBrowserTest::TearDown() {
 #if !defined(OS_CHROMEOS) && defined(OS_LINUX)
   ui::ShutdownInputMethodForTesting();
 #endif
-
-#if defined(OS_ANDROID)
-  shell_main_delegate_.reset();
-#endif
 }
 
 void ContentBrowserTest::PreRunTestOnMainThread() {

content/public/test/content_browser_test.h

@@ -18,7 +18,6 @@
 
 namespace content {
 class Shell;
-class ShellMainDelegate;
 
 // Base class for browser tests which use content_shell.
 class ContentBrowserTest : public BrowserTestBase {
@@ -61,12 +60,6 @@ class ContentBrowserTest : public BrowserTestBase {
   base::mac::ScopedNSAutoreleasePool* pool_ = nullptr;
 #endif
 
-#if defined(OS_ANDROID)
-  // For all other platforms, this is done automatically when calling into
-  // ContentMain. For Android we set things up manually.
-  std::unique_ptr<ShellMainDelegate> shell_main_delegate_;
-#endif
-
   // Used to detect incorrect overriding of PreRunTestOnMainThread() with
   // missung call to base implementation.
   bool pre_run_test_executed_ = false;

content/shell/android/browsertests_apk/content_browser_tests_jni_onload.cc

@@ -2,11 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
 #include "base/android/jni_android.h"
 #include "base/android/library_loader/library_loader_hooks.h"
 #include "base/bind.h"
+#include "base/message_loop/message_loop.h"
 #include "content/public/app/content_jni_onload.h"
 #include "content/public/app/content_main.h"
+#include "content/public/test/nested_message_pump_android.h"
 #include "content/shell/app/shell_main_delegate.h"
 #include "testing/android/native_test/native_test_launcher.h"
 
@@ -15,6 +19,17 @@ JNI_EXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) {
   base::android::InitVM(vm);
   if (!content::android::OnJNIOnLoadInit())
     return -1;
+
+  // This needs to be done before base::TestSuite::Initialize() is called,
+  // as it also tries to set MessagePumpForUIFactory.
+  bool success = base::MessageLoop::InitMessagePumpForUIFactory(
+      []() -> std::unique_ptr<base::MessagePump> {
+        return std::make_unique<content::NestedMessagePumpAndroid>();
+      });
+  // If this fails, MessagePumpForUIFactory is already set, and we're unable to
+  // override.
+  DCHECK(success);
+
   content::SetContentMainDelegate(new content::ShellMainDelegate(true));
   return JNI_VERSION_1_4;
 }

content/test/content_test_launcher.cc

@@ -24,29 +24,8 @@
 #include "ui/base/buildflags.h"
 #include "ui/base/ui_base_switches.h"
 
-#ifdef V8_USE_EXTERNAL_STARTUP_DATA
-#include "gin/v8_initializer.h"
-#endif
-
-#if defined(OS_ANDROID)
-#include "base/message_loop/message_loop.h"
-#include "content/app/mojo/mojo_init.h"
-#include "content/common/url_schemes.h"
-#include "content/public/common/content_paths.h"
-#include "content/public/test/nested_message_pump_android.h"
-#include "content/shell/browser/shell_content_browser_client.h"
-#include "content/shell/common/shell_content_client.h"
-#include "ui/base/ui_base_paths.h"
-#endif
-
 namespace content {
 
-#if defined(OS_ANDROID)
-std::unique_ptr<base::MessagePump> CreateMessagePumpForUI() {
-  return std::unique_ptr<base::MessagePump>(new NestedMessagePumpAndroid());
-}
-#endif
-
 class ContentBrowserTestSuite : public ContentTestSuiteBase {
  public:
   ContentBrowserTestSuite(int argc, char** argv)
@@ -56,47 +35,16 @@ class ContentBrowserTestSuite : public ContentTestSuiteBase {
 
  protected:
   void Initialize() override {
-#if defined(OS_ANDROID)
-    base::i18n::AllowMultipleInitializeCallsForTesting();
-    base::i18n::InitializeICU();
-
-#ifdef V8_USE_EXTERNAL_STARTUP_DATA
-    gin::V8Initializer::LoadV8Snapshot();
-    gin::V8Initializer::LoadV8Natives();
-#endif
-
-    // This needs to be done before base::TestSuite::Initialize() is called,
-    // as it also tries to set MessagePumpForUIFactory.
-    if (!base::MessageLoop::InitMessagePumpForUIFactory(
-            &CreateMessagePumpForUI))
-      VLOG(0) << "MessagePumpForUIFactory already set, unable to override.";
-
-    // For all other platforms, we call ContentMain for browser tests which goes
-    // through the normal browser initialization paths. For Android, we must set
-    // things up manually.
-    content_client_.reset(new ShellContentClient);
-    browser_content_client_.reset(new ShellContentBrowserClient());
-    SetContentClient(content_client_.get());
-    SetBrowserClientForTesting(browser_content_client_.get());
-
-    content::RegisterContentSchemes(false);
-    RegisterPathProvider();
-    ui::RegisterPathProvider();
-    RegisterInProcessThreads();
-
-    InitializeMojo();
-#endif
-
-    // Browser tests are expected not to tear-down various globals.
+    // Browser tests are expected not to tear-down various globals. (Must run
+    // before the base class is initialized.)
     base::TestSuite::DisableCheckForLeakedGlobals();
 
     ContentTestSuiteBase::Initialize();
-  }
 
 #if defined(OS_ANDROID)
-  std::unique_ptr<ShellContentClient> content_client_;
-  std::unique_ptr<ShellContentBrowserClient> browser_content_client_;
+    RegisterInProcessThreads();
 #endif
+  }
 
   DISALLOW_COPY_AND_ASSIGN(ContentBrowserTestSuite);
 };