chromeos: move --mus and --mash to features

This is mildly tricky as --mash implies the window service too. So, there is now a function test if mus (meaning either Mus or Mash is enabled). This nukes IsMusHostingViz() and instead queries the feature directly. Additionally I've nuked supplying how to configure aura in init params (as it can be determined from features directly). BUG=800357 TEST=covered by tests Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ib51c5135caf9df4b4e430ccd20b63a4bea6dae93 Reviewed-on: https://chromium-review.googlesource.com/927360 Reviewed-by: Mark Pearson <mpearson@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#538591}
qweri0p · Feb 22, 2018 · 8ff9c30 · 8ff9c30
1 parent 2a02a69
commit 8ff9c30
Show file tree

Hide file tree

Showing 120 changed files with 412 additions and 628 deletions.
diff --git a/ash/README.md b/ash/README.md
@@ -28,15 +28,15 @@ Ash is transitioning to use the mus window server and gpu process, found in
 //services/ui. Ash continues to use aura, but aura is backed by mus. Code to
 support mus is found in the ash directory. There should be relatively few
 differences between the pure aura and the aura-mus versions of ash. Ash can by
-run in mus mode by passing the --mus command line flag.
+run in mus mode by passing the --enable-features=Mus command line flag.
 
 Ash is also transitioning to run as a mojo service in its own process. This
 means that code in chrome cannot call into ash directly, but must use the mojo
 interfaces in //ash/public/interfaces.
 
 Out-of-process Ash is referred to as "mash" (mojo ash). In-process ash is
 referred to as "classic ash". Ash can run in either mode depending on the
---mash command line flag.
+--enable-features=Mash command line flag.
 
 In the few cases where chrome code is allowed to call into ash (e.g. code that
 will only ever run in classic ash) the #include lines have "// mash-ok"
@@ -45,15 +45,16 @@ have not yet been adapted to mash.
 
 Mustash Tests
 -----
-ash_unittests --mus runs the test suite in mus mode. ash_unittests --mash runs
-in mash mode. Some tests will fail because the underlying code has not yet been
-ported to work with mash. We use filter files to skip these tests, because it
-makes it easier to run the entire suite without the filter to see what passes.
+ash_unittests --enable-features=Mus runs the test suite in mus mode.
+ash_unittests --enable-features=Mash runs in mash mode. Some tests will fail
+because the underlying code has not yet been ported to work with mash. We use
+filter files to skip these tests, because it makes it easier to run the entire
+suite without the filter to see what passes.
 
 To simulate what the bots run (e.g. to check if you broke an existing test that
 works under mash) you can run:
 
-`ash_unittests --mash --test-launcher-filter-file=testing/buildbot/filters/ash_unittests_mash.filter`
+`ash_unittests --enable-features=Mash --test-launcher-filter-file=testing/buildbot/filters/ash_unittests_mash.filter`
 
 Any new feature you add (and its tests) should work under mash. If your test
 cannot pass under mash due to some dependency being broken you may add the test

diff --git a/ash/display/display_synchronizer.cc b/ash/display/display_synchronizer.cc
@@ -10,6 +10,7 @@
 #include "services/ui/public/interfaces/window_manager_constants.mojom.h"
 #include "ui/aura/mus/window_manager_delegate.h"
 #include "ui/aura/mus/window_tree_host_mus.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/base/ui_base_switches_util.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/manager/managed_display_info.h"
