Have mash_browser_tests recreate BackgroundServiceManager per test

Currently one BackgroundServiceManager is created per batch of browser_tests.
This ends up leaving the UI and Ash process running between tests.

This is especially problematic for tests with PRE_test where state is expected
to not be preserved into the following test.

This change delays the initialization of a BackgroundServiceManager until the
child test process has connected to the MojoTestState. Subsequent tests will
tear down the previous BackgroundServiceManager before creating the new one.

This also tears down the UI and Ash processes as desired.

TEST=mash_browser_tests
BUG=678687

Review-Url: https://codereview.chromium.org/2858103002
Cr-Commit-Position: refs/heads/master@{#473897}

chrome/test/base/chrome_test_launcher.cc

@@ -30,6 +30,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/test/test_launcher.h"
 #include "content/public/test/test_utils.h"
+#include "services/service_manager/runner/common/switches.h"
 #include "ui/base/test/ui_controls.h"
 
 #if defined(OS_MACOSX)
@@ -63,6 +64,12 @@ ChromeTestSuiteRunner::ChromeTestSuiteRunner() {}
 ChromeTestSuiteRunner::~ChromeTestSuiteRunner() {}
 
 int ChromeTestSuiteRunner::RunTestSuite(int argc, char** argv) {
+#if defined(USE_AURA)
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(service_manager::switches::kServicePipeToken))
+    content::GetContentMainParams()->env_mode = aura::Env::Mode::MUS;
+#endif  // defined(USE_AURA)
+
   return ChromeTestSuite(argc, argv).Run();
 }
 

chrome/test/base/mash_browser_tests_main.cc

