+malaykeshav who has been working on TopContainerView, and bruthig who did some ink droppy stuff for the bookmark bar. Can you provide any insight as to why this happens and what to do about it?

AFAICT, MenuButton sets its image to paint as a layer when it's hovered due to AddInkDropLayer(). This causes View::PaintChildren to not call Paint on the button, so no icon gets rendered.

How does this work for normal sites with the toolbar? How do the ink drops work there? (My guess is that since tabstrip_ is a child of TopContainerView, the layers render appropriately. BookmarkBarView seems to actually be reparented into the TopContainerView so that layer repaints are triggered correctly?)

My other hack solution is to just remove the ink drop in immersive mode.

Is there a way I can test this on my local machine? How do I install a PWA and open it in a window?

Sorry, here are the full steps:

1. Run with --enable-features=DesktopPWAWindowing
2. Go to https://www.pokedex.org/
3. Three-dot menu -> Add to shelf (may be under More Tools)
4. Click icon in the shelf.

Then follow the steps above.

+mohsen@ who owns the InkDrop now.

It's been a while since I worked on this, but even though View::PaintChildren() doesn't call paint on the icon a View should still be painting children that have layers.  I just can't recall the code that does this.

From the pic it appears the ink drop highlight is not visible either during hover?  Should it be?  These are the two options I would start looking in to.

- Are there any other layers in front of the ink drop/3-dot icon?  How is the TopContainerView implemented?  Is anything doing some custom Paint()/PaintChildren() calling?

- What are the locations of the ink drop and 3-dot icon when they aren't visible?  Is that the same as when they are visible?  You'll probably want to log them in screen space to be able to determine if they've changed.  My thoughts are something may not be updating the proper transforms when the 3-dot icon is made to paint to it's own layer.

You may find the following bread crumbs useful:
- ash::debug::PrintUIHierarchies()
- https://www.chromium.org/developers/how-tos/inspecting-ash

Hope that helps.

IMO a better hack solution would be to not make the icon paint to a layer when the ink drop is visible.

It seems that this is caused because cc decides to not paint any layer within BrowserNonClientFrameViewAsh. BrowserNonClientFrameViewAsh is the part that contains the title bar along with the frame caption buttons(close, minimize and maximize). Since this is hidden and never painted in immersive mode, cc optimizes and never calls paint on any layer within this hierarchy. 

What you see on mouse hover in immersive mode is actually the TopContainerView with the draw commands of BrowserNonClientFrameViewAsh (Frame Caption button + header menu button) appended to its Paint display item list. This is done here:
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/frame/top_container_view.cc?rcl=09bf90eebec2788aa255828a05d0d81a91ec2ea9&l=38
where we pass in the reference of the paint context(Display Item list) of the TopContainerView to the browser frame view's Paint() so that it can append its paint commands.

However this will only work for views. As soon as the recursive paint helper encounters a layer, it skips it because cc is responsible for calling the Paint() on them.
https://cs.chromium.org/chromium/src/ui/views/view.cc?dr=CSs&l=2022


How does this work for normal sites with the toolbar?
The toolbar is part of the TopContainerView, which is a part of the browser view. Since Browser view is visible during immersive mode, cc calls Paint() on all the layers within BrowserView. This includes the MenuButton on the toolbar.

A possible solution to make this work is to make the menu button a part of TopContainerView instead of BrowserNonClientFrameViewAsh. The TabStrip follows the same logic. It is a part of TopContainerView instead of BrowserNonClientFrameView, despite its bounds overlapping with the BrowserNonClientFrameView. 

The other solution is to use a custom non ink drop animation as FrameCaptionButton uses.
https://cs.chromium.org/chromium/src/ash/frame/caption_buttons/frame_caption_button.cc?dr=CSs&l=129

Hope this helps.
As bruthig@ mentioned, 
- ash::debug::PrintUIHierarchies()
- https://www.chromium.org/developers/how-tos/inspecting-ash
are very useful in debugging this.

CL out for review: https://chromium-review.googlesource.com/c/chromium/src/+/1004155

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

commit ce5d9da0f371ea1575542d2f574ecedd74b694ae
Author: Christopher Lam <calamity@chromium.org>
Date: Thu Apr 12 07:10:59 2018

[desktop-pwas] Disable animations and layers on buttons in immersive mode.

This CL fixes an issue where buttons were not visible in immersive mode
due to being painted into layers. This has been fixed by disabling the
layers when entering immersive mode, which disables animations, but keeps
the buttons visible.