@@ -67,7 +68,7 @@ void DisplaySynchronizer::SendDisplayConfigurationToServer() {
       Shell::Get()->window_tree_host_manager()->mirror_window_controller();
   for (const auto& mirror :
        display_manager->software_mirroring_display_list()) {
-    if (::switches::IsMusHostingViz()) {
+    if (base::FeatureList::IsEnabled(features::kMash)) {
       // If mus is hosting viz, the window server handle mirrors internally.
       mirrors.push_back(mirror);
       metrics.push_back(GetMetricsForDisplay(mirror.id()));

diff --git a/ash/display/mirror_window_controller.cc b/ash/display/mirror_window_controller.cc
@@ -24,6 +24,7 @@
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/layout.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/base/ui_base_switches_util.h"
 #include "ui/compositor/reflector.h"
 #include "ui/display/display_layout.h"
@@ -342,7 +343,7 @@ void MirrorWindowController::OnAcceleratedWidgetOverridden(
     aura::WindowTreeHost* host) {
   DCHECK_NE(host->GetAcceleratedWidget(), gfx::kNullAcceleratedWidget);
   DCHECK_NE(Shell::GetAshConfig(), Config::CLASSIC);
-  DCHECK(!switches::IsMusHostingViz());
+  DCHECK(!base::FeatureList::IsEnabled(features::kMash));
   MirroringHostInfo* info = mirroring_host_info_map_[host->GetDisplayId()];
   if (reflector_) {
     reflector_->AddMirroringLayer(info->mirror_window->layer());

diff --git a/ash/display/window_tree_host_manager.cc b/ash/display/window_tree_host_manager.cc
@@ -41,6 +41,7 @@
 #include "ui/base/class_property.h"
 #include "ui/base/ime/input_method_factory.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/base/ui_base_switches_util.h"
 #include "ui/compositor/compositor.h"
 #include "ui/display/display.h"
@@ -100,7 +101,7 @@ aura::Window* GetWindow(AshWindowTreeHost* ash_host) {
 
 bool ShouldUpdateMirrorWindowController() {
   return aura::Env::GetInstance()->mode() == aura::Env::Mode::LOCAL ||
-         !::switches::IsMusHostingViz();
+         !base::FeatureList::IsEnabled(features::kMash);
 }
 
 }  // namespace

diff --git a/ash/shell.cc b/ash/shell.cc
@@ -1347,7 +1347,8 @@ void Shell::OnLocalStatePrefServiceInitialized(
     std::unique_ptr<::PrefService> pref_service) {
   DCHECK(!local_state_);
   // |pref_service| is null if can't connect to Chrome (as happens when
-  // running mash outside of chrome --mash and chrome isn't built).
+  // running mash outside of chrome --enable-features=Mash and chrome isn't
+  // built).
   if (!pref_service)
     return;
 

diff --git a/ash/shell_port_mus.cc b/ash/shell_port_mus.cc
@@ -42,6 +42,7 @@
 #include "ui/aura/mus/window_tree_host_mus.h"
 #include "ui/aura/mus/window_tree_host_mus_init_params.h"
 #include "ui/aura/window.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/base/ui_base_switches_util.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/manager/forwarding_display_delegate.h"
@@ -210,9 +211,9 @@ std::unique_ptr<AshWindowTreeHost> ShellPortMus::CreateAshWindowTreeHost(
   aura_init_params.display_init_params = std::move(display_params);
   aura_init_params.use_classic_ime = !Shell::ShouldUseIMEService();
   aura_init_params.uses_real_accelerated_widget =
-      !::switches::IsMusHostingViz();
+      !base::FeatureList::IsEnabled(features::kMash);
 
-  if (!::switches::IsMusHostingViz()) {
+  if (!base::FeatureList::IsEnabled(features::kMash)) {
     if (init_params.mirroring_unified) {
       return std::make_unique<AshWindowTreeHostMusMirroringUnified>(
           std::move(aura_init_params), init_params.display_id,
@@ -278,7 +279,7 @@ ShellPortMus::CreateAcceleratorController() {
 
 void ShellPortMus::AddVideoDetectorObserver(
     viz::mojom::VideoDetectorObserverPtr observer) {
-  if (switches::IsMusHostingViz()) {
+  if (base::FeatureList::IsEnabled(features::kMash)) {
     // We may not have access to the connector in unit tests.
     if (!window_manager_->connector())
       return;

diff --git a/ash/system/flag_warning/flag_warning_tray.cc b/ash/system/flag_warning/flag_warning_tray.cc
@@ -25,15 +25,14 @@
 namespace ash {
 namespace {
 
-const char kTooltipText[] = "Running with flag --mash";
+const char kTooltipText[] = "Running with feature mash";
 
 }  // namespace
 
 FlagWarningTray::FlagWarningTray(Shelf* shelf) : shelf_(shelf) {
   DCHECK(shelf_);
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
-  // The flag --mash is a chrome-level switch, so check the config instead.
   const bool is_mash = Shell::GetAshConfig() == Config::MASH;
   if (is_mash) {
     container_ = new TrayContainer(shelf);

diff --git a/ash/system/flag_warning/flag_warning_tray.h b/ash/system/flag_warning/flag_warning_tray.h
@@ -20,10 +20,10 @@ namespace ash {
 class Shelf;
 class TrayContainer;
 
-// Adds an indicator to the status area if certain high-risk flags are enabled,
-// for example --mash. Clicking the button opens about:flags so the user can
-// reset the flag. For consistency with other status area tray views, this view
-// is always created but only made visible when the flag is set.
+// Adds an indicator to the status area if certain high-risk flags or features
+// are enabled, for example mash. Clicking the button opens about:flags so the
+// user can reset the flag. For consistency with other status area tray views,
+// this view is always created but only made visible when the flag is set.
 class ASH_EXPORT FlagWarningTray : public views::View,
                                    public views::ButtonListener {
  public:

diff --git a/ash/system/flag_warning/flag_warning_tray_unittest.cc b/ash/system/flag_warning/flag_warning_tray_unittest.cc
@@ -22,10 +22,10 @@ TEST_F(FlagWarningTrayTest, Visibility) {
                               ->flag_warning_tray_for_testing();
   ASSERT_TRUE(tray);
 
-  // Warning should be visible in ash_unittests --mash, but not in regular
-  // ash_unittests. The warning does not show for Config::MUS because mus will
-  // roll out via experiment/Finch trial and showing the tray would reveal
-  // the experiment state to users.
+  // Warning should be visible in ash_unittests when mash is enabled, but not in
+  // regular ash_unittests. The warning does not show for Config::MUS because
+  // mus will roll out via experiment/Finch trial and showing the tray would
+  // reveal the experiment state to users.
   const bool is_mash = Shell::GetAshConfig() == Config::MASH;
   EXPECT_EQ(tray->visible(), is_mash);
 }

diff --git a/ash/system/power/peripheral_battery_notifier_unittest.cc b/ash/system/power/peripheral_battery_notifier_unittest.cc
@@ -167,7 +167,7 @@ TEST_F(PeripheralBatteryNotifierTest, ExtractBluetoothAddress) {
   EXPECT_TRUE(non_bluetooth_device_info.bluetooth_address.empty());
 }
 
-// TODO(crbug.com/765794): Flaky on ash_unittests --mus.
+// TODO(crbug.com/765794): Flaky on ash_unittests with mus.
 TEST_F(PeripheralBatteryNotifierTest, DISABLED_DeviceRemove) {
   message_center::MessageCenter* message_center =
       message_center::MessageCenter::Get();
@@ -183,7 +183,7 @@ TEST_F(PeripheralBatteryNotifierTest, DISABLED_DeviceRemove) {
               nullptr);
 }
 
-// TODO(crbug.com/765794): Flaky on ash_unittests --mus.
+// TODO(crbug.com/765794): Flaky on ash_unittests with mus.
 TEST_F(PeripheralBatteryNotifierTest, DISABLED_StylusNotification) {
   const std::string kTestStylusBatteryPath =
       "/sys/class/power_supply/hid-AAAA:BBBB:CCCC.DDDD-battery";

diff --git a/ash/test/ash_test_helper.cc b/ash/test/ash_test_helper.cc
@@ -44,6 +44,7 @@
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/platform_window_defaults.h"
 #include "ui/base/test/material_design_controller_test_api.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/base/ui_base_switches_util.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/compositor/test/context_factories_for_test.h"
@@ -114,7 +115,8 @@ void AshTestHelper::SetUp(bool start_session, bool provide_local_state) {
       ui::ScopedAnimationDurationScaleMode::ZERO_DURATION));
   ui::InitializeInputMethodForTesting();
 
-  if (config_ == Config::MUS && !::switches::IsMusHostingViz()) {
+  if (config_ == Config::MUS &&
+      !base::FeatureList::IsEnabled(features::kMash)) {
     ui::ContextFactory* context_factory = nullptr;
     ui::ContextFactoryPrivate* context_factory_private = nullptr;
     ui::InitializeContextFactoryForTests(false /* enable_pixel_output */,
@@ -264,7 +266,7 @@ void AshTestHelper::RunAllPendingInMessageLoop() {
 void AshTestHelper::NotifyClientAboutAcceleratedWidgets() {
   if (config_ == Config::CLASSIC)
     return;
-  if (::switches::IsMusHostingViz())
+  if (base::FeatureList::IsEnabled(features::kMash))
     return;
   Shell* shell = Shell::Get();
   window_tree_client_setup_.NotifyClientAboutAcceleratedWidgets(

diff --git a/ash/test/ash_test_suite.cc b/ash/test/ash_test_suite.cc
@@ -11,10 +11,12 @@
 #include "base/files/file_path.h"
 #include "base/i18n/rtl.h"
 #include "base/path_service.h"
+#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/test/aura_test_context_factory.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/base/ui_base_paths.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/compositor/test/context_factories_for_test.h"
@@ -58,24 +60,19 @@ void AshTestSuite::Initialize() {
         ash_test_resources_200, ui::SCALE_FACTOR_200P);
   }
 
-  const bool is_mus = base::CommandLine::ForCurrentProcess()->HasSwitch("mus");
-  const bool is_mash =
-      base::CommandLine::ForCurrentProcess()->HasSwitch("mash");
+  const bool is_mus = features::IsMusEnabled();
+  const bool is_mash = base::FeatureList::IsEnabled(features::kMash);
   AshTestHelper::config_ =
-      is_mus ? Config::MUS : is_mash ? Config::MASH : Config::CLASSIC;
+      is_mash ? Config::MASH : is_mus ? Config::MUS : Config::CLASSIC;
 
   base::DiscardableMemoryAllocator::SetInstance(&discardable_memory_allocator_);
-  env_ = aura::Env::CreateInstance(is_mus || is_mash ? aura::Env::Mode::MUS
-                                                     : aura::Env::Mode::LOCAL);
+  env_ = aura::Env::CreateInstance(is_mus ? aura::Env::Mode::MUS
+                                          : aura::Env::Mode::LOCAL);
 
   if (is_mash) {
     context_factory_ = std::make_unique<aura::test::AuraTestContextFactory>();
     env_->set_context_factory(context_factory_.get());
     env_->set_context_factory_private(nullptr);
-    // mus needs to host viz, because ash by itself cannot.
-    base::CommandLine::ForCurrentProcess()->AppendSwitch(switches::kMus);
-    base::CommandLine::ForCurrentProcess()->AppendSwitch(
-        switches::kMusHostingViz);
   }
 }
 

diff --git a/chrome/app/chrome_main.cc b/chrome/app/chrome_main.cc
@@ -14,14 +14,8 @@
 #include "content/public/app/content_main.h"
 #include "content/public/common/content_switches.h"
 #include "headless/public/headless_shell.h"
-#include "ui/base/ui_base_switches.h"
-#include "ui/base/ui_features.h"
 #include "ui/gfx/switches.h"
 
-#if BUILDFLAG(ENABLE_MUS)
-#include "services/service_manager/runner/common/client_util.h"
-#endif
-
 #if defined(OS_MACOSX)
 #include "chrome/app/chrome_main_mac.h"
 #endif
@@ -49,39 +43,6 @@ int ChromeMain(int argc, const char** argv);
 }
 #endif
 
-namespace {
-
-#if BUILDFLAG(ENABLE_MUS)
-void ConfigureMus(content::ContentMainParams* params) {
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-#if defined(OS_CHROMEOS)
-  // --mash implies --mus, so check it first.
-  if (command_line->HasSwitch(switches::kMash)) {
-    params->create_discardable_memory = false;
-    params->env_mode = aura::Env::Mode::MUS;
-    // Don't add --mus if the user had both --mash and --mus.
-    if (!command_line->HasSwitch(switches::kMus))
-      command_line->AppendSwitch(switches::kMus);
-    command_line->AppendSwitch(switches::kMusHostingViz);
-    return;
-  }
-#endif  // defined(OS_CHROMEOS)
-
-  // In config==mus the ui service runs in process and is shut down well before
-  // the rest of Chrome. Have Chrome create the DiscardableSharedMemoryManager
-  // to ensure the DiscardableSharedMemoryManager is destroyed later on. Doing
-  // this avoids lifetime issues when internal implementation details of
-  // DiscardableSharedMemoryManager assume DiscardableSharedMemoryManager is
-  // long lived.
-  if (command_line->HasSwitch(switches::kMus)) {
-    params->create_discardable_memory = true;
-    params->env_mode = aura::Env::Mode::MUS;
-  }
-}
-#endif  // BUILDFLAG(ENABLE_MUS)
-
-}  // namespace
-
 #if defined(OS_WIN)
 DLLEXPORT int __cdecl ChromeMain(HINSTANCE instance,
                                  sandbox::SandboxInterfaceInfo* sandbox_info,
@@ -137,10 +98,6 @@ int ChromeMain(int argc, const char** argv) {
   }
 #endif  // defined(OS_LINUX) || defined(OS_MACOSX) || defined(OS_WIN)
 
-#if BUILDFLAG(ENABLE_MUS)
-  ConfigureMus(&params);
-#endif
-
   int rv = content::ContentMain(params);
 
   return rv;

diff --git a/chrome/browser/DEPS b/chrome/browser/DEPS
@@ -81,9 +81,8 @@ include_rules = [
   "+third_party/libaom/av1_features.h",
   "+third_party/metrics_proto",
 
-  # Code under //ash runs out-of-process under mustash (chrome --mash) so it
-  # must be accessed via mojo interfaces in //ash/public/interfaces. See
-  # //ash/README.md.
+  # Code under //ash runs out-of-process in mash so it must be accessed via mojo
+  # interfaces in //ash/public/interfaces. See //ash/README.md.
   "-ash",
   "+ash/public",
   "+ash/components/quick_launch/public/mojom",

diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
@@ -1530,10 +1530,10 @@ const FeatureEntry kFeatureEntries[] = {
      FEATURE_VALUE_TYPE(features::kMultiDeviceApi)},
     {"mus", flag_descriptions::kUseMusName,
      flag_descriptions::kUseMusDescription, kOsCrOS,
-     SINGLE_VALUE_TYPE(switches::kMus)},
+     FEATURE_VALUE_TYPE(features::kMus)},
     {"mash", flag_descriptions::kUseMashName,
      flag_descriptions::kUseMashDescription, kOsCrOS,
-     SINGLE_VALUE_TYPE(switches::kMash)},
+     FEATURE_VALUE_TYPE(features::kMash)},
     {"show-taps", flag_descriptions::kShowTapsName,
      flag_descriptions::kShowTapsDescription, kOsCrOS,
      SINGLE_VALUE_TYPE(ash::switches::kShowTaps)},
@@ -3792,7 +3792,7 @@ class FlagsStateSingleton {
 bool SkipConditionalFeatureEntry(const FeatureEntry& entry) {
   version_info::Channel channel = chrome::GetChannel();
 #if defined(OS_CHROMEOS)
-  // Don't expose --mash on stable channel.
+  // Don't expose mash on stable channel.
   if (!strcmp("mash", entry.internal_name) &&
       channel == version_info::Channel::STABLE) {
     return true;

diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
@@ -248,6 +248,7 @@
 #include "third_party/WebKit/public/platform/modules/webshare/webshare.mojom.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/base/ui_features.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "url/gurl.h"
@@ -3247,7 +3248,7 @@ void ChromeContentBrowserClient::RegisterOutOfProcessServices(
       l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_UNZIP_NAME);
 
 #if defined(OS_CHROMEOS)
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kMash))
+  if (base::FeatureList::IsEnabled(features::kMash))
     mash_service_registry::RegisterOutOfProcessServices(services);
 #endif
 }