Ozone: Tests must respect UseOzonePlatform and OzonePlatform switches

There are some tests that create own cmdline that results in
missing kEnableFeatures and kOzonePlatform switches. As a consequence,
the browser parts start with a wrong config, which results in
failures and usage of the non-Ozone path for the use_x11 && use_ozone
config, and also in usage of the wrong OzonePlatform as it doesn't
know what platform the test has been launched with (linux-ozone-rel
testers pass headless/wayland/x11 to the cmdline, for example).

Solution is to copy all the previous switches.

Bug: 1115055
Change-Id: Ia7a1f2688b1d697073a96c6527145342deb13778
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346355
Commit-Queue: Maksim Sisov (GMT+3) <msisov@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799287}

apps/DEPS

@@ -30,5 +30,8 @@ specific_include_rules = {
     "+chrome/browser/extensions/api/file_system/file_system_api.h",
     # To access sandbox includes.
     "+sandbox/policy",
-  ]
+  ],
+ "load_and_launch_browsertest.cc": [
+  "+ui/ozone/public/ozone_switches.h"
+  ],
 }

apps/load_and_launch_browsertest.cc

@@ -7,6 +7,7 @@
 // and when chrome is started from scratch.
 
 #include "apps/switches.h"
+#include "base/base_switches.h"
 #include "base/process/launch.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -27,6 +28,10 @@
 #include "extensions/test/extension_test_message_listener.h"
 #include "sandbox/policy/switches.h"
 
