New issue
Advanced search Search tips

Issue 843315 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Eliminate ash_util.cc and remove dependencies

Project Member Reported by steve...@chromium.org, May 15 2018

Issue description

ash_util.cc contains a handful of unrelated functions, most of which are either used in just one place or are trivial wrappers around other functions (i.e. IsRunningInMash()).

It also has ash/ dependencies that need to be removed.

We should move these functions to where they are used and eliminate any non public ash dependencies.

(The only function used in multiple files is SetupWidgetInitParamsForContainer(), which can probably live elsewhere).

 
Cc: sky@chromium.org
You might wanna ping sky before inline all the IsRunningInMash() calls... he's changing flag names and configs. For example, we now have features::IsMashEnabled(), which... might.... mean the same thing in both chrome and ash processes.

https://cs.chromium.org/chromium/src/ui/base/ui_base_features.cc?q=ismashenabled&sq=package:chromium&l=98

I'll double check with sky before making any changes to IsRunningInMash().

Currently there is a comment in the header that says:

// TODO(sky): convert to chromeos::GetAshConfig() and remove.

And we already use 'chromeos::GetAshConfig()' directly a lot, but also no point in making extra work if we're changing how we do this. We should probably enforce a single public mechanism going forward.

On further consideration, the ash dependencies in ash_util are all gated on ash::Config::MASH, so I will just update the DEPS comment instead.

Also, while the keyboard accelerator helpers are only used in one file, that file is browser_view.cc, which doesn't need additional CrOS conditional code added to it.


Project Member

Comment 4 by bugdroid1@chromium.org, May 22 2018

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

commit cac0f7f85699787a609202f35e32e10ee6db0643
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Tue May 22 18:30:17 2018

Minor DEPS cleanup in c/b/ui/ash and c/b/chromeos

ash_util.cc DEPS are non Mash only so are allowed.
Includes some other minor DEPS cleanup.

Bug:  843315 
Change-Id: I3b637f041774b36b9b14c654cf15b222e9a8742e
Reviewed-on: https://chromium-review.googlesource.com/1069290
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560711}
[delete] https://crrev.com/9fb98d15b5984dadb15ffa2475cc536ab539f62e/chrome/browser/chromeos/app_mode/arc/DEPS
[modify] https://crrev.com/cac0f7f85699787a609202f35e32e10ee6db0643/chrome/browser/ui/ash/DEPS
[modify] https://crrev.com/cac0f7f85699787a609202f35e32e10ee6db0643/chrome/browser/ui/ash/launcher/DEPS

Status: Fixed (was: Started)

Sign in to add a comment