New issue
Advanced search Search tips

Issue 666773 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature


Sign in to add a comment

Drop support for ash on non-Chrome OS build targets

Project Member Reported by jamescook@chromium.org, Nov 18 2016

Issue description

This is a tracking bug.

Change our build rules in src/ash such that they only support builds with target_os="chromeos". ash_unittests and ash_shell_with_content will stop compiling on Windows and on non-chromeos Linux. This will simplify our build configs, source code, and tests.

Details: A few years ago we shipped the ash window manager and system UI on Windows to support "Metro-mode" in Windows 8. This never really caught on, so we stopped doing it and the Windows support for ash in src/chrome was removed.

However, ash_unittests and ash_shell_with_content were left on the build bots for Windows. A couple of developers on the mustash team have been using these build targets for developing the mus window server on Windows. They've agreed that maintaining another config is more expensive than the benefit they get from it (thanks, guys!).

beng@ is going to throw together a lightweight mus window manager that can run on Windows. That will allow development of the mus window server and testing of ui and views code.

The ash targets currently compile on non-chromeos Linux, but don't run, and have not done so for months. I will remove support for non-chromeos Linux as well.

I would like to do the following things:
1. Take the ash_unittests target off all the non-chromeos bots
2. Simplify the BUILD.gn files to remove is_chromeos blocks
3. Eliminate unnecessary cross-platform base classes (like I did recently with TrayAudio vs. TrayAudioChromeos)
4. Flatten the directory structure to remove .../chromeos/... directories
5. (Maybe) clean up #if defined(OS_CHROMEOS) code

See blocked-on for individual tasks.

 
Blockedon: 666775
Blockedon: 666776
Blockedon: 666777
Blockedon: 666778
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20 2016

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

commit 51df1c176fce083f07c22f5b13ec79c45f44fc0e
Author: estade <estade@chromium.org>
Date: Tue Dec 20 18:52:39 2016

Remove non-cros support from ash/common/system.

BUG= 666773 

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