+#if defined(USE_OZONE)
+#include "ui/ozone/public/ozone_switches.h"
+#endif
+
 using extensions::PlatformAppBrowserTest;
 
 namespace apps {
@@ -36,6 +41,19 @@ namespace {
 const char* kSwitchesToCopy[] = {
     sandbox::policy::switches::kNoSandbox,
     switches::kUserDataDir,
+#if defined(USE_OZONE)
+    // Keep the kOzonePlatform switch that the Ozone must use.
+    switches::kOzonePlatform,
+#endif
+    // Some tests use custom cmdline that doesn't hold switches from previous
+    // cmdline. Only a couple of switches are copied. That can result in
+    // incorrect initialization of a process. For example, the work that we do
+    // to have use_x11 && use_ozone, requires UseOzonePlatform feature flag to
+    // be passed to all the process to ensure correct path is chosen.
+    // TODO(https://crbug.com/1096425): update this comment once USE_X11 goes
+    // away.
+    switches::kEnableFeatures,
+    switches::kDisableFeatures,
 };
 
 constexpr char kTestExtensionId[] = "behllobkkfkfnphdnhnkndlbkcpglgmj";

chrome/test/BUILD.gn

@@ -1496,6 +1496,10 @@ if (!is_android) {
       deps += [ "//chrome/app:chrome_dll_resources" ]
     }
 
+    if (use_ozone) {
+      deps += [ "//ui/ozone" ]
+    }
+
     if (is_mac) {
       sources += [ "../browser/metrics/power_metrics_provider_mac_unittest.cc" ]
     }

content/browser/DEPS

@@ -167,4 +167,7 @@ specific_include_rules = {
     "+services/network/cookie_manager.h",
     "+third_party/leveldatabase",
   ],
+  "launch_as_mojo_client_browsertest.cc": [
+    "+ui/ozone/public/ozone_switches.h",
+  ],
 }

content/browser/launch_as_mojo_client_browsertest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/base_switches.h"
 #include "base/cfi_buildflags.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
@@ -22,6 +23,10 @@
 #include "mojo/public/mojom/base/binder.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(USE_OZONE)
+#include "ui/ozone/public/ozone_switches.h"
+#endif
+
 namespace content {
 namespace {
 
@@ -32,6 +37,22 @@ const char kShellExecutableName[] = "content_shell";
 const char kMojoCoreLibraryName[] = "libmojo_core.so";
 #endif
 
+const char* kSwitchesToCopy[] = {
+#if defined(USE_OZONE)
+    // Keep the kOzonePlatform switch that the Ozone must use.
+    switches::kOzonePlatform,
+#endif
+    // Some tests use custom cmdline that doesn't hold switches from previous
+    // cmdline. Only a couple of switches are copied. That can result in
+    // incorrect initialization of a process. For example, the work that we do
+    // to have use_x11 && use_ozone, requires UseOzonePlatform feature flag to
+    // be passed to all the process to ensure correct path is chosen.
+    // TODO(https://crbug.com/1096425): update this comment once USE_X11 goes
+    // away.
+    switches::kEnableFeatures,
+    switches::kDisableFeatures,
+};
+
 base::FilePath GetCurrentDirectory() {
   base::FilePath current_directory;
   CHECK(base::GetCurrentDirectory(&current_directory));
@@ -56,6 +77,9 @@ class LaunchAsMojoClientBrowserTest : public ContentBrowserTest {
         GetFilePathNextToCurrentExecutable(kShellExecutableName));
     command_line.AppendSwitchPath(switches::kContentShellDataPath,
                                   temp_dir_.GetPath());
+    const base::CommandLine& cmdline = *base::CommandLine::ForCurrentProcess();
+    command_line.CopySwitchesFrom(cmdline, kSwitchesToCopy,
+                                  base::size(kSwitchesToCopy));
     return command_line;
   }
 

content/test/BUILD.gn

@@ -1538,6 +1538,9 @@ test("content_browsertests") {
       "//content/shell:content_shell",
       "//mojo/core:shared_library",
     ]
+    if (use_ozone) {
+      deps += [ "//ui/ozone" ]
+    }
   }
 
   if (!is_chrome_branded) {
@@ -1591,6 +1594,10 @@ test("content_browsertests") {
       "../browser/snapshot_browsertest.cc",
     ]
   }
+
+  if (use_ozone) {
+    deps += [ "//ui/ozone" ]
+  }
 }
 
 static_library("run_all_unittests") {

content/test/DEPS

@@ -50,4 +50,7 @@ specific_include_rules = {
     "+services/viz/public/cpp/gpu/command_buffer_metrics.h",
     "+services/viz/public/cpp/gpu/context_provider_command_buffer.h",
   ],
+  "content_browser_test_test.cc": [
+    "+ui/ozone/public/ozone_switches.h",
+  ]
 }

content/test/content_browser_test_test.cc

@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/location.h"
@@ -37,6 +38,10 @@
 #include "testing/gtest/include/gtest/gtest-spi.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(USE_OZONE)
+#include "ui/ozone/public/ozone_switches.h"
+#endif
+
 namespace content {
 
 // Disabled on official builds because symbolization in sandboxes processes
@@ -49,6 +54,34 @@ namespace content {
 #if !defined(OFFICIAL_BUILD) && !defined(OS_ANDROID) && !defined(OS_MAC) && \
     !(defined(COMPONENT_BUILD) && defined(OS_WIN))
 
+namespace {
+
+const char* kSwitchesToCopy[] = {
+#if defined(USE_OZONE)
+    // Keep the kOzonePlatform switch that the Ozone must use.
+    switches::kOzonePlatform,
+#endif
+    // Some tests use custom cmdline that doesn't hold switches from previous
+    // cmdline. Only a couple of switches are copied. That can result in
+    // incorrect initialization of a process. For example, the work that we do
+    // to have use_x11 && use_ozone, requires UseOzonePlatform feature flag to
+    // be passed to all the process to ensure correct path is chosen.
+    // TODO(https://crbug.com/1096425): update this comment once USE_X11 goes
+    // away.
+    switches::kEnableFeatures,
+    switches::kDisableFeatures,
+};
+
+base::CommandLine CreateCommandLine() {
+  const base::CommandLine& cmdline = *base::CommandLine::ForCurrentProcess();
+  base::CommandLine command_line = base::CommandLine(cmdline.GetProgram());
+  command_line.CopySwitchesFrom(cmdline, kSwitchesToCopy,
+                                base::size(kSwitchesToCopy));
+  return command_line;
+}
+
+}  // namespace
+
 IN_PROC_BROWSER_TEST_F(ContentBrowserTest, MANUAL_ShouldntRun) {
   // Ensures that tests with MANUAL_ prefix don't run automatically.
   ASSERT_TRUE(false);
@@ -80,8 +113,8 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest, RendererCrashCallStack) {
   base::ScopedAllowBlockingForTesting allow_blocking;
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  base::CommandLine new_test =
-      base::CommandLine(base::CommandLine::ForCurrentProcess()->GetProgram());
+
+  base::CommandLine new_test = CreateCommandLine();
   new_test.AppendSwitchASCII(base::kGTestFilterFlag,
                              "ContentBrowserTest.MANUAL_RendererCrash");
   new_test.AppendSwitch(switches::kRunManualTestsFlag);
@@ -127,8 +160,8 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest, MAYBE_BrowserCrashCallStack) {
   base::ScopedAllowBlockingForTesting allow_blocking;
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  base::CommandLine new_test =
-      base::CommandLine(base::CommandLine::ForCurrentProcess()->GetProgram());
+
+  base::CommandLine new_test = CreateCommandLine();
   new_test.AppendSwitchASCII(base::kGTestFilterFlag,
                              "ContentBrowserTest.MANUAL_BrowserCrash");
   new_test.AppendSwitch(switches::kRunManualTestsFlag);
@@ -182,8 +215,7 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest, MAYBE_RunMockTests) {
   base::ScopedAllowBlockingForTesting allow_blocking;
   base::ScopedTempDir temp_dir;
 
-  base::CommandLine command_line(
-      base::CommandLine::ForCurrentProcess()->GetProgram());
+  base::CommandLine command_line = CreateCommandLine();
   command_line.AppendSwitchASCII("gtest_filter",
                                  "MockContentBrowserTest.DISABLED_*");
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());