@@ -153,9 +153,11 @@ class MashTestLauncherDelegate : public ChromeTestLauncherDelegate {
 
 std::unique_ptr<content::ServiceManagerConnection>
     CreateServiceManagerConnection(MashTestLauncherDelegate* delegate) {
+  delegate->GetMojoTestConnectorForSingleProcess()->Init();
   std::unique_ptr<content::ServiceManagerConnection> connection(
       content::ServiceManagerConnection::Create(
-          delegate->GetMojoTestConnectorForSingleProcess()->Init(),
+          delegate->GetMojoTestConnectorForSingleProcess()
+              ->InitBackgroundServiceManager(),
           base::ThreadTaskRunnerHandle::Get()));
   connection->Start();
   connection->GetConnector()->StartService(mash::session::mojom::kServiceName);

chrome/test/base/mojo_test_connector.cc

@@ -18,6 +18,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/service_names.mojom.h"
 #include "content/public/test/test_launcher.h"
+#include "mash/session/public/interfaces/constants.mojom.h"
 #include "mojo/edk/embedder/embedder.h"
 #include "mojo/edk/embedder/outgoing_broker_client_invitation.h"
 #include "mojo/edk/embedder/platform_channel_pair.h"
@@ -43,13 +44,12 @@ const char kMusTestRunnerName[] = "mus_browser_tests";
 // service manager.
 class MojoTestState : public content::TestState {
  public:
-  explicit MojoTestState(
-      service_manager::BackgroundServiceManager* background_service_manager,
-      base::CommandLine* command_line,
-      base::TestLauncher::LaunchOptions* test_launch_options,
-      const std::string& mus_config_switch,
-      base::OnceClosure on_process_launched)
-      : background_service_manager_(background_service_manager),
+  MojoTestState(MojoTestConnector* connector,
+                base::CommandLine* command_line,
+                base::TestLauncher::LaunchOptions* test_launch_options,
+                const std::string& mus_config_switch,
+                base::OnceClosure on_process_launched)
+      : connector_(connector),
         platform_channel_(base::MakeUnique<mojo::edk::PlatformChannelPair>()),
         main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
         on_process_launched_(std::move(on_process_launched)),
@@ -99,7 +99,10 @@ class MojoTestState : public content::TestState {
   // after ChildProcessLaunched as previous test runs will tear down existing
   // connections.
   void SetupService(base::ProcessId pid) {
-    background_service_manager_->RegisterService(
+    connector_->InitBackgroundServiceManager();
+    service_manager::BackgroundServiceManager* background_service_manager =
+        connector_->background_service_manager();
+    background_service_manager->RegisterService(
         service_manager::Identity(content::mojom::kPackagedServicesServiceName,
                                   service_manager::mojom::kRootUserID),
         std::move(service_), mojo::MakeRequest(&pid_receiver_));
@@ -112,7 +115,7 @@ class MojoTestState : public content::TestState {
   }
 
   mojo::edk::OutgoingBrokerClientInvitation broker_client_invitation_;
-  service_manager::BackgroundServiceManager* const background_service_manager_;
+  MojoTestConnector* connector_;
 
   // The ServicePtr must be created before child process launch so that the pipe
   // can be set on the command line. It is held until SetupService is called at
@@ -200,7 +203,7 @@ MojoTestConnector::MojoTestConnector(
       background_service_manager_(nullptr),
       catalog_contents_(std::move(catalog_contents)) {}
 
-service_manager::mojom::ServiceRequest MojoTestConnector::Init() {
+void MojoTestConnector::Init() {
   // In single-process test mode, browser code will initialize the EDK and IPC.
   // Otherwise we ensure it's initialized here.
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -213,7 +216,10 @@ service_manager::mojom::ServiceRequest MojoTestConnector::Init() {
         ipc_thread_->task_runner(),
         mojo::edk::ScopedIPCSupport::ShutdownPolicy::FAST);
   }
+}
 
+service_manager::mojom::ServiceRequest
+MojoTestConnector::InitBackgroundServiceManager() {
   service_manager::mojom::ServicePtr service;
   auto request = mojo::MakeRequest(&service);
 
@@ -223,7 +229,7 @@ service_manager::mojom::ServiceRequest MojoTestConnector::Init() {
   background_service_manager_ =
       base::MakeUnique<service_manager::BackgroundServiceManager>(
           service_process_launcher_delegate_.get(),
-          std::move(catalog_contents_));
+          catalog_contents_->CreateDeepCopy());
   background_service_manager_->RegisterService(
       service_manager::Identity(config_ == MojoTestConnector::Config::MASH
                                     ? kMashTestRunnerName
@@ -240,7 +246,7 @@ std::unique_ptr<content::TestState> MojoTestConnector::PrepareForTest(
     base::TestLauncher::LaunchOptions* test_launch_options,
     base::OnceClosure on_process_launched) {
   return base::MakeUnique<MojoTestState>(
-      background_service_manager_.get(), command_line, test_launch_options,
+      this, command_line, test_launch_options,
       config_ == MojoTestConnector::Config::MASH ? switches::kMash
                                                  : switches::kMus,
       std::move(on_process_launched));

chrome/test/base/mojo_test_connector.h

@@ -57,8 +57,15 @@ class MojoTestConnector {
                     Config config);
   ~MojoTestConnector();
 
+  service_manager::BackgroundServiceManager* background_service_manager() {
+    return background_service_manager_.get();
+  }
+
+  // Initializes the Mojo environment, and IPC thread.
+  void Init();
+
   // Initializes the background thread the ServiceManager runs on.
-  service_manager::mojom::ServiceRequest Init();
+  service_manager::mojom::ServiceRequest InitBackgroundServiceManager();
 
   std::unique_ptr<content::TestState> PrepareForTest(
       base::CommandLine* command_line,

services/ui/service.cc

@@ -100,9 +100,9 @@ Service::~Service() {
 #endif
 }
 
-void Service::InitializeResources(service_manager::Connector* connector) {
+bool Service::InitializeResources(service_manager::Connector* connector) {
   if (ui::ResourceBundle::HasSharedInstance())
-    return;
+    return true;
 
   std::set<std::string> resource_paths;
   resource_paths.insert(kResourceFileStrings);
@@ -112,7 +112,10 @@ void Service::InitializeResources(service_manager::Connector* connector) {
   catalog::ResourceLoader loader;
   filesystem::mojom::DirectoryPtr directory;
   connector->BindInterface(catalog::mojom::kServiceName, &directory);
-  CHECK(loader.OpenFiles(std::move(directory), resource_paths));
+  if (!loader.OpenFiles(std::move(directory), resource_paths)) {
+    LOG(ERROR) << "Service failed to open resource files.";
+    return false;
+  }
 
   ui::RegisterPathProvider();
 
@@ -125,6 +128,7 @@ void Service::InitializeResources(service_manager::Connector* connector) {
                          ui::SCALE_FACTOR_100P);
   rb.AddDataPackFromFile(loader.TakeFile(kResourceFile200),
                          ui::SCALE_FACTOR_200P);
+  return true;
 }
 
 Service::UserState* Service::GetUserState(
@@ -156,7 +160,11 @@ void Service::OnStart() {
   if (test_config_)
     ui::test::EnableTestConfigForPlatformWindows();
 
-  InitializeResources(context()->connector());
+  // If resources are unavailable do not complete start-up.
+  if (!InitializeResources(context()->connector())) {
+    context()->QuitNow();
+    return;
+  }
 
 #if defined(USE_OZONE)
   // The ozone platform can provide its own event source. So initialize the

services/ui/service.h

@@ -87,7 +87,9 @@ class Service : public service_manager::Service,
 
   using UserIdToUserState = std::map<ws::UserId, std::unique_ptr<UserState>>;
 
-  void InitializeResources(service_manager::Connector* connector);
+  // Attempts to initialize the resource bundle. Returns true if successful,
+  // otherwise false if resources cannot be loaded.
+  bool InitializeResources(service_manager::Connector* connector);
 
   // Returns the user specific state for the user id of |remote_identity|.
   // Service owns the return value.

testing/buildbot/chromium.chromiumos.json

@@ -239,6 +239,18 @@
         },
         "test": "latency_unittests"
       },
+      {
+        "args": [
+          "--ozone-platform=headless",
+          "--override-use-software-gl-for-tests",
+          "--test-launcher-filter-file=../../testing/buildbot/filters/mash.browser_tests.filter"
+        ],
+        "swarming": {
+          "can_use_on_swarming_builders": true,
+          "hard_timeout": 900
+        },
+        "test": "mash_browser_tests"
+      },
       {
         "swarming": {
           "can_use_on_swarming_builders": true

testing/buildbot/filters/mash.browser_tests.filter

-BrowserTest.*
 # Failing test
 -BrowserTest.FullscreenBookmarkBar
 
@@ -15,25 +14,7 @@ BrowserTest.*
 -ExtensionsLoadTestWithLoginScreenApps.CommandLineExtensionsDontLoad
 
 # Trying to whitelist
-AppWindowApiTest.AlphaEnabledHasPermissions
-AppWindowApiTest.AlphaEnabledInStable
-AppWindowApiTest.AlphaEnabledNoPermissions
-AppWindowApiTest.AlphaEnabledWrongFrameType
-AppWindowApiTest.AlwaysOnTopNoPermissions
-AppWindowApiTest.AlwaysOnTopWithOldPermissions
-AppWindowApiTest.AlwaysOnTopWithPermissions
-AppWindowApiTest.Get
-AppWindowApiTest.ImeWindowHasPermissions
-AppWindowApiTest.ImeWindowNoPermissions
-AppWindowApiTest.ImeWindowNotFullscreen
-AppWindowApiTest.SetShapeHasPerm
-AppWindowApiTest.SetShapeNoPerm
-AppWindowApiTest.VisibleOnAllWorkspacesInStable
-BluetoothInternalsTest.Startup_BluetoothInternals
-BootstrapTest.CleanUpFailedUser
-BootstrapTest.PRE_CleanUpFailedUser
 BrowserDialogTest.Invoke
-BrowsingDataRemoverTransportSecurityStateBrowserTest.ClearTransportSecurityState
 CastSessionBrowserTest.CreateAndDestroy
 ChromeContentRendererClientSearchBoxTest.RewriteThumbnailURL
 ChromeRenderFrameObserverTest.SkipCapturingSubFrames
@@ -42,25 +23,10 @@ ChromeRenderViewTest.ImagesAllowedByDefault
 ChromeRenderViewTest.ImagesBlockedByDefault
 ChromeRenderViewTest.JSBlockSentAfterPageLoad
 ChromeRenderViewTest.PluginsTemporarilyAllowed
-ChromeServiceWorkerManifestFetchTest.OtherOrigin
-ChromeServiceWorkerManifestFetchTest.OtherOriginUseCredentials
-ChromeServiceWorkerManifestFetchTest.SameOrigin
-ChromeServiceWorkerManifestFetchTest.SameOriginUseCredentials
-ChromeServiceWorkerTest.CanCloseIncognitoWindowWithServiceWorkerController
-ChromeServiceWorkerTest.CanShutDownWithRegisteredServiceWorker
-ChromeServiceWorkerTest.FailRegisterServiceWorkerWhenJSDisabled
-ChromeServiceWorkerTest.FallbackMainResourceRequestWhenJSDisabled
-ConstrainedWebDialogBrowserTest.ReleaseWebContents
-ContentVerifierPolicyTest.PRE_PolicyCorruptedOnStartup
-ContentVerifierPolicyTest.PolicyCorruptedOnStartup
 CreateNewFolder/FileManagerBrowserTest.Test/0
 CreateNewFolder/FileManagerBrowserTest.Test/1
 CreateNewFolder/FileManagerBrowserTest.Test/2
 CreateNewFolder/FileManagerBrowserTest.Test/3
-DefaultTaskDialog/FileManagerBrowserTest.Test/0
-DefaultTaskDialog/FileManagerBrowserTest.Test/1
-DefaultTaskDialog/FileManagerBrowserTest.Test/2
-Delete/FileManagerBrowserTest.Test/1
 DeviceStatusCollectorNetworkInterfacesTest.NetworkInterfaces
 DeviceStatusCollectorNetworkInterfacesTest.ReportIfPublicSession
 DeviceStatusCollectorTest.ActivityCrossingMidnight
@@ -89,44 +55,8 @@ DeviceStatusCollectorTest.TestSoundVolume
 DeviceStatusCollectorTest.TestVolumeInfo
 DeviceStatusCollectorTest.Times
 DeviceStatusCollectorTest.VersionInfo
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/0
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/1
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/10
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/11
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/12
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/13
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/14
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/15
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/16
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/17
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/18
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/19
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/2
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/20
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/21
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/22
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/24
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/3
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/4
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/5
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/6
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/7
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/8
-DirectoryTreeContextMenu/FileManagerBrowserTest.Test/9
 DownloadInterruptReasonEnumsSynced.DownloadInterruptReasonEnumsSynced
-DriveSpecific/FileManagerBrowserTest.Test/0
-DriveSpecific/FileManagerBrowserTest.Test/1
-DriveSpecific/FileManagerBrowserTest.Test/2
-DriveSpecific/FileManagerBrowserTest.Test/3
-DriveSpecific/FileManagerBrowserTest.Test/4
-DriveSpecific/FileManagerBrowserTest.Test/5
-DriveSpecific/FileManagerBrowserTest.Test/6
-ExecuteDefaultTaskOnDownloads/FileManagerBrowserTest.Test/0
-ExecuteDefaultTaskOnDownloads/FileManagerBrowserTest.Test/1
-ExecuteDefaultTaskOnDrive/FileManagerBrowserTest.Test/0
 ExtensionDetermineDownloadFilenameInternal.ExtensionDetermineDownloadFilenameInternal
-FolderShortcuts/FileManagerBrowserTest.Test/0
-FolderShortcuts/FileManagerBrowserTest.Test/1
 FormAutocompleteTest.AjaxSucceeded_FilledFormIsInvisible
 FormAutocompleteTest.AjaxSucceeded_FilledFormStillVisible
 FormAutocompleteTest.AjaxSucceeded_FormlessElements
@@ -158,18 +88,6 @@ FormClassifierTest.SomeFormWithoutPasswordFields
 FormClassifierTest.kChangeFormWithTreePasswordFieldsHTML
 InstantProcessNavigationTest.ForkForNavigationsFromInstantProcess
 InstantProcessNavigationTest.ForkForNavigationsToSearchURLs
-MultiProfileFileManagerBrowserTest.BasicDownloads
-MultiProfileFileManagerBrowserTest.BasicDrive
-MultiProfileFileManagerBrowserTest.PRE_BasicDownloads
-OpenAudioFiles/FileManagerBrowserTest.Test/1
-OpenAudioFiles/FileManagerBrowserTest.Test/2
-OpenAudioFiles/FileManagerBrowserTest.Test/3
-OpenAudioFiles/FileManagerBrowserTest.Test/4
-OpenAudioFiles/FileManagerBrowserTest.Test/5
-OpenAudioFiles/FileManagerBrowserTest.Test/6
-OpenFileDialog/FileManagerBrowserTest.Test/0
-OpenFileDialog/FileManagerBrowserTest.Test/2
-OpenFileDialog/FileManagerBrowserTest.Test/5
 PageClickTrackerTest.PageClickTrackerClickDisabledInputDoesNotResetClickCounter
 PageClickTrackerTest.PageClickTrackerDisabledInputClickedNoEvent
 PageClickTrackerTest.PageClickTrackerInputClicked
@@ -219,13 +137,5 @@ PnaclHeaderTest.TestHasPnaclHeader
 PolicyCertVerifierTest.VerifyTrustedCert
 PolicyCertVerifierTest.VerifyUntrustedCert
 PolicyCertVerifierTest.VerifyUsingAdditionalTrustAnchor
-Providers/FileManagerBrowserTest.Test/0
-Providers/FileManagerBrowserTest.Test/1
-Providers/FileManagerBrowserTest.Test/2
-Providers/FileManagerBrowserTest.Test/3
-RestoreGeometry/FileManagerBrowserTest.Test/0
-RestoreGeometry/FileManagerBrowserTest.Test/1
 ScriptContextTest.GetEffectiveDocumentURL
-ShowGridView/FileManagerBrowserTest.Test/0
-ShowGridView/FileManagerBrowserTest.Test/1
 SiteEngagementUiBrowserTest.Basic