New issue
Advanced search Search tips

Issue 836992 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 887051



Sign in to add a comment

Mash: browser themes in non-client area/frame

Project Member Reported by est...@chromium.org, Apr 25 2018

Issue description

In Mash, ash owns the non-client frame and the caption buttons within. Pointer events in the non-client area are handled by Ash. Chrome draws the client area, including the tabstrip, on top of the ash window, but doesn't draw anything over the frame. The problem is that custom browser themes need to draw either a color or an image, depending on the theme, but if Chrome does this right now it paints over the ash-provided caption buttons. (In addition to custom themes, we also aren't drawing the right color for incognito frames, see  bug 	704942 .)

Possible approaches:

1) change the Z-order so that the ash top level window draws on top of the client window. The only thing it should draw is the caption buttons.[i] Chrome is responsible for drawing the (possibly themed) background of the frame, as it is in classic ash.

2) send sufficient theme-related data over mojo to ash so that ash can draw the themed frame. This probably requires a lot of code duplication from BrowserFrameHeaderAsh.

3) make Chrome wholly responsible for the frame, making caption buttons usable from Chrome (currently they reach into ash::Shell for a couple things, but that should be possible to mojo-ify. For example, use of TableModeClient instead of TabletModeController).

Also note that Chrome creates extra caption buttons for some hosted app windows, including a back button and an app menu icon. Interaction with these is handled by Chrome, not Ash, so they can probably stay on Chrome side. Only tricky bit would be making sure that layout works even though some caption buttons are in one process and some in the other.

[i] This probably breaks something, but I couldn't immediately figure out how to test this as I had difficulty locating the code that controls client (chrome) vs. window manager (ash) window stacking.
 
Cc: msw@chromium.org
+msw

How does this work on Microsoft Windows? Does chrome do all its own caption button painting for browser windows?

Comment 2 by msw@chromium.org, Apr 26 2018

I'm out of touch with the latest details, but it looks like Chrome draws its own on Windows:
GlassBrowserFrameView uses chrome/browser/ui/views/frame/windows_10_caption_button.cc
OpaqueBrowserFrameView uses its own in OpaqueBrowserFrameView::InitWindowCaptionButton

I guess it would make sense for Chrome to draw everything, like on Win/Linux, but I'm not sure.

Comment 3 by sky@chromium.org, Apr 26 2018

I believe the caption buttons end up doing very ash specific things. So, if we went with Chrome entirely drawing them we would lose the behavior these buttons offer. For example, I believe they handle snapping. Similarly, we really want ash to handle dragging of the window.

Perhaps we need a way to for chrome to provide the location of the caption buttons to ash. An alternative is something similar to an NCCLIENT hit test like function.

Comment 4 by estade@google.com, Apr 26 2018

re: 1, on linux, windows and classic ash (not sure about mac), Chrome draws all its own buttons. (On Linux, there's the option to use a WM-provided frame, but then it's not nicely integrated with the tabstrip).

> if we went with Chrome entirely drawing them we would lose the behavior these buttons offer

Unless we port that behavior, i.e. PhantomWindowController, to mojo. But I agree it seems easiest to leave this up  to ash.

> Perhaps we need a way to for chrome to provide the location of the caption buttons to ash

I was imagining the opposite, i.e. Ash tells Chrome where the caption buttons are and Chrome does layout of the tabstrip or other buttons around that. Either way, any pointers for bullet point 1, i.e. how to get the host/toplevel window painting on top of rather than behind the client window?
Owner: est...@chromium.org
Status: Started (was: Untriaged)
after poking at this for a bit today, it seems that trying to layer the WM-side non client frame view on top of the embedded client window is harder than (2). BrowserFrameHeaderAsh should be possible to use from Ash with a few adjustments.
Project Member

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

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

commit 3cf20c1972f791f9c1ddc21e5673bad4592b118f
Author: Evan Stade <estade@chromium.org>
Date: Thu May 03 23:42:25 2018

Mash: remove //chrome dependencies from BrowserFrameHeaderAsh

This is a first step towards moving BrowserFrameHeaderAsh into
//ash/public and making it useable both in Ash (for Mash) and Chrome
(for classic).

Bug:  836992 
Change-Id: I76109ef3949f803e87c153008c0bf462e8228492
Reviewed-on: https://chromium-review.googlesource.com/1042786
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555908}
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/ash/frame/default_frame_header.cc
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/ash/frame/frame_header_util.cc
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/ash/frame/frame_header_util.h
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/browser_frame.cc
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/browser_frame.h
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/browser_frame_header_ash.cc
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/browser_frame_header_ash.h
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/3cf20c1972f791f9c1ddc21e5673bad4592b118f/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc

Project Member

Comment 7 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 8 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 9 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 10 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