[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/date/date_default_view.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/date/system_info_default_view.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/date/system_info_default_view.h
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/date/tray_date.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/date/tray_date.h
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/date/tray_system_info.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/date/tray_system_info.h
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/status_area_widget.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/status_area_widget.h
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/system_notifier.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/tiles/tiles_default_view.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/tiles/tray_tiles_unittest.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/user/tray_user_unittest.cc
[modify] https://crrev.com/51df1c176fce083f07c22f5b13ec79c45f44fc0e/ash/common/system/web_notification/web_notification_tray.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 6 2017

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

commit 599dea53e8b6318344fd9d7375c9021d2bde0adb
Author: jamescook <jamescook@chromium.org>
Date: Fri Jan 06 21:08:23 2017

cros: Remove OS_CHROMEOS ifdefs in ash/common

Ash only builds on Chrome OS these days, so it doesn't need platform
ifdefs.

The only thing remaining in ash/common is accelerators, which is a bigger
reshuffle.

BUG= 666773 
TEST=ash_unittests

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

[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/ash_constants.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/ash_constants.h
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/ash_switches.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/ash_switches.h
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/devtools/ash_devtools_unittest.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/mojo_interface_factory.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/shutdown_controller.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/wm/maximize_mode/maximize_mode_controller.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/wm/maximize_mode/maximize_mode_controller.h
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/wm/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/wm_root_window_controller.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/wm_shell.cc
[modify] https://crrev.com/599dea53e8b6318344fd9d7375c9021d2bde0adb/ash/common/wm_shell.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 9 2017

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

commit 01bf23e7e27102560b44f1353d8f55e6dd62457a
Author: jamescook <jamescook@chromium.org>
Date: Mon Jan 09 19:58:15 2017

cros: Remove some OS platform ifdefs from ash

Ash only builds on Chrome OS, so there's no need for OS_CHROMEOS and OS_WIN
checks.

BUG= 666773 
TEST=compiles

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

[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/app_list/app_list_presenter_delegate.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/aura/wm_shell_aura.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/aura/wm_shell_aura.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/content/shell_content_state.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/content/shell_content_state.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/dip_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/drag_drop/drag_drop_controller_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/drag_drop/drag_drop_interactive_uitest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/magnifier/magnification_controller_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/metrics/user_metrics_recorder.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/metrics/user_metrics_recorder.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/mus/bridge/wm_shell_mus.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/mus/bridge/wm_shell_mus.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/mus/context_menu_mus.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/mus/window_manager_application.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/mus/window_manager_application.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/root_window_controller.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/root_window_controller.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/root_window_controller_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/shell.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/shell.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/shell/content/shell_with_content_main.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/shell_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/system/overview/overview_button_tray_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/system/web_notification/ash_popup_alignment_delegate_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/test/content/test_shell_content_state.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/test/test_shell_delegate.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/tooltips/tooltip_controller_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/touch/touch_observer_hud.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/touch/touch_observer_hud.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/touch/touch_observer_hud_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/utility/screenshot_controller_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/ash_native_cursor_manager.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/immersive_fullscreen_controller_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/lock_state_controller.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/lock_state_controller_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/overview/window_selector_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/power_button_controller.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/power_button_controller.h
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/screen_dimmer_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/system_gesture_event_filter.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/toplevel_window_event_handler_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/window_manager_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/window_positioner_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/workspace/workspace_event_handler_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/01bf23e7e27102560b44f1353d8f55e6dd62457a/ash/wm/workspace/workspace_window_resizer_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 19 2017

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

commit 095fbc7f5a33b47c9506cecf8007e809552aa7b1
Author: estade <estade@chromium.org>
Date: Thu Jan 19 02:10:30 2017

Revert of Remove some ifdefs from ash since it only supports ChromeOS now. (patchset #3 id:40001 of https://codereview.chromium.org/2631623003/ )

Reason for revert:
ASH_EXPORT was necessary after all.

Original issue's description:
> Remove some ifdefs from ash since it only supports ChromeOS now.
>
> BUG= 666773 
>
> Review-Url: https://codereview.chromium.org/2631623003
> Cr-Commit-Position: refs/heads/master@{#444590}
> Committed: https://chromium.googlesource.com/chromium/src/+/4471855e032876c6194f5a414fa72f28b30e057d

TBR=jamescook@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 666773 

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

[modify] https://crrev.com/095fbc7f5a33b47c9506cecf8007e809552aa7b1/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/095fbc7f5a33b47c9506cecf8007e809552aa7b1/ash/display/display_util.cc
[modify] https://crrev.com/095fbc7f5a33b47c9506cecf8007e809552aa7b1/ash/display/display_util.h
[modify] https://crrev.com/095fbc7f5a33b47c9506cecf8007e809552aa7b1/ash/display/mirror_window_controller.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 20 2017

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

commit bd3a851941ee14ff689f94cc0edaef35deade131
Author: estade <estade@chromium.org>
Date: Fri Jan 20 01:15:12 2017

Reland 4471855e032876c

Without change to ASH_EXPORTs.

------------------------------------------------------------------------
Remove some ifdefs from ash since it only supports ChromeOS now.

BUG= 666773 

Review-Url: https://codereview.chromium.org/2631623003
------------------------------------------------------------------------

BUG= 666773 

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

[modify] https://crrev.com/bd3a851941ee14ff689f94cc0edaef35deade131/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/bd3a851941ee14ff689f94cc0edaef35deade131/ash/display/display_util.cc
[modify] https://crrev.com/bd3a851941ee14ff689f94cc0edaef35deade131/ash/display/display_util.h
[modify] https://crrev.com/bd3a851941ee14ff689f94cc0edaef35deade131/ash/display/mirror_window_controller.cc

Blockedon: 687350
Project Member

Comment 14 by bugdroid1@chromium.org, Feb 13 2017

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

commit 6ed406b794156c16f82ba5d76776381acaeb8f33
Author: jamescook <jamescook@chromium.org>
Date: Mon Feb 13 18:31:13 2017

ash: Remove OS_CHROMEOS ifdefs from accelerator files

Ash used to build on Windows but now only builds on Chrome OS. It does't
need OS_CHROMEOS ifdefs anymore.

This removes the last ifdefs from src/ash.

BUG= 666773 
TEST=compiles, ash_unittests

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

[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/accelerators/accelerator_controller_delegate_aura.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/accelerators/accelerator_filter_unittest.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/accelerators/magnifier_key_scroller.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/ash_strings.grd
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/common/accelerators/accelerator_controller.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/common/accelerators/accelerator_router.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/common/accelerators/accelerator_table.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/common/accelerators/accelerator_table.h
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/common/accelerators/accelerator_table_unittest.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/common/accelerators/debug_commands.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/common/accelerators/exit_warning_handler.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/host/ash_window_tree_host_x11.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/host/ash_window_tree_host_x11.h
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/host/ash_window_tree_host_x11_unittest.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/mus/accelerators/accelerator_controller_delegate_mus.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/mus/accelerators/accelerator_controller_unittest.cc
[modify] https://crrev.com/6ed406b794156c16f82ba5d76776381acaeb8f33/ash/wm/workspace/workspace_window_resizer_unittest.cc

Project Member

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

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

commit 087deb321baa68111033bbe9e74d1ef255606ac3
Author: jamescook <jamescook@chromium.org>
Date: Tue Feb 28 21:08:44 2017

chromeos: Merge ash_chromeos_strings.grdp into ash_strings.grd

Ash only runs on Chrome OS, so there's no need for a separate Chrome OS-only
strings file.

BUG= 666773 
TEST=ash_unittests, run chrome and verify strings are visible

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

[modify] https://crrev.com/087deb321baa68111033bbe9e74d1ef255606ac3/ash/OWNERS
[delete] https://crrev.com/6a6e60653e99e4f6ae358a282e2893d05ef04427/ash/ash_chromeos_strings.grdp
[modify] https://crrev.com/087deb321baa68111033bbe9e74d1ef255606ac3/ash/ash_strings.grd
[modify] https://crrev.com/087deb321baa68111033bbe9e74d1ef255606ac3/ash/common/accelerators/accelerator_table.cc

Blockedon: 690096
Blockedon: -690096
Status: Fixed (was: Assigned)
This is done!

Labels: code-change
Status: Verified (was: Fixed)

Sign in to add a comment