Bug:  787640 
Change-Id: Ie34f71a30b7a3f995d01e11fdeb66ce49ab12c0f
Reviewed-on: https://chromium-review.googlesource.com/1004155
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550064}
[modify] https://crrev.com/ce5d9da0f371ea1575542d2f574ecedd74b694ae/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/ce5d9da0f371ea1575542d2f574ecedd74b694ae/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/ce5d9da0f371ea1575542d2f574ecedd74b694ae/chrome/browser/ui/views/frame/immersive_mode_controller.h
[modify] https://crrev.com/ce5d9da0f371ea1575542d2f574ecedd74b694ae/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/ce5d9da0f371ea1575542d2f574ecedd74b694ae/chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc

Note: This requires follow-up work because the CL was a bit of a hack. But the issue should now be fixed.

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

commit 780455c702a8e03dad789f2bc872ecdf833c14e1
Author: Ahmed Fakhry <afakhry@google.com>
Date: Wed May 16 17:46:51 2018

Fix caption buttons not showing on hover in immersive fullscreen mode

Due to ink drop effects on the caption buttons, they're painted to
layers. This doesn't work in immersive fullscreen mode and causes
the buttons to disappear. This CL fixes them by adding them temporarily
as children of the TopContainerView which is already painting to
layer in immersive mode, and is resposible for painting its sibling
BrowserNonClientFrameViewAsh.

BUG=840242, 787640 

Change-Id: I8353f3d393fc5113deb7776b742da64aa386c9a0
Reviewed-on: https://chromium-review.googlesource.com/1052675
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559170}
[modify] https://crrev.com/780455c702a8e03dad789f2bc872ecdf833c14e1/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/780455c702a8e03dad789f2bc872ecdf833c14e1/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/780455c702a8e03dad789f2bc872ecdf833c14e1/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/780455c702a8e03dad789f2bc872ecdf833c14e1/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/780455c702a8e03dad789f2bc872ecdf833c14e1/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/780455c702a8e03dad789f2bc872ecdf833c14e1/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/780455c702a8e03dad789f2bc872ecdf833c14e1/chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc

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

commit 7d71112e87ff4aa973f7cb873a468d5d7e917541
Author: Ahmed Fakhry <afakhry@chromium.org>
Date: Thu May 17 21:11:31 2018

Revert "Fix caption buttons not showing on hover in immersive fullscreen mode"

This reverts commit 780455c702a8e03dad789f2bc872ecdf833c14e1.

Reason for revert: The last patch broke apps where there's no tabstrip

Original change's description:
> Fix caption buttons not showing on hover in immersive fullscreen mode
> 
> Due to ink drop effects on the caption buttons, they're painted to
> layers. This doesn't work in immersive fullscreen mode and causes
> the buttons to disappear. This CL fixes them by adding them temporarily
> as children of the TopContainerView which is already painting to
> layer in immersive mode, and is resposible for painting its sibling
> BrowserNonClientFrameViewAsh.
> 
> BUG=840242, 787640 
> 
> Change-Id: I8353f3d393fc5113deb7776b742da64aa386c9a0
> Reviewed-on: https://chromium-review.googlesource.com/1052675
> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#559170}

TBR=jamescook@chromium.org,sky@chromium.org,estade@chromium.org,afakhry@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 840242,  787640 
Change-Id: I233204f80ec73b7fa06c955de05e29cf45fbf067
Reviewed-on: https://chromium-review.googlesource.com/1064771
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559679}
[modify] https://crrev.com/7d71112e87ff4aa973f7cb873a468d5d7e917541/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/7d71112e87ff4aa973f7cb873a468d5d7e917541/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/7d71112e87ff4aa973f7cb873a468d5d7e917541/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/7d71112e87ff4aa973f7cb873a468d5d7e917541/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/7d71112e87ff4aa973f7cb873a468d5d7e917541/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/7d71112e87ff4aa973f7cb873a468d5d7e917541/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/7d71112e87ff4aa973f7cb873a468d5d7e917541/chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc

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

commit 0f7c5244d79454520b5a93e2326117f1302cf2d7
Author: Ahmed Fakhry <afakhry@google.com>
Date: Mon May 21 17:33:38 2018

[Reland] Fix caption buttons not showing on hover in immersive fullscreen mode

Due to ink drop effects on the caption buttons, they're painted to
layers. This doesn't work in immersive fullscreen mode and causes
the buttons to disappear. This CL fixes them by adding them temporarily
as children of the TopContainerView which is already painting to
layer in immersive mode, and is resposible for painting its sibling
BrowserNonClientFrameViewAsh.

BUG=840242, 787640 
TEST=Added two new browser tests

Change-Id: Ia953fa0d742366540855b6e4b0655b76b3c823cd
Reviewed-on: https://chromium-review.googlesource.com/1065020
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560291}
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc
[modify] https://crrev.com/0f7c5244d79454520b5a93e2326117f1302cf2d7/testing/buildbot/filters/mash.browser_tests.filter