Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1998653002:
Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag (Closed)
|
Created:
4 years, 7 months ago by Julien Isorce Samsung Modified:
4 years, 5 months ago CC:
chromium-reviews, tfarina Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag
If depth is 32 and if there is a compositing manager then transparency is supported.
Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported.
Which is the same behavior as on OS_WIN.
Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent().
BUG=589509
R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org
TEST=views_unittests --gtest_filter=*WidgetTest.Transparency*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36
Committed: https://crrev.com/bd03f87c1657e4b15b0fed1627fdceb6c3519ae1
Cr-Original-Commit-Position: refs/heads/master@{#401826}
Cr-Commit-Position: refs/heads/master@{#401862}
Patch Set 1 #Patch Set 2 : Fix undefined on OSX. Also on OS_WIN do not assume that Aero Glass is enabled. #Patch Set 3 : Rebase and add nogncheck to fix chromecast and chromeos builds #Patch Set 4 : Just rebase #Patch Set 5 : When using X11 check if there is a transparent visual #Patch Set 6 : Just rebase #Patch Set 7 : Relax depth check since Xvfb only run as 24 color depth #Patch Set 8 : Actually no need to relax since Xvfb supports 32 depth, it just does not set _NET_WM_CM_S0 bit. #
Total comments: 1
Patch Set 9 : Add the fix in DesktopWindowTreeHostX11 that I mentioned in earlier comments #Patch Set 10 : Rebase #Patch Set 11 : Remove the fix to make the new test fails on the linux try bot #Patch Set 12 : Add assert to check that depth is 32 when kEnableTransparentVisuals is on #Patch Set 13 : Rebase #Patch Set 14 : Add more contraints in the test #Patch Set 15 : increment counter #Patch Set 16 : Rebase #Patch Set 17 : Rebase #Patch Set 18 : Re-add the fix #Patch Set 19 : Rebase and use USE_X11 instead of OS_LINUX #
Total comments: 2
Patch Set 20 : Rebase and use ShouldWindowContentsBeTransparent #
Total comments: 6
Patch Set 21 : Rebase and simplify the unit test #
Total comments: 2
Patch Set 22 : Add init_param.opacity = TRANSLUCENT_WINDOW #Patch Set 23 : Also test INFER_OPACITY and OPAQUE_WINDOW #Patch Set 24 : Add assert to check if compositing is enabled on OS_WIN #Patch Set 25 : Rebase and build fix on OS_WIN #Patch Set 26 : Rebase and build fix on OS_WIN #Patch Set 27 : Rebase and add TODO since AeroGlass is not enabled on OS_WIN #Patch Set 28 : Rebase and check for compositing manager #
Total comments: 2
Patch Set 29 : Use atom_cache_ #Patch Set 30 : Rebase #Patch Set 31 : Remove TODO/CHECK since Win10 bots can run tests with AeroGlass enabled #
Messages
Total messages: 184 (84 generated)
j.isorce@samsung.com changed reviewers: + jbauman@chromium.org, sadrul@chromium.org
Currently this CL does not contain the fix because I want to demonstrate that this test currently fails on Linux using try bots.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
On 2016/05/19 15:11:01, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) Let's try again the demonstration with Patch Set 2.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...) android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...) android_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) cast_shell_android on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...) chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddently turned off if it was not set on at the beginning. BUG=589509 R=jam@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* ========== to ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddently turned off if it was not set on at the beginning. BUG=589509 R=jam@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddently turned off if it was not set on at the beginning. BUG=589509 R=jam@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=jam@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
j.isorce@samsung.com changed reviewers: + piman@chromium.org
On 2016/05/20 18:17:04, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) Hi, as you can see the new test fails on that linux bot above. First the test checks that transparent visual flag is effective by checking depth == 32. Then the test saves what is the transparency state of the widget when the widget is created, see WidgetObserver::OnWidgetCreated. This saved state will be then accessible by calling widget_observer.transparent(). Then after returning from widget.Init the test compares the new transparency state. The following test failures shows that the window was transparent and then not: ../../ui/views/widget/widget_unittest.cc:3778: Failure Value of: IsNativeWindowTransparent(widget.GetNativeWindow()) Actual: false -->> new state after widget.Init Expected: widget_observer.transparent() Which is: true -->> recorded state from WidgetObserver::OnWidgetCreated Value of: widget.ShouldWindowContentsBeTransparent() Actual: false -->> bug, see below. The fix will be: +++ b/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc bool DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() const { - return false; + // Returning use_argb_visual_ ensures consistency with + // SetWindowTransparency() implementation. + return use_argb_visual_; } I plan to merge that fix into this CL of course but I wanted to highlight the failure without it Please have a look, thx.
https://codereview.chromium.org/1998653002/diff/140001/ui/base/x/x11_util.cc File ui/base/x/x11_util.cc (right): https://codereview.chromium.org/1998653002/diff/140001/ui/base/x/x11_util.cc#... ui/base/x/x11_util.cc:1431: // XAtom NET_WM_CM_S0 = XInternAtom(display, "_NET_WM_CM_S0", False); This is not right. The Composite extension may exist, but if there's no compositing WM (what the _NET_WM_CM_S0 selection indicates), then compositing will not work.
On 2016/05/20 20:40:39, piman wrote: > https://codereview.chromium.org/1998653002/diff/140001/ui/base/x/x11_util.cc#... > ui/base/x/x11_util.cc:1431: // XAtom NET_WM_CM_S0 = XInternAtom(display, > "_NET_WM_CM_S0", False); > This is not right. The Composite extension may exist, but if there's no > compositing WM (what the _NET_WM_CM_S0 selection indicates), then compositing > will not work. Hi Antoine, thx for you comment. Sorry for not having explained that change. This is just to workaround the fact that the existing atom check return false when using xvfb (=used by try bots) while in fact it can return a X Visual with 32 bit depths. I am not entirely sure at the moment whether it is a bug or not. But the idea here was to relax the check a bit so that the bots can really enable transparent visuals so that my new test can fail on the particular EXPECT_EQs I mentioned in the previous comment. On a real X11 server there is not need of this change to make the test shows that the transparency state is on then off. On my desktop the test just always fails.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/05/23 11:10:07, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. Now the CL contains the fix for the test I added. PTAL.
Description was changed from ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=jam@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=jam@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
j.isorce@samsung.com changed reviewers: + sky@chromium.org
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998653002/220001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/240001
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/280001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/300001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/320001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/340001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/06/06 14:57:30, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. In Patch Sets 12, 13, 14, 15, 16, and 17, the new test views_unittests::WidgetTest.Transparency_DesktopWidget fails on bot linux_chromium_rel_ng. In Patch Set 18, it now succeeds thank to the fix https://codereview.chromium.org/1998653002/diff2/320001:340001/ui/views/widge... which is the goal of this CL. (In Patch Set 11 it should have failed but it passed, though it was failing as expected on my machine. I tried to reproduce that false positive but then Patch Sets from 12 to 17 failed as expected) Note that the new test uses --enable-transparent-visuals which is effective since src/testing/xvfb.py now runs xcompmgr in addition to xvfb and openbox. So the CL is ready for review, PTAL. Thx.
jam@chromium.org changed reviewers: - jam@chromium.org
not sure why i was added as a reviewer, i'm not an owner in this area.
Description was changed from ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=jam@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
j.isorce@samsung.com changed reviewers: + erg@chromium.org
On 2016/06/06 21:58:48, jam wrote: > not sure why i was added as a reviewer, i'm not an owner in this area. No problem, I added @erg instead.
I'm not sure I understand what's going on here. So reading through the bug, you appear to want to make sure that the webkit content areas can be transparent. However this test is testing the views layer, which is chrome ui and not the output of the renderer. (IIUC. I only briefly touched transparency stuff a few years ago.)
j.isorce@samsung.com changed reviewers: + mgiuca@chromium.org
On 2016/06/07 22:38:44, Elliot Glaysher wrote:
> I'm not sure I understand what's going on here.
> So reading through the bug, you appear to want to make sure that the webkit
> content areas can be transparent.
You are right this CL is just one piece so I kept the same bug. For other pieces
I realized I can play with a theme extension (actually just a manifest.json).
But setting "nt_background" as [0, 0, 0, 0] in a theme extension has no effect
since the blink root graphic layer is opaque and above the background object
that use "COLOR_NTP_BACKGROUND".
> However this test is testing the views layer,
> which is chrome ui and not the output of the renderer.
>
The cc::Layer associated with blink::GraphicLayer::contentLayer() which is the
graphic layer returned by blink::PaintLayerCompositor::::rootGraphicsLayer()
("Overflow Controls Host Layer"), is the layer returned by
BrowserView::frame_->GetLayer(), and it is the layer for the window named
"BrowserFrameAura" (see GetNativeWindow()->SetName("BrowserFrameAura");
call from desktop_browser_frame_aura.cc::DesktopBrowserFrameAura's constructor.
So it is connected with webkit.
> (IIUC. I only briefly touched transparency stuff a few years ago.)
No problem, it seems that 2 years ago you added "use_argb_visual_" +
DesktopWindowTreeHostX11::SetWindowTransparency() in CL
https://codereview.chromium.org/266643003 .
The CL https://codereview.chromium.org/136093007 from @mgiuca added
DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() and
was merged some months before your CL.
Hi @mgiuca, what do you think of this fix ? Without it the new unit test fails.
Thx
On 2016/06/07 23:39:15, Julien Isorce wrote:
> On 2016/06/07 22:38:44, Elliot Glaysher wrote:
> > I'm not sure I understand what's going on here.
> > So reading through the bug, you appear to want to make sure that the webkit
> > content areas can be transparent.
>
> You are right this CL is just one piece so I kept the same bug. For other
pieces
> I realized I can play with a theme extension (actually just a manifest.json).
> But setting "nt_background" as [0, 0, 0, 0] in a theme extension has no effect
> since the blink root graphic layer is opaque and above the background object
> that use "COLOR_NTP_BACKGROUND".
>
> > However this test is testing the views layer,
> > which is chrome ui and not the output of the renderer.
> >
>
> The cc::Layer associated with blink::GraphicLayer::contentLayer() which is the
> graphic layer returned by blink::PaintLayerCompositor::::rootGraphicsLayer()
> ("Overflow Controls Host Layer"), is the layer returned by
> BrowserView::frame_->GetLayer(), and it is the layer for the window named
> "BrowserFrameAura" (see GetNativeWindow()->SetName("BrowserFrameAura");
> call from desktop_browser_frame_aura.cc::DesktopBrowserFrameAura's
constructor.
> So it is connected with webkit.
>
> > (IIUC. I only briefly touched transparency stuff a few years ago.)
>
> No problem, it seems that 2 years ago you added "use_argb_visual_" +
> DesktopWindowTreeHostX11::SetWindowTransparency() in CL
> https://codereview.chromium.org/266643003 .
>
> The CL https://codereview.chromium.org/136093007 from @mgiuca added
> DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() and
> was merged some months before your CL.
>
> Hi @mgiuca, what do you think of this fix ? Without it the new unit test
fails.
> Thx
Hi, hmm... this is not something I've done any work on for 2 years either so I'm
quite unfamiliar with it. I've had a bit of a look around. It seems reasonable
to have ShouldWindowContentsBeTransparent reflect the use_argb_visual_ field
that erg@ added after my CL. I'm not really sure what the consequences are,
though (this was originally designed to allow, IIRC, tabs to overlap the native
window title in Windows, which we don't need in Linux). Please make sure this
doesn't change any visuals in Linux, trying both on and off settings for "Use
System Title Bar and Borders" (on the right-click menu of the title bar). If
that works, I think this is fine.
I'm also a bit confused about what this change has to do with making the <body>
element background transparent in the renderer.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/360001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/06/09 01:29:27, Matt Giuca wrote: > Hi, hmm... this is not something I've done any work on for 2 years either so I'm > quite unfamiliar with it. I've had a bit of a look around. It seems reasonable > to have ShouldWindowContentsBeTransparent reflect the use_argb_visual_ field > that erg@ added after my CL. I'm not really sure what the consequences are, > though (this was originally designed to allow, IIRC, tabs to overlap the native > window title in Windows, which we don't need in Linux). Please make sure this > doesn't change any visuals in Linux, trying both on and off settings for "Use > System Title Bar and Borders" (on the right-click menu of the title bar). If > that works, I think this is fine. Thx Matt for your inputs. I have played with the tests you suggested on both intel and nouveau drivers, and I have not noticed any problem or any visual difference whether to pass --enable-transparent-visuals flag or not. > > I'm also a bit confused about what this change has to do with making the <body> > element background transparent in the renderer. I updated the bug title since it was wrong. Also I added a comment to describe the relation with this CL. But basically there is no problem with <body style="background: transparent;"> itself. On 2016/06/09 09:59:03, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. In the case you still agree with the CL, who should review it ? Thx in advance.
Description was changed from ========== Add WidgetTest.Transparency_DesktopWidget to widget_unittest.cc This new test check ensure that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Also add new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
> On 2016/06/09 09:59:03, commit-bot: I haz the power wrote: > > Dry run: This issue passed the CQ dry run. > > In the case you still agree with the CL, who should review it ? Thx in advance. The OWNERS file for ui/views/ says either sadrul@chromium.org or sky@chromium.org.
On 2016/06/09 17:30:50, Elliot Glaysher wrote: > > On 2016/06/09 09:59:03, commit-bot: I haz the power wrote: > > > Dry run: This issue passed the CQ dry run. > > > > In the case you still agree with the CL, who should review it ? Thx in > advance. > > The OWNERS file for ui/views/ says either mailto:sadrul@chromium.org or > mailto:sky@chromium.org. OK desktop_window_tree_host_x11.cc lgtm (but don't take that as a full code review of the whole CL).
https://codereview.chromium.org/1998653002/diff/360001/ui/views/widget/deskto... File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right): https://codereview.chromium.org/1998653002/diff/360001/ui/views/widget/deskto... ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:785: return use_argb_visual_; DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent() returns ShouldUseNativeFrame() here. Any reason to not want that here too? i.e. this should check both ShouldUseNativeFrame() && use_argb_visual_?
Description was changed from ========== Also add new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag Also add new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/380001
https://codereview.chromium.org/1998653002/diff/360001/ui/views/widget/deskto... File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right): https://codereview.chromium.org/1998653002/diff/360001/ui/views/widget/deskto... ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:785: return use_argb_visual_; On 2016/06/13 15:13:17, sadrul wrote: > DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent() returns > ShouldUseNativeFrame() here. Any reason to not want that here too? i.e. this > should check both ShouldUseNativeFrame() && use_argb_visual_? I think it should not rely on ShouldUseNativeFrame(). DesktopWindowTreeHostX11::SetWindowTransparency() does not rely on it whereas DesktopWindowTreeHostWin::SetWindowTransparency() does. But one could argue that DesktopWindowTreeHostX11::SetWindowTransparency() is wrong too. In any case your remark made me realized that I should also update IsTranslucentWindowOpacitySupported() and made DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on it.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/06/13 15:13:17, sadrul wrote: > https://codereview.chromium.org/1998653002/diff/360001/ui/views/widget/deskto... > ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:785: return > use_argb_visual_; > DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent() returns > ShouldUseNativeFrame() here. Any reason to not want that here too? i.e. this > should check both ShouldUseNativeFrame() && use_argb_visual_? Hi sadrul, thx again for the comment, I have a better answer than previously. I checked deeper and actually whether the flag use_native_frame_ is false or true, DesktopWindowTreeHostX11 still uses the xwindow_. It is just a matter of using os decoration or not: SetUseOSWindowFrame(XID window, bool use_os_window_frame) { // use_os_window_frame matches 'use_native_frame' flag. motif_hints.decorations = use_os_window_frame ? 1 : 0; XChangeProperty(window, motif_hints); } DesktopWindowTreeHostX11::SetUseNativeFrame(bool use_native_frame) { use_native_frame_ = use_native_frame; ui::SetUseOSWindowFrame(xwindow_, use_native_frame); } In other words, whether to pass true or false to SetUseNativeFrame, is just about using os decoration or not but still uses the xwindow_ in both cases. So for me, it does not make sense for IsTranslucentWindowOpacitySupported() (and ShouldWindowContentsBeTransparent()) to rely on whether the xwindow uses os decoration or not. It is really just a matter of use_argb_visual_ flag being true or false.
https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3758: nite: remove the empty line here. https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3777: widget.ShouldWindowContentsBeTransparent()); You can just check if the Widget is transparent here, right? You shouldn't need the WidgetTransparencyObserver? https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3780: EXPECT_TRUE(widget_observer.transparent()); Can you explain this expectation? Why should a Widget created normally be transparent?
https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3758: On 2016/06/14 14:32:13, sadrul wrote: > nite: remove the empty line here. Acknowledged. https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3777: widget.ShouldWindowContentsBeTransparent()); On 2016/06/14 14:32:13, sadrul wrote: > You can just check if the Widget is transparent here, right? You shouldn't need > the WidgetTransparencyObserver? That is true I could just reduce and keep: IsNativeWindowTransparent(widget.GetNativeWindow()) == widget.ShouldWindowContentsBeTransparent() after OnWidgetCreated is called. But the observer is there to note what was the value of IsNativeWindowTransparent(widget.GetNativeWindow()). Because without the fix (i.e. return use_argb_visual_), IsNativeWindowTransparent(widget.GetNativeWindow()) return true but then return false once widget.Init() entirely finishes. Indeed when calling widget.Init(), at some point it calls OnWidgetCreated. https://codereview.chromium.org/1998653002/diff/380001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3780: EXPECT_TRUE(widget_observer.transparent()); On 2016/06/14 14:32:13, sadrul wrote: > Can you explain this expectation? Why should a Widget created normally be > transparent? The function name is miss-leading, WidgetTransparencyObserver::transparent() should be renamed to NativeWindowTransparencyStatusAfterCreation(). But I think you are right, let's make the test easier, the observer was just there to demonstrate that previously the transparency status was inconsistent. I'll update the test and remove the observer.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/400001
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag Also add new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sky@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag Also add new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag Also add new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency is not suddenly turned off if it was set on at the beginning. BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
On 2016/06/14 15:22:28, Julien Isorce wrote: > I'll update the test and remove the observer. Done in Patch Set 21.
https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3740: widget.ShouldWindowContentsBeTransparent()); You should need to at least set Widget::InitParams::opacity to TRANSLUCENT_WINDOW?
https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... ui/views/widget/widget_unittest.cc:3740: widget.ShouldWindowContentsBeTransparent()); On 2016/06/14 15:45:31, sadrul wrote: > You should need to at least set Widget::InitParams::opacity to > TRANSLUCENT_WINDOW? Yes you are right. Somehow the test still succeeds with default value INFER_OPACITY and same with OPAQUE_WINDOW. (and with default value it seems to succeed on all platforms)
On 2016/06/14 16:02:40, Julien Isorce wrote: > https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... > File ui/views/widget/widget_unittest.cc (right): > > https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... > ui/views/widget/widget_unittest.cc:3740: > widget.ShouldWindowContentsBeTransparent()); > On 2016/06/14 15:45:31, sadrul wrote: > > You should need to at least set Widget::InitParams::opacity to > > TRANSLUCENT_WINDOW? > > Yes you are right. Somehow the test still succeeds with default value > INFER_OPACITY and same with OPAQUE_WINDOW. (and with default value it seems to > succeed on all platforms) I didn't suggest you update just the test. I was suggesting you should change the code so this doesn't return true unless TRANSLUCENT_WINDOW is set.
On 2016/06/14 16:11:06, sadrul wrote: > On 2016/06/14 16:02:40, Julien Isorce wrote: > > > https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... > > File ui/views/widget/widget_unittest.cc (right): > > > > > https://codereview.chromium.org/1998653002/diff/400001/ui/views/widget/widget... > > ui/views/widget/widget_unittest.cc:3740: > > widget.ShouldWindowContentsBeTransparent()); > > On 2016/06/14 15:45:31, sadrul wrote: > > > You should need to at least set Widget::InitParams::opacity to > > > TRANSLUCENT_WINDOW? > > > > Yes you are right. Somehow the test still succeeds with default value > > INFER_OPACITY and same with OPAQUE_WINDOW. (and with default value it seems to > > succeed on all platforms) > > I didn't suggest you update just the test. I was suggesting you should change > the code so this doesn't return true unless TRANSLUCENT_WINDOW is set. Ah sorry I got it wrong. Make sense. I'll also add another test to check the opposite. I am curious how other platforms behave on this.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/440001
On 2016/06/15 09:36:43, commit-bot: I haz the power wrote: > Dry run: CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1998653002/440001 I am first checking how behave other platforms regarding init_params.opacity vs ShouldWindowContentsBeTransparent() .
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
On 2016/06/15 10:31:54, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) It seems that Aero Glass is not enabled on bots.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/460001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/480001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/500001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/520001
On 2016/06/14 15:45:31, sadrul wrote: > ui/views/widget/widget_unittest.cc:3740: > widget.ShouldWindowContentsBeTransparent()); > You should need to at least set Widget::InitParams::opacity to > TRANSLUCENT_WINDOW? Hi sadrul, I wanted to use try bots to show that what you are pointed out is currently not required on OS_WIN. But unfortunately compositing is not enabled on win try bots: JOB_FAILED http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) error: Value of: ui::win::IsAeroGlassEnabled() Actual: false Expected: true So here are some extracted code: Common part: void DesktopNativeWidgetAura::UpdateWindowTransparency() { content_window_->SetTransparent(desktop_window_tree_host_->ShouldWindowContentsBeTransparent()); } OS_WIN: bool DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent() const { ui::win::IsAeroGlassEnabled(); // i.e. is compositing enabled. } OS_LINUX(with_path): bool DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() const { return use_argb_visual_; // i.e. is compositing enabled. } So I agree with you that it should be different result whether TRANSLUCENT_WINDOW or OPAQUE_WINDOW is set in init params, but for now I prefer to make behavior on OS_LINUX be the same as on OS_WIN for consistency (-> Patch Set 27). What do you think ? Thx.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/540001
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was unchecked by commit-bot@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/1998653002/diff/540001/ui/views/widget/deskto... File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right): https://codereview.chromium.org/1998653002/diff/540001/ui/views/widget/deskto... ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1152: XAtom NET_WM_CM_S0 = XInternAtom(xdisplay_, "_NET_WM_CM_S0", False); Use atom_cache_.GetAtom("_NET_WM_CM_S0") instead
Thx for the review. https://codereview.chromium.org/1998653002/diff/540001/ui/views/widget/deskto... File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right): https://codereview.chromium.org/1998653002/diff/540001/ui/views/widget/deskto... ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1152: XAtom NET_WM_CM_S0 = XInternAtom(xdisplay_, "_NET_WM_CM_S0", False); On 2016/06/24 03:55:13, sadrul wrote: > Use atom_cache_.GetAtom("_NET_WM_CM_S0") instead Done.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/580001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by j.isorce@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from mgiuca@chromium.org, sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/1998653002/#ps580001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/580001
Message was sent while issue was closed.
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #30 (id:580001)
Message was sent while issue was closed.
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Cr-Commit-Position: refs/heads/master@{#401826} ==========
Message was sent while issue was closed.
Patchset 30 (id:??) landed as https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Cr-Commit-Position: refs/heads/master@{#401826}
Message was sent while issue was closed.
A revert of this CL (patchset #30 id:580001) has been created in https://codereview.chromium.org/2086363005/ by blundell@chromium.org. The reason for reverting is: The unittests added in widget_unittest.cc fail on Win10. See the report with failure output here: https://chromium-swarm.appspot.com/user/task/2f9a5518a647e510 (scroll down to see the output of running the tests).
Message was sent while issue was closed.
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Cr-Commit-Position: refs/heads/master@{#401826} ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Cr-Commit-Position: refs/heads/master@{#401826} ==========
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998653002/600001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org, mgiuca@chromium.org Link to the patchset: https://codereview.chromium.org/1998653002/#ps600001 (title: "Remove TODO/CHECK since Win10 bots can run tests with AeroGlass enabled")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Cr-Commit-Position: refs/heads/master@{#401826} ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Cr-Commit-Position: refs/heads/master@{#401826} ==========
Message was sent while issue was closed.
Committed patchset #31 (id:600001)
Message was sent while issue was closed.
Description was changed from ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Cr-Commit-Position: refs/heads/master@{#401826} ========== to ========== Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag If depth is 32 and if there is a compositing manager then transparency is supported. Do not force transparency if Widget::InitParams::TRANSLUCENT_WINDOW is set whereas transparency is not supported. Which is the same behavior as on OS_WIN. Also add a new test WidgetTest.Transparency_DesktopWidget to widget_unittest.cc which ensures that transparency state of the native window matches what returns ShouldWindowContentsBeTransparent(). BUG=589509 R=erg@chromium.org, mgiuca@chromium.org, sadrul@chromium.org TEST=views_unittests --gtest_filter=*WidgetTest.Transparency* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/29d8c51452a83dcbc359679e8ce36af41af64e36 Committed: https://crrev.com/bd03f87c1657e4b15b0fed1627fdceb6c3519ae1 Cr-Original-Commit-Position: refs/heads/master@{#401826} Cr-Commit-Position: refs/heads/master@{#401862} ==========
Message was sent while issue was closed.
Patchset 31 (id:??) landed as https://crrev.com/bd03f87c1657e4b15b0fed1627fdceb6c3519ae1 Cr-Commit-Position: refs/heads/master@{#401862}
Message was sent while issue was closed.
Hi Julien, The tests you added in this CL are failing for me locally under xvfb and on the "CFI Linux" bot. Can you please take a look? https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux/builds/5924/st... $ cat out_gn/rel/args.gn # Build arguments go here. Examples: # is_component_build = true is_debug = false $ ninja -C out_gn/rel views_unittests $ xvfb-run -s "-screen 0 1024x768x24" out_gn/rel/views_unittests --single-process-tests --gtest_filter=WidgetTest.Transparency_DesktopWidgetInferOpacity Note: Google Test filter = WidgetTest.Transparency_DesktopWidgetInferOpacity [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from WidgetTest [ RUN ] WidgetTest.Transparency_DesktopWidgetInferOpacity Xlib: extension "RANDR" missing on display ":99". ../../ui/views/widget/widget_unittest.cc:3753: Failure Value of: 32 Expected: depth Which is: 24 ../../ui/views/widget/widget_unittest.cc:3765: Failure Value of: widget.IsTranslucentWindowOpacitySupported() Actual: false Expected: true [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity (31 ms) [----------] 1 test from WidgetTest (32 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test case ran. (32 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity 1 FAILED TEST
Message was sent while issue was closed.
On 2016/06/30 22:39:18, pcc1 wrote: > Hi Julien, > > The tests you added in this CL are failing for me locally under xvfb and on the > "CFI Linux" bot. Can you please take a look? > > https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux/builds/5924/st... > > $ cat out_gn/rel/args.gn > # Build arguments go here. Examples: > # is_component_build = true > is_debug = false > $ ninja -C out_gn/rel views_unittests > $ xvfb-run -s "-screen 0 1024x768x24" out_gn/rel/views_unittests > --single-process-tests > --gtest_filter=WidgetTest.Transparency_DesktopWidgetInferOpacity > Note: Google Test filter = WidgetTest.Transparency_DesktopWidgetInferOpacity > [==========] Running 1 test from 1 test case. > [----------] Global test environment set-up. > [----------] 1 test from WidgetTest > [ RUN ] WidgetTest.Transparency_DesktopWidgetInferOpacity > Xlib: extension "RANDR" missing on display ":99". > ../../ui/views/widget/widget_unittest.cc:3753: Failure > Value of: 32 > Expected: depth > Which is: 24 > ../../ui/views/widget/widget_unittest.cc:3765: Failure > Value of: widget.IsTranslucentWindowOpacitySupported() > Actual: false > Expected: true > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity (31 ms) > [----------] 1 test from WidgetTest (32 ms total) > > [----------] Global test environment tear-down > [==========] 1 test from 1 test case ran. (32 ms total) > [ PASSED ] 0 tests. > [ FAILED ] 1 test, listed below: > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity > > 1 FAILED TEST Hi, locally you need to use: python testing/xvfb.py since it requires a compositing manager xcompmgr. But I am surprised that it fails on that bot. Have you seen the failure on other bots ?
Message was sent while issue was closed.
On 2016/06/30 22:58:39, Julien Isorce wrote: > On 2016/06/30 22:39:18, pcc1 wrote: > > Hi Julien, > > > > The tests you added in this CL are failing for me locally under xvfb and on > the > > "CFI Linux" bot. Can you please take a look? > > > > > https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux/builds/5924/st... > > > > $ cat out_gn/rel/args.gn > > # Build arguments go here. Examples: > > # is_component_build = true > > is_debug = false > > $ ninja -C out_gn/rel views_unittests > > $ xvfb-run -s "-screen 0 1024x768x24" out_gn/rel/views_unittests > > --single-process-tests > > --gtest_filter=WidgetTest.Transparency_DesktopWidgetInferOpacity > > Note: Google Test filter = WidgetTest.Transparency_DesktopWidgetInferOpacity > > [==========] Running 1 test from 1 test case. > > [----------] Global test environment set-up. > > [----------] 1 test from WidgetTest > > [ RUN ] WidgetTest.Transparency_DesktopWidgetInferOpacity > > Xlib: extension "RANDR" missing on display ":99". > > ../../ui/views/widget/widget_unittest.cc:3753: Failure > > Value of: 32 > > Expected: depth > > Which is: 24 > > ../../ui/views/widget/widget_unittest.cc:3765: Failure > > Value of: widget.IsTranslucentWindowOpacitySupported() > > Actual: false > > Expected: true > > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity (31 ms) > > [----------] 1 test from WidgetTest (32 ms total) > > > > [----------] Global test environment tear-down > > [==========] 1 test from 1 test case ran. (32 ms total) > > [ PASSED ] 0 tests. > > [ FAILED ] 1 test, listed below: > > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity > > > > 1 FAILED TEST > > Hi, locally you need to use: python testing/xvfb.py since it requires > a compositing manager xcompmgr. > But I am surprised that it fails on that bot. Have you seen the failure > on other bots ? Okay, I can get the test to pass locally with the testing/xvfb.py script. I suspect that the bot is similar in that it isn't running a compositing manager. However, it seems strange to me that we would have tests that fail because of a missing compositing manager. Is there some reason why we can't disable these tests if a compositing manager is not active?
Message was sent while issue was closed.
On 2016/07/01 18:05:43, pcc1 wrote: > On 2016/06/30 22:58:39, Julien Isorce wrote: > > On 2016/06/30 22:39:18, pcc1 wrote: > > > Hi Julien, > > > > > > The tests you added in this CL are failing for me locally under xvfb and on > > the > > > "CFI Linux" bot. Can you please take a look? > > > > > > > > > https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux/builds/5924/st... > > > > > > $ cat out_gn/rel/args.gn > > > # Build arguments go here. Examples: > > > # is_component_build = true > > > is_debug = false > > > $ ninja -C out_gn/rel views_unittests > > > $ xvfb-run -s "-screen 0 1024x768x24" out_gn/rel/views_unittests > > > --single-process-tests > > > --gtest_filter=WidgetTest.Transparency_DesktopWidgetInferOpacity > > > Note: Google Test filter = WidgetTest.Transparency_DesktopWidgetInferOpacity > > > [==========] Running 1 test from 1 test case. > > > [----------] Global test environment set-up. > > > [----------] 1 test from WidgetTest > > > [ RUN ] WidgetTest.Transparency_DesktopWidgetInferOpacity > > > Xlib: extension "RANDR" missing on display ":99". > > > ../../ui/views/widget/widget_unittest.cc:3753: Failure > > > Value of: 32 > > > Expected: depth > > > Which is: 24 > > > ../../ui/views/widget/widget_unittest.cc:3765: Failure > > > Value of: widget.IsTranslucentWindowOpacitySupported() > > > Actual: false > > > Expected: true > > > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity (31 ms) > > > [----------] 1 test from WidgetTest (32 ms total) > > > > > > [----------] Global test environment tear-down > > > [==========] 1 test from 1 test case ran. (32 ms total) > > > [ PASSED ] 0 tests. > > > [ FAILED ] 1 test, listed below: > > > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity > > > > > > 1 FAILED TEST > > > > Hi, locally you need to use: python testing/xvfb.py since it requires > > a compositing manager xcompmgr. > > But I am surprised that it fails on that bot. Have you seen the failure > > on other bots ? > > Okay, I can get the test to pass locally with the testing/xvfb.py script. I > suspect that the bot is similar in that it isn't running a compositing manager. > > However, it seems strange to me that we would have tests that fail because of a > missing compositing manager. Is there some reason why we can't disable these > tests if a compositing manager is not active? You are right I will improve this. I will add you on CC when it is done.
Message was sent while issue was closed.
On 2016/07/04 16:49:20, Julien Isorce wrote: > On 2016/07/01 18:05:43, pcc1 wrote: > > On 2016/06/30 22:58:39, Julien Isorce wrote: > > > On 2016/06/30 22:39:18, pcc1 wrote: > > > > Hi Julien, > > > > > > > > The tests you added in this CL are failing for me locally under xvfb and > on > > > the > > > > "CFI Linux" bot. Can you please take a look? > > > > > > > > > > > > > > https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux/builds/5924/st... > > > > > > > > $ cat out_gn/rel/args.gn > > > > # Build arguments go here. Examples: > > > > # is_component_build = true > > > > is_debug = false > > > > $ ninja -C out_gn/rel views_unittests > > > > $ xvfb-run -s "-screen 0 1024x768x24" out_gn/rel/views_unittests > > > > --single-process-tests > > > > --gtest_filter=WidgetTest.Transparency_DesktopWidgetInferOpacity > > > > Note: Google Test filter = > WidgetTest.Transparency_DesktopWidgetInferOpacity > > > > [==========] Running 1 test from 1 test case. > > > > [----------] Global test environment set-up. > > > > [----------] 1 test from WidgetTest > > > > [ RUN ] WidgetTest.Transparency_DesktopWidgetInferOpacity > > > > Xlib: extension "RANDR" missing on display ":99". > > > > ../../ui/views/widget/widget_unittest.cc:3753: Failure > > > > Value of: 32 > > > > Expected: depth > > > > Which is: 24 > > > > ../../ui/views/widget/widget_unittest.cc:3765: Failure > > > > Value of: widget.IsTranslucentWindowOpacitySupported() > > > > Actual: false > > > > Expected: true > > > > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity (31 ms) > > > > [----------] 1 test from WidgetTest (32 ms total) > > > > > > > > [----------] Global test environment tear-down > > > > [==========] 1 test from 1 test case ran. (32 ms total) > > > > [ PASSED ] 0 tests. > > > > [ FAILED ] 1 test, listed below: > > > > [ FAILED ] WidgetTest.Transparency_DesktopWidgetInferOpacity > > > > > > > > 1 FAILED TEST > > > > > > Hi, locally you need to use: python testing/xvfb.py since it requires > > > a compositing manager xcompmgr. > > > But I am surprised that it fails on that bot. Have you seen the failure > > > on other bots ? > > > > Okay, I can get the test to pass locally with the testing/xvfb.py script. I > > suspect that the bot is similar in that it isn't running a compositing > manager. > > > > However, it seems strange to me that we would have tests that fail because of > a > > missing compositing manager. Is there some reason why we can't disable these > > tests if a compositing manager is not active? > > You are right I will improve this. I will add you on CC when it is done. https://codereview.chromium.org/2124633002/patch/80001/90010 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
