Make DesktopWindowTreeHostX11::ShouldWindowContentsBeTransparent() relies on argb visual flag

ui/views/widget/widget_unittest.cc

Index: ui/views/widget/widget_unittest.cc
diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc
index 8761f7f5cc83857e0164d4f81eb654e02b4c555d..40a125d55d18074317f2cb710124c4e8c5636f24 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -7,6 +7,7 @@
 #include <set>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
@@ -47,6 +48,11 @@
 #include "base/mac/mac_util.h"
 #endif
 
+#if defined(USE_X11) && !defined(OS_CHROMEOS)
+#include "ui/base/x/x11_util_internal.h"  // nogncheck
+#include "ui/gfx/x/x11_switches.h"  // nogncheck
+#endif
+
 namespace views {
 namespace test {
 
@@ -3705,5 +3711,76 @@ TEST_F(WidgetTest, WindowModalOwnerDestroyedEnabledTest) {
 
 #endif  // defined(OS_WIN)
 
+#if !defined(OS_CHROMEOS)
+namespace {
+
+// An observer that registers the transparent flag of a widget on creation.
+class WidgetTransparencyObserver : public WidgetObserver {
+ public:
+  WidgetTransparencyObserver() : count_(0), transparent_(false) {}
+  ~WidgetTransparencyObserver() override {}
+
+  bool transparent() { return transparent_; }
+
+  // WidgetObserver:
+  void OnWidgetCreated(Widget* widget) override {
+    EXPECT_EQ(count_, 0);
+    EXPECT_TRUE(widget->GetNativeWindow());
+
+    ++count_;
+    transparent_ =
+        WidgetTest::IsNativeWindowTransparent(widget->GetNativeWindow());
+
+#if defined(USE_X11)
+    EXPECT_TRUE(transparent_);
+#endif
+  }
+
+ private:
+  int count_;
+  bool transparent_;
+
+  DISALLOW_COPY_AND_ASSIGN(WidgetTransparencyObserver);
+};
+
+}  // namespace
+
+// Test to ensure that the aura Window remains transparent after its creation
+// has finished if transparent visuals are enabled.
+TEST_F(WidgetTest, Transparency_DesktopWidget) {
+#if defined(USE_X11)
+  // On Linux, transparent visuals is currently not activated by default.
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  command_line->AppendSwitch(switches::kEnableTransparentVisuals);
+
+  int depth = 0;
+  ui::ChooseVisualForWindow(NULL, &depth);
+

[sadrul, 2016/06/14 14:32:13]

nite: remove the empty line here.

[Julien Isorce Samsung, 2016/06/14 15:22:28]

Acknowledged.

+  EXPECT_EQ(depth, 32);
+#endif
+
+  Widget::InitParams init_params =
+      CreateParams(Widget::InitParams::TYPE_WINDOW);
+  init_params.show_state = ui::SHOW_STATE_NORMAL;
+  init_params.bounds = gfx::Rect(0, 0, 500, 500);
+  init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  Widget widget;
+  WidgetTransparencyObserver widget_observer;
+  init_params.native_widget =
+      CreatePlatformDesktopNativeWidgetImpl(init_params, &widget, nullptr);
+  widget.AddObserver(&widget_observer);
+  widget.Init(init_params);
+
+  EXPECT_EQ(widget_observer.transparent(),
+            IsNativeWindowTransparent(widget.GetNativeWindow()));
+  EXPECT_EQ(widget_observer.transparent(),
+            widget.ShouldWindowContentsBeTransparent());

[sadrul, 2016/06/14 14:32:13]

You can just check if the Widget is transparent here, right? You shouldn't need
the WidgetTransparencyObserver?

[Julien Isorce Samsung, 2016/06/14 15:22:28]

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.

+
+#if defined(USE_X11)
+  EXPECT_TRUE(widget_observer.transparent());

[sadrul, 2016/06/14 14:32:13]

Can you explain this expectation? Why should a Widget created normally be
transparent?

[Julien Isorce Samsung, 2016/06/14 15:22:28]

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.

+#endif
+}
+#endif  // !defined(OS_CHROMEOS)
+
 }  // namespace test
 }  // namespace views