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

ui/views/widget/widget_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <memory>
 #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"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/hit_test.h"
 #include "ui/compositor/layer_animation_observer.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
@@ skipping 20 matching lines @@
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/view_prop.h"
 #include "ui/base/win/window_event_target.h"
 #include "ui/views/win/hwnd_util.h"
 #endif
 
 #if defined(OS_MACOSX)
 #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 {
 
 namespace {
 
 // TODO(tdanderson): This utility function is used in different unittest
 //                   files. Move to a common location to avoid
 //                   repeated code.
 gfx::Point ConvertPointFromWidgetToView(View* view, const gfx::Point& p) {
   gfx::Point tmp(p);
@@ skipping 3638 matching lines @@
 
   EXPECT_FALSE(!!IsWindow(owner_hwnd));
   EXPECT_FALSE(!!IsWindow(owned_hwnd));
   EXPECT_TRUE(!!IsWindowEnabled(top_hwnd));
 
   top_level_widget.CloseNow();
 }
 
 #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