New issue
Advanced search Search tips

Issue 626778 link

Starred by 5 users

Issue metadata

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

Blocking:
issue 581462
issue 612242



Sign in to add a comment

Unify ui::DisplaySnapshot and ash::DisplayInfo

Project Member Reported by kylec...@chromium.org, Jul 8 2016

Issue description

There is a large overlap between ui::DisplaySnapshot and ash::DisplayInfo. It would be ideal if we could unify the implementations somehow.

One possibility is for ash::DisplayInfo to hold onto a ui::DisplaySnapshot. Another more difficult possibility is to merge the classes together.

This is complicated by DIP vs DDP differences between the two classes.
 
Blocking: 612242
Cc: msw@chromium.org
msw@ saw this recently as well

Comment 3 by msw@chromium.org, Jul 8 2016

I had noticed the similarities between ui and ash DisplayMode types:
  ui::DisplayMode (ui/display/types/display_mode.h)
  ash::DisplayMode (ash/common/display/display_info.h)
But, yeah, there seem to be a number of similar types and access methods.
Owner: rjkroege@chromium.org
Status: Started (was: Untriaged)
Blocking: 620927
Labels: screen tadpole
Outline of approach:

* make ash::DisplayMode look like ui::DisplayMode
* augment ui::DisplayMode
* remove ash::DisplayMode
* move ash::DisplayInfo and friends to ui
* merge ash::DisplayInfo with ui::DisplaySnapshot
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 5 2016

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

commit 8503c15d538e221dcca5843c9376f86efd244c3e
Author: rjkroege <rjkroege@chromium.org>
Date: Fri Aug 05 21:19:42 2016

Make ash::DisplayMode more like ui::DisplayMode

As a first step towards merging ui::DisplaySnapshot and ash::DisplayInfo,
make ash::DisplayMode more like ui::DisplayMode so that a subsequent CL
can remove ash::DisplayMode.

BUG=626778

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

[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/accelerators/accelerator_commands_aura.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/common/display/display_info.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/common/display/display_info.h
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/common/display/display_info_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_change_observer_chromeos.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_change_observer_chromeos.h
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_change_observer_chromeos_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_manager.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_manager.h
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_manager_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_util.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/display_util.h
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/resolution_notification_controller.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/resolution_notification_controller.h
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/resolution_notification_controller_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/test/display_manager_test_api.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/touch/touch_transformer_controller_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/ash/touch/touchscreen_util_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/chrome/browser/chromeos/display/display_preferences.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/chrome/browser/chromeos/display/display_preferences_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/chrome/browser/extensions/display_info_provider_chromeos.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/chrome/browser/extensions/display_info_provider_chromeos_unittest.cc
[modify] https://crrev.com/8503c15d538e221dcca5843c9376f86efd244c3e/chrome/browser/ui/webui/options/chromeos/display_options_handler.cc

Status: Available (was: Started)
This still seems desirable but is not strictly necessary to advance the larger goal of full display management on mus.
Blocking: -620927
Components: Internals>MUS
Labels: Proj-Mustash
Blocking: 581462
Labels: -mus -Pri-2 -tadpole -pixelmus -mash -screen Proj-Mustash-Mus-GPU Proj-Mustash-Mash Proj-Mustash-Milestone-Tadpole Pri-3
Components: -Internals>MUS Internals>Services>WindowService
Components: -MUS
Components: -Internals>Services>WindowService Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Status: Assigned (was: Available)
Cc: abodenha@chromium.org
Components: UI>Shell
Labels: -Proj-Mustash -Proj-Mustash-Mus-GPU -Proj-Mustash-Milestone-Tadpole Hotlist-CodeHealth OS-Chrome
Bug scrub: Doesn't seem on critical path for mash.

abodenha, this is a good example of a code health issue for the cros team.

Sign in to add a comment