New issue
Advanced search Search tips

Issue 789043 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

ENABLE_MUS inconsistent in aura platforms

Project Member Reported by sadrul@chromium.org, Nov 28 2017

Issue description

We currently build and run mus related tests (in services_unittests, aura_unittests etc.) in all platforms: chromeos, windows, and Linux. However, the ENABLE_MUS (enable_mus in gn) are only turned on for chromeos [1]. As a result, code behind ENABLE_MUS (e.g. kMus [2]) are not defined in non-chromeos platforms. This means, mus-specific code actually needs to check ENABLE_MUS, which I think is pretty confusing. We should either define ENABLE_MUS in all aura platforms, or run the mus-specific tests only in chromeos.

[1] https://cs.chromium.org/chromium/src/ui/base/ui_features.gni?l=22
[2] https://cs.chromium.org/chromium/src/ui/base/ui_base_switches.h?l=61
 
Cc: toniki...@chromium.org
ENABLE_MUS for all aura platforms. Otherwise it would be very inconvenient to bring up the mus tests in external mode.
Cc: msi...@igalia.com
On a side note, this is something that we have in our downstream branch indeed (against Nov/26 ToT):

diff --git a/chrome/app/chrome_main.cc b/chrome/app/chrome_main.cc
index 53b55c8ad029..9c7b71b601ed 100644
--- a/chrome/app/chrome_main.cc
+++ b/chrome/app/chrome_main.cc
@@ -104,7 +104,7 @@ int ChromeMain(int argc, const char** argv) {
   }
 #endif  // defined(OS_LINUX) || defined(OS_MACOSX) || defined(OS_WIN)
 
-#if defined(OS_CHROMEOS) && BUILDFLAG(ENABLE_MUS)
+#if BUILDFLAG(ENABLE_MUS)
   if (service_manager::ServiceManagerIsRemote() ||
       command_line->HasSwitch(switches::kMash)) {
     params.create_discardable_memory = false;

Although the change by itself does not solve the issue sadrul@ reported (having GN/enable_mus enabled on !is_chromeos builds), but is a step forward to allow that to run.

I actually just put this specific bit up for review now: https://chromium-review.googlesource.com/c/chromium/src/+/793952
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bd5352f4973f3d84a8e09c0a5898cbb993d3310

commit 0bd5352f4973f3d84a8e09c0a5898cbb993d3310
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Tue Nov 28 17:37:47 2017

Untangle ENABLE_MUS from OS_CHROMEOS in chrome_main.cc

CL is a first step to untangle ENABLE_MUS from OS_CHROMEOS.

This does not change any ChromeOS behavior, since in ui/base/ui_features.gni
there is

  'enable_mus = is_chromeos'

.. but makes it easier to run Mus on non-ChromeOS builds.

BUG=789043

Change-Id: I0febf99ee8352af9f39c4e3411864e0a1baf3460
Reviewed-on: https://chromium-review.googlesource.com/793952
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#519728}
[modify] https://crrev.com/0bd5352f4973f3d84a8e09c0a5898cbb993d3310/chrome/app/chrome_main.cc

Components: -Internals>MUS Internals>Services>WindowService
Labels: -Proj-Mustash-Mus Proj-Mustash
Migrating Proj-Mustash-Mus to components Internals>Services>WindowService and Internals>Services>Ash

Labels: -Proj-Mustash Proj-Mash

Sign in to add a comment