Issue metadata
Sign in to add a comment
|
Regression:The border outline for the 'Finance Toolbar' extension bubble is not seen properly
Reported by
vineetha...@etouch.net,
Mar 14 2018
|
||||||||||||||||||||||
Issue descriptionChrome Version: 67.0.3370.0 (Official Build) Revision 540244ed4952574c2bb88ac553397e47e5f03c4e-refs/heads/master@{#542909}(64 bit) OS: Mac 10.13.4(Mac Touchbar) OS Test URL: https://chrome.google.com/webstore/search/finance%20toolbar?hl=en-GB What steps will reproduce the problem? (1) Launch Chrome, navigate to above URL and add 'Finance Toolbar' extension to chrome. (2) Now click on the extension icon in omnibox, to open the bubble and observe. Actual Result: The border outline for the 'Finance Toolbar' extension bubble is not seen properly. Expected Result: The border outline for the 'Finance Toolbar' extension bubble should be seen properly. This is regression issue broken in ‘M-67’ and providing the bisect using per-revision bisect, Good build: 67.0.3362.0(Revision: 540778) Bad build: 67.0.3264.0(Revision: 541279) You are probably looking for a change made after 540879 (known good), but no later than 540880 (first known bad). CHANGE-LOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/7bea94e12121bc43c45d68779803b5628f7ebbf1..a0259b3ae165b0673416f02848bed75082d639f4 Suspect:https://chromium.googlesource.com/chromium/src/+/a0259b3ae165b0673416f02848bed75082d639f4 @robliao: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note: 1. The issue is not seen Windows(7,8,8.1,10), Mac(10.12.6,10.13.1) and Linux(14.04). Thank You!
,
Mar 16 2018
,
Mar 29 2018
Per https://cs.chromium.org/chromium/src/ui/views/bubble/bubble_border.h?rcl=2ed30ffa70000045d888892851a3e7d69b181a7b&l=113, the MacOS Window Server should be painting the shadows, so ExtensionPopup might be triggering some sort of bug. The repro is also very flaky and doesn't occur all of the time.
,
Apr 4 2018
,
Apr 4 2018
,
Apr 4 2018
Redirecting to ellyjones@ as I go OOO. Findings thus far: This appears to be a shadow invalidation problem in BridgedNativeWidget. Calling [window_ invalidateShadow] every time AcceleratedWidgetSwapCompleted() is called seems to fix up the shadow issue. The real question is if this is okay? The repro for the shadow only seems to happen most reliably on Release builds, making inspection under a debugger difficult. The sequence of events leading up to the loss of a shadow and the normal case appears to be the same with respect to the BridgedNativeWidget: SetBounds OnPositionChanged SetRootView SetFocusManager CreateLayer CreateCompositor AddCompositorSuperview InitCompositor UpdateLayerProperties ReorderChildViews ReorderChildViews ReorderChildViews ReorderChildViews SetAssociationForView ReorderChildViews SetNativeWindowProperty SetBounds OnSizeChanged UpdateLayerProperties OnBackingPropertiesChanged UpdateLayerProperties OnPositionChanged OnSizeConstraintsChanged SetBounds OnPositionChanged AcceleratedWidgetSwapCompleted SetVisibilityState OnWindowKeyStatusChangedTo OnVisibilityChanged OnVisibilityChanged NotifyVisibilityChangeDown OnVisibilityChanged OnPaintLayer SetBounds OnSizeChanged UpdateLayerProperties SetBounds OnSizeChanged UpdateLayerProperties AcceleratedWidgetSwapCompleted OnPaintLayer AcceleratedWidgetSwapCompleted <---- Generally doesn't require shadow invalidation ShouldDragWindow The quick bandaid is to always invalidate the shadow. If this is safe to always do, maybe we can just do that. Given that this bug is relatively hard to repro, we could reassess the triage decision of P1+RBS.
,
Apr 5 2018
weili@, can you take a peek please?
,
Apr 5 2018
If extension bubbles don't need to overlap the window (do they?) might be able to do something like this here: https://chromium-review.googlesource.com/c/chromium/src/+/735754
,
Apr 6 2018
Looks like the shadow doesn't reliably show/no show. I am looking into whether lgrey@'s suggestion would work for extension bubble.
,
Apr 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8de1e07f4f84b63ceca67736158649b0c1f7bf89 commit 8de1e07f4f84b63ceca67736158649b0c1f7bf89 Author: Wei Li <weili@chromium.org> Date: Tue Apr 10 16:47:33 2018 Use views border for ExtensionPopup ExtensionPopup used native windows server to draw its border beforei this change in the same way as Cocoa did. However, through hiding and showing the bubble, the shadow was not reliably drawn. Now we change to use views to draw its shadow. Other bubbles on Mac, as well as the extension popup on Cocoa, will still use native windows server to draw their shadows. BUG= 821739 Change-Id: Ie4fa4f4499e9feed1f266eb79ab878e7fee92ab4 Reviewed-on: https://chromium-review.googlesource.com/1003046 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Leonard Grey <lgrey@chromium.org> Commit-Queue: Wei Li <weili@chromium.org> Cr-Commit-Position: refs/heads/master@{#549561} [modify] https://crrev.com/8de1e07f4f84b63ceca67736158649b0c1f7bf89/chrome/browser/ui/views/extensions/extension_popup.cc [modify] https://crrev.com/8de1e07f4f84b63ceca67736158649b0c1f7bf89/ui/views/bubble/bubble_dialog_delegate.cc [modify] https://crrev.com/8de1e07f4f84b63ceca67736158649b0c1f7bf89/ui/views/bubble/bubble_dialog_delegate.h
,
Apr 10 2018
,
Apr 11 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by manoranj...@chromium.org
, Mar 16 2018