Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2552813006:
MacViews: Fix CompositingWidgetTest.Transparency_DesktopWidgetTranslucent (Closed)
DescriptionMacViews: Fix CompositingWidgetTest.Transparency_DesktopWidgetTranslucent
NativeWidgetMac::ShouldWindowContentsBeTransparent() always returns
false. The method is used by c/b/ui/views/tab_strip.cc when painting the
tab strip. When the Cocoa browser started supporting translucency around
the tab strip (using NSVisualEffectView) it was too subtle to require
changes around the painting of tabs, so this is probably going to remain
the case.
BUG=623421
Committed: https://crrev.com/4c69b77738d6120a4e347cdcc25714fbfc77dc13
Cr-Commit-Position: refs/heads/master@{#437131}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 15 (10 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== MacViews: Fix and re-enable WidgetTest.Transparency_DesktopWidgetTranslucent NativeWidgetMac::ShouldWindowContentsBeTransparent() always returns false. The method is used by c/b/ui/views/tab_strip.cc when painting the tab strip. When the Cocoa browser started supporting translucency around the tab strip (using NSVisualEffectView) it was too subtle to require changes around the painting of tabs, so this is probably going to remain the case. BUG=623421 ========== to ========== MacViews: Fix CompositingWidgetTest.Transparency_DesktopWidgetTranslucent NativeWidgetMac::ShouldWindowContentsBeTransparent() always returns false. The method is used by c/b/ui/views/tab_strip.cc when painting the tab strip. When the Cocoa browser started supporting translucency around the tab strip (using NSVisualEffectView) it was too subtle to require changes around the painting of tabs, so this is probably going to remain the case. BUG=623421 ==========
tapted@chromium.org changed reviewers: + sadrul@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2552813006/diff/1/ui/views/widget/widget_unit... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/2552813006/diff/1/ui/views/widget/widget_unit... ui/views/widget/widget_unittest.cc:3878: should_be_transparent); Should NativeWidgetMac::ShouldWindowContentsBeTransparent() be updated instead? Although from the comment above, it sounds like not?
Thanks! (I forgot to publish after optional bots reported in, but they're green [hooray \o/]) https://codereview.chromium.org/2552813006/diff/1/ui/views/widget/widget_unit... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/2552813006/diff/1/ui/views/widget/widget_unit... ui/views/widget/widget_unittest.cc:3878: should_be_transparent); On 2016/12/07 18:03:35, sadrul wrote: > Should NativeWidgetMac::ShouldWindowContentsBeTransparent() be updated instead? > Although from the comment above, it sounds like not? Yeah I thought about it, but I think NativeWidgetMac::ShouldWindowContentsBeTransparent() is returning the right thing for now. There's a bit more in the CL description about how tab_strip.cc uses it. Looking a the code in TabStrip::GetInactiveAlpha(), I think if I end up needing to change NativeWidgetMac::ShouldWindowContentsBeTransparent(), the right fix would be to rename the function to something like `GetNativeWindowFrameAlpha()`, and return a float (or [0,255], or an SkAlpha). (currently TabStrip::GetInactiveAlpha() relies on #if defined(USE_ASH))
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1481153809157720, "parent_rev":
"5f265fae4c6a7e622b23abe04ad525a30cd08c1f", "commit_rev":
"28b37eb7e69376f031d6947a83cc974fa5673a0f"}
Message was sent while issue was closed.
Description was changed from ========== MacViews: Fix CompositingWidgetTest.Transparency_DesktopWidgetTranslucent NativeWidgetMac::ShouldWindowContentsBeTransparent() always returns false. The method is used by c/b/ui/views/tab_strip.cc when painting the tab strip. When the Cocoa browser started supporting translucency around the tab strip (using NSVisualEffectView) it was too subtle to require changes around the painting of tabs, so this is probably going to remain the case. BUG=623421 ========== to ========== MacViews: Fix CompositingWidgetTest.Transparency_DesktopWidgetTranslucent NativeWidgetMac::ShouldWindowContentsBeTransparent() always returns false. The method is used by c/b/ui/views/tab_strip.cc when painting the tab strip. When the Cocoa browser started supporting translucency around the tab strip (using NSVisualEffectView) it was too subtle to require changes around the painting of tabs, so this is probably going to remain the case. BUG=623421 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== MacViews: Fix CompositingWidgetTest.Transparency_DesktopWidgetTranslucent NativeWidgetMac::ShouldWindowContentsBeTransparent() always returns false. The method is used by c/b/ui/views/tab_strip.cc when painting the tab strip. When the Cocoa browser started supporting translucency around the tab strip (using NSVisualEffectView) it was too subtle to require changes around the painting of tabs, so this is probably going to remain the case. BUG=623421 ========== to ========== MacViews: Fix CompositingWidgetTest.Transparency_DesktopWidgetTranslucent NativeWidgetMac::ShouldWindowContentsBeTransparent() always returns false. The method is used by c/b/ui/views/tab_strip.cc when painting the tab strip. When the Cocoa browser started supporting translucency around the tab strip (using NSVisualEffectView) it was too subtle to require changes around the painting of tabs, so this is probably going to remain the case. BUG=623421 Committed: https://crrev.com/4c69b77738d6120a4e347cdcc25714fbfc77dc13 Cr-Commit-Position: refs/heads/master@{#437131} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/4c69b77738d6120a4e347cdcc25714fbfc77dc13 Cr-Commit-Position: refs/heads/master@{#437131} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
