New issue
Advanced search Search tips

Issue 778261 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task



Sign in to add a comment

session_manager's chrome_setup.cc shouldn't include unconditionally-set flags

Project Member Reported by derat@chromium.org, Oct 25 2017

Issue description

If session_manager always passes a flag to Chrome, we should consider hardcoding the behavior within OS_CHROMEOS builds of Chrome instead. Long command lines make debugging harder.

I see the following always-set flags:

--max-unused-resource-memory-usage-percentage=5
--login-profile=user
--enterprise-enrollment-initial-modulus=15
--enterprise-enrollment-modulus-limit=19

and a zillion --vmodule flags. :-(

If we want any of these flags to be set when running on a real device but not when a developer runs their own OS_CHROMEOS build of Chrome on their workstation, we could probably achieve that by checking within Chrome if we're running on a Chrome OS system.

I don't have a good answer for our logging mess yet. We use VLOG(1) and --vmodule because of the LOG(INFO) ban, IIRC. At the very least, we should audit the modules that we enable and unset ones that are no longer relevant.

Simon, if you have someone just getting into Chrome development, feel free to steal this for them.
 
I'm new to chromium dev and I know this is assigned, but could I help on it? If I can, how does it work with the Chrome OS, can I do anything on my machine (mac) or would I need to run some in a vm? Thanks

Comment 2 by derat@chromium.org, Oct 30 2017

If you're interested in helping with Chrome OS development, the first step is getting familiar with building and using system images. https://www.chromium.org/chromium-os/developer-guide is the development guide. Please use the chromium-os-dev mailing list (https://groups.google.com/a/chromium.org/forum/#!forum/chromium-os-dev) if you run into problems.

Comment 3 Deleted

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/e3ac496bc7d22a1d2d9080a3b87458eef3dfcf47

commit e3ac496bc7d22a1d2d9080a3b87458eef3dfcf47
Author: Daniel Erat <derat@chromium.org>
Date: Wed Jan 31 13:58:01 2018

login: Remove --max-unused-resource-memory-usage-percentage.

Make session_manager stop passing an unused Chrome flag.

BUG=chromium:778261
TEST=none

Change-Id: I9c75f6cdc9101f87f7cd49cc1177204d0f792ee0
Reviewed-on: https://chromium-review.googlesource.com/893618
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>

[modify] https://crrev.com/e3ac496bc7d22a1d2d9080a3b87458eef3dfcf47/login_manager/chrome_setup.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 1 2018

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

commit abe1fb41ae6bba252b1c2e07b643bdfdfad1af44
Author: Daniel Erat <derat@chromium.org>
Date: Thu Feb 01 01:13:29 2018

chromeos: Remove never-set flags from crash_keys.cc.

Remove a few flags that are never set from crash_keys.cc:

--max-unused-resource-memory-usage-percentage
--termination-message-file

Bug: 778261
Change-Id: Ib06e832fa9133c4310cf5358f50778ea1917e4d2
Reviewed-on: https://chromium-review.googlesource.com/894828
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533506}
[modify] https://crrev.com/abe1fb41ae6bba252b1c2e07b643bdfdfad1af44/chrome/common/crash_keys.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/5c3eaef69a1ebc495a40fc787a1fbfb61aa236d9

commit 5c3eaef69a1ebc495a40fc787a1fbfb61aa236d9
Author: Daniel Erat <derat@chromium.org>
Date: Thu Feb 01 02:00:59 2018

system_api: Delete unused termination-message-file constant.

BUG=chromium:778261, chromium:233833 
TEST=none

Change-Id: I5f35e25a040bf8968642eedc9a6cbba917d45189
Reviewed-on: https://chromium-review.googlesource.com/894831
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>

[modify] https://crrev.com/5c3eaef69a1ebc495a40fc787a1fbfb61aa236d9/switches/chrome_switches.h

Comment 7 by sjg@chromium.org, May 14 2018

Is this bug still active?

Comment 8 by derat@chromium.org, May 15 2018

Status: Available (was: Assigned)
I'm not probably going to do anything more here in the near future.

I deleted some obsolete flags last week in https://crrev.com/c/1053566.

I don't see much in the way of additional always-set flags in chrome_setup.cc beyond these:

--enterprise-enrollment-initial-modulus=15
--enterprise-enrollment-modulus-limit=19

There are also these from platform2/libchromeos-ui/chromeos/ui/chromium_command_builder.cc (which session_manager uses):

--enable-logging
--log-level=1
--use-cras
--enable-wayland-server
--ui-prioritize-in-gpu-process

We'd want to check that Chrome defaults to the behavior that those flags request and then stop passing the flags.
Status: Assigned (was: Available)

Sign in to add a comment