aura: Some changes in Window and Desktop.

ui/aura/window_unittest.cc

Index: ui/aura/window_unittest.cc
diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc
index 1494ba7522a3a10caacd32b2609c30b4a2963a02..ef2170e3665c37507364a2aebdc6c78762a68690 100644
--- a/ui/aura/window_unittest.cc
+++ b/ui/aura/window_unittest.cc
@@ -6,6 +6,7 @@
 #include "base/compiler_specific.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/desktop.h"
+#include "ui/aura/desktop_observer.h"
 #include "ui/aura/event.h"
 #include "ui/aura/focus_manager.h"
 #include "ui/aura/hit_test.h"
@@ -14,6 +15,7 @@
 #include "ui/aura/test/test_desktop_delegate.h"
 #include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/window_delegate.h"
+#include "ui/aura/window_observer.h"
 #include "ui/gfx/canvas_skia.h"
 #include "ui/gfx/compositor/layer.h"
 #include "ui/base/keycodes/keyboard_codes.h"
@@ -760,5 +762,104 @@ TEST_F(WindowTest, Deactivate) {
   EXPECT_EQ(w2.get(), parent->children()[1]);
 }
 
+class ObserverTest : public WindowTest,
+                     public DesktopObserver,
+                     public WindowObserver {
+ public:
+  ObserverTest()
+      : active_(NULL),
+        shown_(NULL),
+        hidden_(NULL) {
+  }
+
+  virtual ~ObserverTest() {}
+
+  Window* active() const { return active_; }
+  Window* shown() const { return shown_; }
+  Window* hidden() const { return hidden_; }
+
+  void Reset() {
+    active_ = NULL;
+    shown_ = NULL;
+    hidden_ = NULL;
+  }
+
+ private:
+  virtual void SetUp() OVERRIDE {
+    WindowTest::SetUp();
+    Desktop::GetInstance()->AddObserver(this);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    Desktop::GetInstance()->RemoveObserver(this);
+    WindowTest::TearDown();
+  }
+
+  virtual void OnActiveWindowChanged(Window* active) OVERRIDE {
+    active_ = active;
+  }
+
+  virtual void OnWindowVisibilityChanged(Window* window) OVERRIDE {
+    if (window->IsVisible())
+      shown_ = window;
+    else
+      hidden_ = window;
+  }
+
+  Window* active_;
+  Window* shown_;
+  Window* hidden_;
+
+  DISALLOW_COPY_AND_ASSIGN(ObserverTest);
+};
+
+TEST_F(ObserverTest, WindowActivationObserve) {
+  scoped_ptr<Window> w1(CreateTestWindowWithId(1, NULL));
+  scoped_ptr<Window> w2(CreateTestWindowWithId(2, NULL));
+  scoped_ptr<Window> w3(CreateTestWindowWithId(3, w1.get()));
+
+  EXPECT_EQ(NULL, active());
+
+  w2->Activate();
+  EXPECT_EQ(w2.get(), active());
+
+  w3->Activate();
+  EXPECT_EQ(w2.get(), active());
+
+  w1->Activate();
+  EXPECT_EQ(w1.get(), active());
+}
+
+TEST_F(ObserverTest, WindowVisibilityObserve) {
+  scoped_ptr<Window> w1(CreateTestWindowWithId(1, NULL));
+  scoped_ptr<Window> w2(CreateTestWindowWithId(2, NULL));
+  scoped_ptr<Window> w3(CreateTestWindowWithId(3, w1.get()));
+
+  w1->AddObserver(this);
+  w2->AddObserver(this);
+  w3->AddObserver(this);
+
+  // All the windows are already shown. So showing them again won't trigger the
+  // observers.
+  Reset();
+  w1->Show();
+  w2->Show();
+  w3->Show();
+  EXPECT_FALSE(shown());
+
+  w1->Hide();
+  EXPECT_EQ(w1.get(), hidden());
+
+  Reset();
+  w3->Hide();

[sadrul, 2011/10/13 00:44:18]

This does not trigger the observer, because the parent window, w1, is already
hidden. Is this expectation correct?

[sky, 2011/10/13 00:51:08]

I think so. But it does mean you'll want to notify all descendants if an
ancestor is made visible/invisible.

[sadrul, 2011/10/13 02:24:54]

I personally think the observer should trigger whenever the Window's local
visibility state changes (not necessarily onscreen visibility). There was a very
brief discussion on this here:
http://codereview.chromium.org/8202026/diff/12010/ui/aura/window.cc#newcode373
This will be more in-line with how this works at least on GTK. It's of course
not necessary that we follow GTK, but in case there's consistency among other
toolkits (I do not know how this works on windows/other toolkits), perhaps it
would make sense for this to do the same.

Or an observer could just observe both local state change and onscreen
visibility change.

+  EXPECT_FALSE(hidden());
+
+  w1->Show();
+  EXPECT_EQ(w1.get(), shown());
+
+  w3->Show();
+  EXPECT_EQ(w3.get(), shown());
+}
+
 }  // namespace test
 }  // namespace aura