Test coverage for bug in WmWindowAura::OnWindowVisibilityChanged()

ash/common/wm_window_unittest.cc

Index: ash/common/wm_window_unittest.cc
diff --git a/ash/common/wm_window_unittest.cc b/ash/common/wm_window_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0b51b0e055023b1efe31d9d87cee2e709710cc8e
--- /dev/null
+++ b/ash/common/wm_window_unittest.cc
@@ -0,0 +1,70 @@
+// Copyright 2016 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 "ash/common/wm_window.h"
+
+#include <memory>
+
+#include "ash/common/test/ash_test.h"
+#include "ash/common/wm_window_observer.h"
+
+namespace ash {
+
+using WmWindowTest = AshTest;
+
+namespace {
+
+// Used to verify OnWindowVisibilityChanged() is called with a particular
+// window and value.
+class VisibilityObserver : public WmWindowObserver {
+ public:
+  // Attaches a WmWindowObserver to |window_to_add_observer_to| and sets
+  // |got_expected_value_| to true if OnWindowVisibilityChanged() is called
+  // with |expected_window| and |expected_value|.
+  VisibilityObserver(WmWindow* window_to_add_observer_to,
+                     WmWindow* expected_window,
+                     bool expected_value)
+      : window_to_add_observer_to_(window_to_add_observer_to),
+        expected_window_(expected_window),
+        expected_value_(expected_value),
+        got_expected_value_(false) {
+    window_to_add_observer_to_->AddObserver(this);
+  }
+  ~VisibilityObserver() override {
+    window_to_add_observer_to_->RemoveObserver(this);
+  }
+
+  bool got_expected_value() const { return got_expected_value_; }
+
+  // WmWindowObserver:
+  void OnWindowVisibilityChanged(WmWindow* window, bool visible) override {
+    if (window == expected_window_ && visible == expected_value_)
+      got_expected_value_ = true;
+  }
+
+ private:
+  WmWindow* window_to_add_observer_to_;
+  WmWindow* expected_window_;
+  const bool expected_value_;
+  bool got_expected_value_;
+
+  DISALLOW_COPY_AND_ASSIGN(VisibilityObserver);
+};
+
+}  // namespace
+
+// Verifies OnWindowVisibilityChanged() is called on a WmWindowObserver attached
+// to the parent when the child window's visibility changes.
+TEST_F(WmWindowTest, OnWindowVisibilityChangedCalledOnAncestor) {
+  std::unique_ptr<WindowOwner> window_owner = CreateTestWindow();
+  WmWindow* window = window_owner->window();
+  std::unique_ptr<WindowOwner> child_owner =
+      CreateChildWindow(window_owner->window());
+  WmWindow* child_window = child_owner->window();
+  VisibilityObserver observer(window, child_window, false);
+  child_window->Hide();
+  EXPECT_TRUE(observer.got_expected_value());

[James Cook, 2016/09/28 01:19:58]

optional: This might be easier to read if the VisibilityObserver just stored a
last_changed_window_ and last_changed_visible_ and the test did
EXPECT_EQ(child_window, observer.last_changed_window()) and
EXPECT_FALSE(observer.last_changed_visible())

Then I wouldn't have to think about the two WmWindow* parameters or what "false"
means on line 65.

(If this doesn't work because VisibilityObserver gets called multiple times and
you want to check if it hits your expectations at least once you should document
that. I just thought of that now.)

[sky, 2016/09/28 18:16:36]

Great idea! Done.

+}
+
+}  // namespace ash