New issue
Advanced search Search tips

Issue 704942 link

Starred by 0 users

Issue metadata

Status: WontFix
Owner:
Closed: Aug 14
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Incognito icon looks terrible in mash

Project Member Reported by xiy...@chromium.org, Mar 24 2017

Issue description

See attached screenshot. Looks like the frame bg color is too bright.
 
mash-incognito-browser-frame.png
6.3 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 16 2018

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

commit 7aecde2142f40040f62bbab2cad88c56dcec9079
Author: Trent Apted <tapted@chromium.org>
Date: Fri Mar 16 09:40:33 2018

Ensure omnibox_theme.cc adapts to theme changes.

This is easily done with an override of View::OnThemeChanged()

Less easily done is a test: themes are complex. Add a test for the
selection highlight, which captures the current logic around theme
color propagation.

Bug: 801583,  819425 ,  704942 
Change-Id: I86f3e4d9b5a30db66ad5a28d2ce7581aa9f7320b
Reviewed-on: https://chromium-review.googlesource.com/952802
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543657}
[modify] https://crrev.com/7aecde2142f40040f62bbab2cad88c56dcec9079/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/7aecde2142f40040f62bbab2cad88c56dcec9079/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/7aecde2142f40040f62bbab2cad88c56dcec9079/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 7 2018

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

commit 5d480c704994ea82c4f3ca1fd94b1761668c6849
Author: Evan Stade <estade@chromium.org>
Date: Mon May 07 20:20:07 2018

Mash - Support for browser themes in window frame.

1. Move BrowserFrameHeaderAsh to //ash/frame and rename to
   CustomFrameHeader.
2. Use CustomFrameHeader for browser windows (in HeaderView).
   Continue to use DefaultFrameHeader for other windows.
   Possible TODO: share more code between the two FrameHeader classes.
3. Transport frame images over mojo so Mash can apply them
   via CustomFrameHeader.

For now, only the active frame image is propagated (as a POC).
The inactive image, overlay images, and frame colors are TODO.
Ideally, there should be no behavioral changes in classic Ash.

BUG= 704942 , 836992 

Change-Id: I15b2d357bd3c8b60b6656ebce910d476ce107b44
Reviewed-on: https://chromium-review.googlesource.com/1043221
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556549}
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/BUILD.gn
[rename] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/custom_frame_header.cc
[rename] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/custom_frame_header.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/custom_frame_view_ash.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/custom_frame_view_ash.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/default_frame_header.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/default_frame_header.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/default_frame_header_unittest.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/frame_header.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/header_view.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/header_view.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/frame/wide_frame_view.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/public/cpp/mus_property_mirror_ash.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/public/cpp/window_properties.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/window_manager.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/wm/non_client_frame_controller.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ash/wm/panels/panel_frame_view.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[add] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/frame/DEPS
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/frame/browser_frame_mus.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/services/ui/public/interfaces/window_manager.mojom
[modify] https://crrev.com/5d480c704994ea82c4f3ca1fd94b1761668c6849/ui/views/mus/desktop_window_tree_host_mus.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 8 2018

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

commit e14e1aeddc34da3339c0e8a825376400061f7573
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue May 08 00:59:22 2018

Revert "Mash - Support for browser themes in window frame."

This reverts commit 5d480c704994ea82c4f3ca1fd94b1761668c6849.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 556549 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzVkNDgwYzcwNDk5NGVhODJjNGYzY2ExZmQ5NGIxNzYxNjY4YzY4NDkM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/5560

Sample Failed Step: browser_tests

Original change's description:
> Mash - Support for browser themes in window frame.
> 
> 1. Move BrowserFrameHeaderAsh to //ash/frame and rename to
>    CustomFrameHeader.
> 2. Use CustomFrameHeader for browser windows (in HeaderView).
>    Continue to use DefaultFrameHeader for other windows.
>    Possible TODO: share more code between the two FrameHeader classes.
> 3. Transport frame images over mojo so Mash can apply them
>    via CustomFrameHeader.
> 
> For now, only the active frame image is propagated (as a POC).
> The inactive image, overlay images, and frame colors are TODO.
> Ideally, there should be no behavioral changes in classic Ash.
> 
> BUG= 704942 , 836992 
> 
> Change-Id: I15b2d357bd3c8b60b6656ebce910d476ce107b44
> Reviewed-on: https://chromium-review.googlesource.com/1043221
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556549}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG= 704942 , 836992 