Project Member

Comment 11 by bugdroid1@chromium.org, May 29 2018

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

commit 7aab45d415d995be506c0d1ac9e2bcc49793bf05
Author: Evan Stade <estade@chromium.org>
Date: Tue May 29 18:10:35 2018

Mash: get rid of redundant frame color init properties.

Just use ash::kFrameActiveColorKey/ash::kFrameInactiveColorKey.
HeaderView already listens to changes in these properties and
updates the header painter appropriately.

Test: files app, settings app, and chrome web store apps all have
correct frame colors with --enable-features=Mash

Bug:  836992 
Change-Id: I3dd4636ea0654b1b66c95a300b345facb9b76927
Reviewed-on: https://chromium-review.googlesource.com/1072426
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562507}
[modify] https://crrev.com/7aab45d415d995be506c0d1ac9e2bcc49793bf05/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/7aab45d415d995be506c0d1ac9e2bcc49793bf05/ash/wm/non_client_frame_controller.cc
[modify] https://crrev.com/7aab45d415d995be506c0d1ac9e2bcc49793bf05/ash/wm/property_util.cc
[modify] https://crrev.com/7aab45d415d995be506c0d1ac9e2bcc49793bf05/ash/wm/property_util.h
[modify] https://crrev.com/7aab45d415d995be506c0d1ac9e2bcc49793bf05/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/7aab45d415d995be506c0d1ac9e2bcc49793bf05/chrome/browser/ui/views/frame/browser_frame_mus.cc
[modify] https://crrev.com/7aab45d415d995be506c0d1ac9e2bcc49793bf05/services/ui/public/interfaces/window_manager.mojom

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 14 2018

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

commit f3c89c60f17ebddb705d5ac6094598a94a096b3b
Author: Evan Stade <estade@chromium.org>
Date: Thu Jun 14 04:44:12 2018

Mash: Create a way to share images without repeatedly sending via mojo.

Clients can send an image over mojo once and then reference it by its
unique token. Apply this to the browser frame theme image.

Bug:  836992 
Change-Id: I91bbb060eead928780dc3d4d78dda9e5ba1b96d3
Reviewed-on: https://chromium-review.googlesource.com/1088235
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567137}
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/BUILD.gn
[add] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/client_image_registry.cc
[add] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/client_image_registry.h
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/frame/header_view.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/manifest.json
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/mojo_interface_factory.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/public/cpp/window_properties.h
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/public/interfaces/client_image_registry.mojom
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/shell.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ash/shell.h
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/chrome/browser/ui/ash/browser_image_registrar.cc
[add] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/chrome/browser/ui/ash/browser_image_registrar.h
[add] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/chrome/browser/ui/ash/browser_image_registrar_unittest.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/chrome/browser/ui/views/frame/browser_non_client_frame_view_mash.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/chrome/browser/ui/views/frame/browser_non_client_frame_view_mash.h
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/chrome/test/BUILD.gn
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/services/ui/public/cpp/property_type_converters.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/services/ui/public/cpp/property_type_converters.h
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ui/aura/mus/property_converter.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ui/aura/mus/property_converter.h
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ui/gfx/image/image_skia.cc
[modify] https://crrev.com/f3c89c60f17ebddb705d5ac6094598a94a096b3b/ui/gfx/image/image_skia.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 15 2018

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

commit ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648
Author: Evan Stade <estade@chromium.org>
Date: Fri Jun 15 20:47:59 2018

More theming for Mash browser windows.

Pass inactive image, overlay images, frame colors over mojo.

For the frame colors, it's easiest to verify it's working with a
theme that doesn't have frame images but does set colors. These are
relatively rare but here's an example:
  https://chrome.google.com/webstore/detail/simple-silver-aero/cnjlfhkndhkliabjhkikpnmkadfcndnd

Bug:  836992 
Change-Id: I7141c6c79e1ee1229eb430e8178271ecc4673689
Reviewed-on: https://chromium-review.googlesource.com/1081161
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@{#567780}
[modify] https://crrev.com/ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648/ash/frame/header_view.cc
[modify] https://crrev.com/ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648/ash/public/cpp/mus_property_mirror_ash.cc
[modify] https://crrev.com/ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648/ash/public/cpp/window_properties.h
[modify] https://crrev.com/ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648/chrome/browser/ui/views/frame/browser_non_client_frame_view_mash.cc
[modify] https://crrev.com/ca43e3c1a88834a5e21f1f5aa1f40a1ce11f6648/chrome/browser/ui/views/frame/browser_non_client_frame_view_mash.h

Blockedon: 887051
Status: Fixed (was: Started)
Most of the above code got deleted because the browser now renders its own header. We're just using the code that already existed for classic Ash Chrome.

Sign in to add a comment