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

Issue 693282 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

clean up tray constants in tray_constants.cc

Project Member Reported by est...@chromium.org, Feb 17 2017

Issue description

some pre-md stuff here that can be cleared out.
 
Issue 623987 has been merged into this issue.
Labels: Hotlist-CodeHealth
Project Member

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

Project Member

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

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

commit da123cacc94145db78abf09bcc10c1c5008ef0d0
Author: estade <estade@chromium.org>
Date: Wed Mar 01 05:02:55 2017

Consolidate some more ash tray constants.

BUG= 693282 

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

[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/chromeos/audio/volume_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/chromeos/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/chromeos/network/network_state_list_detailed_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/date/date_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/status_area_widget_delegate.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/hover_highlight_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/system_tray.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/system_tray_bubble.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/throbber_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/tray_background_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/tray_constants.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/tray_constants.h
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/tray_item_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/tray_popup_header_button.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/tray_popup_utils.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/tray/tray_utils.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/user/user_card_view.cc
[modify] https://crrev.com/da123cacc94145db78abf09bcc10c1c5008ef0d0/ash/common/system/user/user_view.cc

Status: Fixed (was: Started)
The CL in #4 deletes the GetTrayConstant() function in tray_constants, which means that all of the pre-MD constants values should now be gone. Marking this as Fixed.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 20 2017

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

commit 29fc92dc9444d3012f6259e87dd624bcd5455996
Author: estade <estade@chromium.org>
Date: Thu Apr 20 04:24:46 2017

Clean up some ash constants.

Eliminate a couple instances of TODO(estade).

BUG= 693282 

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

[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/BUILD.gn
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/root_window_controller_unittest.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/app_list_button.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/overflow_button.cc
[delete] https://crrev.com/7e7ec6b34968ce11c7cc565063a7e13a8076d3f4/ash/shelf/shelf_constants.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/shelf_constants.h
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/shelf_view.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/shelf/shelf_widget_unittest.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/system/session/logout_button_tray.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/system/status_area_widget_delegate.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/system/tray/tray_background_view.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/system/tray/tray_constants.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/system/tray/tray_constants.h
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/system/tray/tray_utils.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/system/web_notification/ash_popup_alignment_delegate.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/wm/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/ash/wm/workspace/workspace_window_resizer_unittest.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/chrome/browser/ui/apps/chrome_app_delegate.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/chrome/browser/ui/ash/launcher/launcher_favicon_loader.cc
[modify] https://crrev.com/29fc92dc9444d3012f6259e87dd624bcd5455996/chrome/browser/ui/ash/launcher/launcher_favicon_loader_browsertest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 25 2017

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

commit 097f9cde453ea57eb4aa037f44add782391c5eb9
Author: estade <estade@chromium.org>
Date: Tue Apr 25 01:01:27 2017

Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

BUG= 711183 , 693282 
TBR=stevenjb@chromium.org

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

[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/audio/volume_view.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/tray/system_tray.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/tray/tray_constants.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/tray/tray_constants.h
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/tray/tray_details_view.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/tray/tray_popup_utils.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/tray_accessibility.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/user/user_card_view.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/user/user_view.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ash/system/user/user_view.h
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/chrome/browser/ui/views/sad_tab_view.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/chrome/browser/ui/views/sad_tab_view.h
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/message_center/views/message_bubble_base.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/message_center/views/message_bubble_base.h
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/views/background.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/views/background.h
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/views/bubble/tray_bubble_view.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/views/bubble/tray_bubble_view.h
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/views/view.cc
[modify] https://crrev.com/097f9cde453ea57eb4aa037f44add782391c5eb9/ui/views/view_observer.h

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 25 2017

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

commit 1cf2905163bc2e0ec4ae52c533e751f4de7cde20
Author: pkalinnikov <pkalinnikov@chromium.org>
Date: Tue Apr 25 12:31:34 2017

Revert of Introduce a type of View background that stays in sync with its host (patchset #10 id:180001 of https://codereview.chromium.org/2816193002/ )

Reason for revert:
Seems causing a lot of test crashes on "Linux Chromium OS ASan" bots (ash_unittests, browser_tests, interactive_ui_tests, unit_tests).

Original issue's description:
> Introduce a type of View background that stays in sync with its host
> View's native theme.
>
> This fixes a few bugs where we weren't using the right native theme,
> whether by not updating after the NativeTheme changes or by trying to
> access the NativeTheme before the View is added to a hierarchy (which
> yields the default NativeTheme --- for most platforms, this didn't
> effectively create a bug as there's only one NativeTheme).
>
> Best example is that now the sad tab respects the GTK native theme.
>
> Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
> bubble background color from the NativeTheme.
>
> BUG= 711183 , 693282 
> TBR=stevenjb@chromium.org
>
> Review-Url: https://codereview.chromium.org/2816193002
> Cr-Commit-Position: refs/heads/master@{#466847}
> Committed: https://chromium.googlesource.com/chromium/src/+/097f9cde453ea57eb4aa037f44add782391c5eb9

TBR=msw@chromium.org,tdanderson@chromium.org,stevenjb@chromium.org,estade@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 711183 , 693282 

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

[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/audio/volume_view.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/tray/system_tray.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/tray/tray_constants.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/tray/tray_constants.h
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/tray/tray_details_view.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/tray/tray_popup_utils.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/tray_accessibility.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/user/user_card_view.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/user/user_view.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ash/system/user/user_view.h
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/chrome/browser/ui/views/sad_tab_view.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/chrome/browser/ui/views/sad_tab_view.h
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/message_center/views/message_bubble_base.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/message_center/views/message_bubble_base.h
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/views/background.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/views/background.h
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/views/bubble/tray_bubble_view.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/views/bubble/tray_bubble_view.h
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/views/view.cc
[modify] https://crrev.com/1cf2905163bc2e0ec4ae52c533e751f4de7cde20/ui/views/view_observer.h

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 28 2017

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

commit 56fc9d639f67506841a0cb90a421a869e5107eea
Author: estade <estade@chromium.org>
Date: Fri Apr 28 22:28:06 2017

Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

This is a reland of 097f9cde453ea57eb4aa037f44add782391c5eb9 with fix
for asan failure.

BUG= 711183 ,  693282 
TBR=stevenjb@chromium.org,tdanderson@chromium.org

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

[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/audio/volume_view.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/tray/system_tray.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/tray/tray_constants.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/tray/tray_constants.h
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/tray/tray_details_view.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/tray/tray_popup_utils.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/tray_accessibility.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/user/user_card_view.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/user/user_view.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ash/system/user/user_view.h
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/chrome/browser/ui/views/sad_tab_view.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/chrome/browser/ui/views/sad_tab_view.h
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/message_center/views/message_bubble_base.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/message_center/views/message_bubble_base.h
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/views/background.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/views/background.h
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/views/bubble/tray_bubble_view.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/views/bubble/tray_bubble_view.h
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/views/view.cc
[modify] https://crrev.com/56fc9d639f67506841a0cb90a421a869e5107eea/ui/views/view_observer.h

Comment 10 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment