New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 697053 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Code cleanup of ash_util

Project Member Reported by fw...@igalia.com, Feb 28 2017

Issue description

This is a meta bug to track some code cleanup regarding the ash_util module.
 
Cc: -toniki...@igalia.com toniki...@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 28 2017

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

commit 02332dbe5aff4778c597467c4fc62796586464cf
Author: fwang <fwang@igalia.com>
Date: Tue Feb 28 17:48:37 2017

Replace more hardcoded "ash" strings with ash::mojom::kServiceName

The ash::mojom::kServiceName constant was introduced in [1] and replaced
some of the hardcoded "ash" strings. This CL finishes that work. It also
removes the ash_util::GetAshServiceName helper function and replaced it
with direct use of ash::mojom::kServiceName.

[1] https://codereview.chromium.org/2710673002/

BUG= 697053 

Review-Url: https://codereview.chromium.org/2718043002
Cr-Commit-Position: refs/heads/master@{#453636}

[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/chromeos/locale_change_guard.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/chromeos/settings/shutdown_policy_forwarder.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/app_list/app_list_presenter_service.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/ash_util.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/ash_util.h
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/cast_config_client_media_router.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/chrome_new_window_client.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/media_client.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/session_controller_client.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/system_tray_client.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/volume_controller.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/ash/vpn_list_forwarder.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
[modify] https://crrev.com/02332dbe5aff4778c597467c4fc62796586464cf/mash/package/mash_packaged_service.cc

Project Member

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

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

commit 81eed96866bf40ae48128b4551e0cad292bb2a7a
Author: fwang <fwang@igalia.com>
Date: Tue Feb 28 20:46:47 2017

Use ash_util::IsRunningInMash in chromeos::OutputProtectionDelegate

BUG= 697053 

Review-Url: https://codereview.chromium.org/2721883002
Cr-Commit-Position: refs/heads/master@{#453693}

[modify] https://crrev.com/81eed96866bf40ae48128b4551e0cad292bb2a7a/chrome/browser/chromeos/display/output_protection_delegate.cc

Comment 5 by fw...@igalia.com, Mar 1 2017

All the files including ash_util.h should now use one of its function at least once. Here is the count:

$ for file in `find -type f -name '*.cc' -o -name '*.h' | xargs grep '/ash_util.h"' | sed 's/:.*//'`; do echo $file `egrep -c 'CreateEmbeddedAshService|ShouldOpenAshOnStartup|IsRunningInMash|IsAcceleratorDeprecated' $file` ; done
./chrome/browser/chrome_content_browser_client.cc 2
./chrome/browser/chrome_browser_main_extra_parts_exo.cc 1
./chrome/browser/extensions/display_info_provider_chromeos.cc 11
./chrome/browser/ui/browser_command_controller.cc 1
./chrome/browser/ui/browser_command_controller.cc 1
./chrome/browser/ui/views/frame/browser_view.cc 1
./chrome/browser/ui/views/frame/browser_view.cc 1
./chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc 6
./chrome/browser/ui/views/task_manager_view.cc 1
./chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc 5
./chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc 4
./chrome/browser/ui/views/chrome_web_dialog_view.cc 1
./chrome/browser/ui/views/chrome_views_delegate.cc 1
./chrome/browser/ui/ash/system_tray_client.cc 1
./chrome/browser/ui/ash/app_list/app_list_service_ash.cc 1
./chrome/browser/ui/ash/ash_util.cc 6
./chrome/browser/ui/window_sizer/window_sizer.cc 2
./chrome/browser/ui/window_sizer/window_sizer_ash.cc 1
./chrome/browser/ui/window_sizer/window_sizer_ash_unittest.cc 1
./chrome/browser/ui/webui/settings/md_settings_ui.cc 1
./chrome/browser/ui/webui/chromeos/login/oobe_ui.cc 2
./chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc 3
./chrome/browser/ui/webui/options/chromeos/power_handler.cc 1
./chrome/browser/ui/webui/options/chromeos/display_options_handler.cc 3
./chrome/browser/fullscreen_chromeos.cc 1
./chrome/browser/chromeos/accessibility/accessibility_manager.cc 4
./chrome/browser/chromeos/extensions/wallpaper_private_api.cc 2
./chrome/browser/chromeos/chrome_browser_main_chromeos.cc 9
./chrome/browser/chromeos/bluetooth/bluetooth_pairing_dialog.cc 1
./chrome/browser/chromeos/input_method/input_method_manager_impl.cc 1
./chrome/browser/chromeos/input_method/candidate_window_controller_impl.cc 1
./chrome/browser/chromeos/login/wizard_controller.cc 1
./chrome/browser/chromeos/login/ui/input_events_blocker.cc 2
./chrome/browser/chromeos/login/ui/login_display_host_impl.cc 19
./chrome/browser/chromeos/login/ui/webui_login_view.cc 3
./chrome/browser/chromeos/login/session/chrome_session_manager.cc 1
./chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc 1
./chrome/browser/chromeos/display/output_protection_delegate.cc 1
./chrome/browser/chromeos/options/network_config_view.cc 1

IsAcceleratorDeprecated is actually used only once. See https://codereview.chromium.org/1177773002#msg25 for the rationale.

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 1 2017

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

commit 052ae58bf06d71d46567ea3c1e8952c24ec69b29
Author: fwang <fwang@igalia.com>
Date: Wed Mar 01 06:29:36 2017

Move ash_util functions from 'chrome' namespace to 'ash_util' namespace

This CL moves the following functions of ash_util.h from the 'chrome'
namespace to the 'ash_util' namespace:

- ShouldOpenAshOnStartup
- IsRunningInMash
- IsAcceleratorDeprecated

BUG= 697053 

Review-Url: https://codereview.chromium.org/2718333002
Cr-Commit-Position: refs/heads/master@{#453864}

[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chrome_browser_main_extra_parts_exo.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/bluetooth/bluetooth_pairing_dialog.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/display/output_protection_delegate.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/input_method/candidate_window_controller_impl.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/login/session/chrome_session_manager.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/login/ui/input_events_blocker.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/login/ui/login_display_host_impl.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/login/ui/webui_login_view.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/login/wizard_controller.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/chromeos/options/network_config_view.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/extensions/display_info_provider_chromeos.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/fullscreen_chromeos.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/ash/app_list/app_list_service_ash.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/ash/ash_util.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/ash/ash_util.h
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/ash/system_tray_client.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/browser_command_controller.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/views/chrome_views_delegate.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/views/chrome_web_dialog_view.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/views/task_manager_view.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/webui/options/chromeos/display_options_handler.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/webui/options/chromeos/power_handler.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/window_sizer/window_sizer.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/window_sizer/window_sizer_ash.cc
[modify] https://crrev.com/052ae58bf06d71d46567ea3c1e8952c24ec69b29/chrome/browser/ui/window_sizer/window_sizer_ash_unittest.cc

Comment 7 by fw...@igalia.com, Mar 2 2017

Status: Verified (was: Assigned)
Closing this bug as I believe enough work has been done on ash_util clean up.
Components: -MUS Internals>Services>WindowService

Sign in to add a comment