New issue
Advanced search Search tips

Issue 826472 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

[MacViews-Browser] Graphical issue appears when the Extension Icons are hidden in the Settings Icon Menu

Project Member Reported by meh...@chromium.org, Mar 27 2018

Issue description

Chrome Version: Version 67.0.3381.0
OS: macOS 10.12.6

What steps will reproduce the problem?
(1) Enable chrome://flags/#views-browser-windows
(2) Install some extensions
(3) Hide them in the Settings Icon Menu 
(4) Open the Settings Icon Menu

What is the expected result?
No graphical issue.

What happens instead?
A graphical issue is to see.

 
Bildschirmfoto 2018-03-27 um 22.16.22.png
89.3 KB View Download

Comment 1 by meh...@chromium.org, Mar 27 2018

Description: Show this description
Labels: Target-67 M-67 MacViews-Browser
Owner: ellyjo...@chromium.org
Status: Assigned (was: Untriaged)
Repros locally. I'll take this.

Comment 3 by gov...@chromium.org, Mar 29 2018

** Bulk Edit **

There are only two M67 dev releases left on 04/03 & 04/10 before M67 branch on 04/12. Please try to land the fix ASAP to trunk so we can move forward with 50%-50% experiment on M67 Canary/Dev (if possible at all). Thank you.
This one's odd.

I don't immediately understand how those pixels are not getting drawn - if I paint the background of MenuItemView, it's clearly within that area. Mac has the funny transparent menu background, though, so I wonder if that's affecting this...

Comment 5 by tapted@chromium.org, Mar 29 2018

This might be related to ToolkitViewsScrollWithLayers
Status: Started (was: Assigned)
#5: yep, you're right about that - disabling ToolkitViewsScrollWithLayers fixes this, and *enabling* ToolkitViewsScrollWithLayers causes a much worse version of this bug on other platforms.

It looks like the root cause is basically that ScrollView::SetContents() does a_view->SetPaintToLayer() while a_view may not be expecting to have to fill its entire layer, and the default behavior where layers are opaque leads to trouble. I see two approaches here:

1) Ensure that BrowserActionsContainer opaquely fills its bounds: that would require giving it an explicit background color (which requires knowing what that color should be - difficult since it is dependent on the menu's opacity/etc)
2) Do a_view->layer()->SetFillsBoundsOpaquely(false), but there are warnings in layer.h about causing unnecessary overdraw by doing this.

I'm heavily inclined towards (2), so I'll put up a CL that does that shortly.


Project Member

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

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

commit 6e4083439bbb6764ca940db97417358a15f75716
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Mon Apr 02 16:36:26 2018

views: have scrollview mark its contents as non-opaque

ScrollView, when using layers to scroll, imposes a layer on its contents;
doing so implies by default that the contents opaquely fill their entire
bounds. This is not true for many kinds of contents, so don't assume it to
avoid patches of un-drawn area.

Bug:  826472 
Change-Id: I1364c06f43241e6cecbc4eaddb20311047b89bdf
Reviewed-on: https://chromium-review.googlesource.com/987527
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547448}
[modify] https://crrev.com/6e4083439bbb6764ca940db97417358a15f75716/ui/views/controls/scroll_view.cc

Status: Fixed (was: Started)
Fix landed.
Labels: TE-Verified-M67 TE-Verified-67.0.3387.0
Able to reproduce the issue on chrome reported version Canary 67.0.3381.0
Verified the fix on Mac 10.12.6 on Chrome version #67.0.3387.0 as per the comment#0
Attaching screen cast for reference.
Observed "No graphical issue"
Hence, the fix is working as expected.
Adding the verified label.

Thanks!
826472.mp4
714 KB View Download

Sign in to add a comment