Change-Id: Ibe6fc13c61d33eac241fed00edb194cc248429de
Reviewed-on: https://chromium-review.googlesource.com/1049145
Cr-Commit-Position: refs/heads/master@{#556631}
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/BUILD.gn
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/custom_frame_view_ash.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/custom_frame_view_ash.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/default_frame_header.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/default_frame_header.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/default_frame_header_unittest.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/frame_header.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/header_view.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/header_view.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/frame/wide_frame_view.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/public/cpp/mus_property_mirror_ash.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/public/cpp/window_properties.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/window_manager.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/wm/non_client_frame_controller.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ash/wm/panels/panel_frame_view.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[delete] https://crrev.com/0b887fb9fe13e3a647ee2cc7b9a36c02beec3fd1/chrome/browser/ui/views/frame/DEPS
[rename] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_frame_header_ash.cc
[rename] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_frame_header_ash.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_frame_mus.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/services/ui/public/interfaces/window_manager.mojom
[modify] https://crrev.com/e14e1aeddc34da3339c0e8a825376400061f7573/ui/views/mus/desktop_window_tree_host_mus.cc

Project Member

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

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

commit 22800d9f3d0c979b6edff2727535b180081046c5
Author: Evan Stade <estade@chromium.org>
Date: Tue May 08 21:17:46 2018

Fix BrowserNonClientFrameViewAshTest.ToggleTabletModeOnMinimizedWindow

5d480c704994ea82c4f3ca1fd9 broke this test because it moved the code
that sets the icons from //chrome/ into //ash/. These targets
duplicate //ash/public/vector_icons (when using the component build),
so it's not valid to directly compare icon addresses. Compare by
identifier (name) instead.

Bug:  704942 , 836992 
Change-Id: I47a916a265c976afb61058be569283623d773c10
Reviewed-on: https://chromium-review.googlesource.com/1050379
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556960}
[modify] https://crrev.com/22800d9f3d0c979b6edff2727535b180081046c5/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 9 2018

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

commit 4a9711966b9d8f26dee3abf7a8f5454051ba4ab7
Author: Evan Stade <estade@chromium.org>
Date: Wed May 09 00:47:40 2018

Reland "Mash - Support for browser themes in window frame."

- originally landed as 5d480c704994ea82c
- reverted as e14e1aeddc34da33
- test failures fixed as 22800d9f3d0c979b

Original change description:
1. Move BrowserFrameHeaderAsh to //ash/frame and rename to
   CustomFrameHeader.
2. Use CustomFrameHeader for browser windows (in HeaderView).
   Continue to use DefaultFrameHeader for other windows.
   Possible TODO: share more code between the two FrameHeader classes.
3. Transport frame images over mojo so Mash can apply them
   via CustomFrameHeader.

For now, only the active frame image is propagated (as a POC).
The inactive image, overlay images, and frame colors are TODO.
Ideally, there should be no behavioral changes in classic Ash.

BUG= 704942 , 836992 
TBR=sky@chromium.org

Change-Id: Ic0763d3cebfe2a6f354d1345ed84ed02631e8210
Reviewed-on: https://chromium-review.googlesource.com/1050799
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557034}
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/BUILD.gn
[rename] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/custom_frame_header.cc
[rename] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/custom_frame_header.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/custom_frame_view_ash.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/custom_frame_view_ash.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/default_frame_header.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/default_frame_header.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/default_frame_header_unittest.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/frame_header.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/header_view.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/header_view.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/frame/wide_frame_view.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/public/cpp/mus_property_mirror_ash.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/public/cpp/window_properties.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/window_manager.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/wm/non_client_frame_controller.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ash/wm/panels/panel_frame_view.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[add] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/frame/DEPS
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/frame/browser_frame_mus.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/services/ui/public/interfaces/window_manager.mojom
[modify] https://crrev.com/4a9711966b9d8f26dee3abf7a8f5454051ba4ab7/ui/views/mus/desktop_window_tree_host_mus.cc

Status: Assigned (was: Available)
Status: WontFix (was: Assigned)
Obsolete. It looks fine after the MD2 top-chrome redesign.

Sign in to add a comment