X11 and Ozone: early initialize FeatureList

We need to initialize the FeatureList as early as possible.
Otherwise, VizTestSuite, AuraTestSuite, and others may not be
properly initialized because they need to check whether OzonePlatform
is used (that is not a problem when either use_x11 or use_ozone is set.
But when both are set, the tests fail to initialize for Ozone case and
take non-Ozone X11 path).

IsUsingOzonePlatform has static const initializer now so that tests
won't override the return value of this method. However, we need
to call that at least one as early as possible. Thus, TestSuite::Run
is the best place for that.

What is more, we also need to append UseOzonePlatform feature to
the CommandLine so that other processes get the same value as the
original process. The reason why we need to do that is that CommandLine
takes enabled/disabled features during the tests overriding the initial
FeatureList. Once it is restored, the CommandLine has already been
updated and won't be updated again. Thus, we need to append the
flag and let other processes start correctly (like GPU process).

-----

PS: Please note that this is a temp solution that will help to choose
between ozone and non-ozone X11 build. The switch that will be used
to choose the path is --enable-features=UseOzonePlatform. Once
non-Ozone X11 path is removed (hopefully by Q1 2021 depending on how
the finch trial goes), the wrapper will be removed.

Please also note that it's impossible to build use_x11 && use_ozone
without some hacks in PlatformCursor code. The changes to that are
on their way to upstream.

----

Bug: 1085700
Change-Id: I122bec7441b3dbd199eb4287338f8903878d2b3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333844Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Maksim Sisov (GMT+3) <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#798991}

base/test/test_suite.cc

@@ -440,7 +440,32 @@ int TestSuite::Run() {
   mac::ScopedNSAutoreleasePool scoped_pool;
 #endif
 
-  Initialize();
+  {
+    // Some features are required to be checked as soon as possible. Thus, make
+    // sure that the FeatureList is initalized before Initialize() is called so
+    // that tests that rely on this call are able to check the enabled and
+    // disabled featured passed via a command line.
+    //
+    // PS: When use_x11 and use_ozone are both true, some test suites need to
+    // check if Ozone is being used during the Initialize() call below.
+    // However, the feature list isn't initialized until later, when running
+    // each test suite inside RUN_ALL_TESTS() below. Eagerly initialize a
+    // ScopedFeatureList here to ensure the correct value is set for
+    // feature::IsUsingOzonePlatform.
+    //
+    // TODO(https://crbug.com/1096425): Remove the comment about
+    // UseOzonePlatform when USE_X11 is removed.
+    std::string enabled =
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kEnableFeatures);
+    std::string disabled =
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kDisableFeatures);
+    base::test::ScopedFeatureList feature_list;
+    feature_list.InitFromCommandLine(enabled, disabled);
+    Initialize();
+  }
+
   std::string client_func =
       CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kTestChildProcess);

content/public/test/browser_test_base.cc

@@ -68,6 +68,7 @@
 #include "services/network/public/mojom/network_service_test.mojom.h"
 #include "services/service_manager/embedder/switches.h"
 #include "services/tracing/public/cpp/trace_startup.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/compositor/compositor_switches.h"
 #include "ui/display/display_switches.h"
 #include "ui/gl/gl_implementation.h"
@@ -176,6 +177,18 @@ class InitialNavigationObserver : public WebContentsObserver {
 }  // namespace
 
 BrowserTestBase::BrowserTestBase() {
+#if defined(USE_OZONE) && defined(USE_X11)
+  // In case of the USE_OZONE + USE_X11 build, the OzonePlatform can either be
+  // enabled or disabled. However, tests may override the FeatureList that will
+  // result in unknown state for the UseOzonePlatform feature. Thus, the
+  // features::IsUsingOzonePlatform has static const initializer that won't be
+  // changed despite FeatureList being overridden. However, it requires to call
+  // this method at least once so that the value is set correctly. This place
+  // looks the most appropriate as tests haven't started to add own FeatureList
+  // yet and we still have the original value set by base::TestSuite.
+  ignore_result(features::IsUsingOzonePlatform());
+#endif
+
   CHECK(!g_instance_already_created)
       << "Each browser test should be run in a new process. If you are adding "
          "a new browser test suite that runs on Android, please add it to "
@@ -353,6 +366,19 @@ void BrowserTestBase::SetUp() {
                                                           &disabled_features);
   }
 
+#if defined(USE_X11) && defined(USE_OZONE)
+  // Append OzonePlatform to the enabled features so that the CommandLine
+  // instance has correct values, and other processes if any (GPU, for example),
+  // also use correct path.  features::IsUsingOzonePlatform() has static const
+  // initializer, which means the value of the features::IsUsingOzonePlatform()
+  // doesn't change even if tests override the FeatureList. Thus, it's correct
+  // to call it now as it is set way earlier than tests override the features.
+  //
+  // TODO(https://crbug.com/1096425): remove this as soon as use_x11 goes away.
+  if (features::IsUsingOzonePlatform())
+    enabled_features += ",UseOzonePlatform";
+#endif
+
   if (!enabled_features.empty()) {
     command_line->AppendSwitchASCII(switches::kEnableFeatures,
                                     enabled_features);

ui/base/ui_base_features.cc

@@ -228,15 +228,18 @@ const base::Feature kUseOzonePlatform {
 bool IsUsingOzonePlatform() {
   // Only allow enabling and disabling the OzonePlatform on USE_X11 && USE_OZONE
   // builds.
+  static const bool using_ozone_platform =
 #if defined(USE_X11) && defined(USE_OZONE)
-  return base::FeatureList::IsEnabled(kUseOzonePlatform);
+      base::FeatureList::IsEnabled(kUseOzonePlatform);
 #elif defined(USE_X11) && !defined(USE_OZONE)
-  // This shouldn't be switchable for pure X11 builds.
-  return false;
+      // This shouldn't be switchable for pure X11 builds.
+      false;
 #else
-  // All the other platforms must use Ozone by default and can't disable that.
-  return true;
+      // All the other platforms must use Ozone by default and can't disable
+      // that.
+      true;
 #endif
+  return using_ozone_platform;
 }
 #endif  // defined(USE_X11) || defined(USE_OZONE)