From 38c39b06af1141f104b2c35a6975c7861d19ebbe Mon Sep 17 00:00:00 2001
From: "sadrul@chromium.org"
 <sadrul@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 13 Jul 2011 21:10:54 +0000
Subject: Add Widget::Observer for observing Widgets.

Use this new observation technique to avoid a crash in views-desktop when the
active widget is closed.

BUG=none
TEST=Run views_desktop, close a window, click, it doesn't crash!

Review URL: http://codereview.chromium.org/7342015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@92415 0039d316-1c4b-4281-b951-d872f2087c98
---
 views/desktop/desktop_window_view.cc  |  16 +++++
 views/desktop/desktop_window_view.h   |   9 ++-
 views/widget/native_widget_delegate.h |   3 +
 views/widget/native_widget_gtk.cc     |   2 +
 views/widget/native_widget_view.cc    |   5 ++
 views/widget/native_widget_view.h     |   1 +
 views/widget/native_widget_views.cc   |   3 +-
 views/widget/native_widget_win.cc     |   4 ++
 views/widget/widget.cc                |  23 ++++++-
 views/widget/widget.h                 |  17 +++++
 views/widget/widget_delegate.cc       |   3 -
 views/widget/widget_delegate.h        |   8 ---
 views/widget/widget_unittest.cc       | 115 ++++++++++++++++++++++++++++++++++
 13 files changed, 193 insertions(+), 16 deletions(-)

(limited to 'views')

diff --git a/views/desktop/desktop_window_view.cc b/views/desktop/desktop_window_view.cc
index 050c64e..e291bfc 100644
--- a/views/desktop/desktop_window_view.cc
+++ b/views/desktop/desktop_window_view.cc
@@ -124,6 +124,8 @@ void DesktopWindowView::ActivateWidget(Widget* widget) {
     widget->MoveToTop();
     active_widget_ = static_cast<NativeWidgetViews*>(widget->native_widget());
     active_widget_->OnActivate(true);
+    if (!widget->HasObserver(this))
+      widget->AddObserver(this);
   }
 }
 
@@ -189,5 +191,19 @@ View* DesktopWindowView::GetContentsView() {
   return this;
 }
 
+void DesktopWindowView::OnWidgetClosing(Widget* widget) {
+  if (active_widget_ && static_cast<internal::NativeWidgetPrivate*>
+      (active_widget_)->GetWidget() == widget)
+    active_widget_ = NULL;
+}
+
+void DesktopWindowView::OnWidgetVisibilityChanged(Widget* widget,
+                                                  bool visible) {
+}
+
+void DesktopWindowView::OnWidgetActivationChanged(Widget* widget,
+                                                  bool active) {
+}
+
 }  // namespace desktop
 }  // namespace views
diff --git a/views/desktop/desktop_window_view.h b/views/desktop/desktop_window_view.h
index 011f4be..b222f13 100644
--- a/views/desktop/desktop_window_view.h
+++ b/views/desktop/desktop_window_view.h
@@ -6,6 +6,7 @@
 #define VIEWS_DESKTOP_DESKTOP_WINDOW_H_
 
 #include "views/view.h"
+#include "views/widget/widget.h"
 #include "views/widget/widget_delegate.h"
 
 namespace views {
@@ -13,7 +14,8 @@ class NativeWidgetViews;
 
 namespace desktop {
 
-class DesktopWindowView : public WidgetDelegateView {
+class DesktopWindowView : public WidgetDelegateView,
+                          public Widget::Observer {
  public:
   static DesktopWindowView* desktop_window_view;
 
@@ -45,6 +47,11 @@ class DesktopWindowView : public WidgetDelegateView {
   virtual void WindowClosing() OVERRIDE;
   virtual View* GetContentsView() OVERRIDE;
 
+  // Overridden from Widget::Observer.
+  virtual void OnWidgetClosing(Widget* widget) OVERRIDE;
+  virtual void OnWidgetVisibilityChanged(Widget* widget, bool visible) OVERRIDE;
+  virtual void OnWidgetActivationChanged(Widget* widget, bool active) OVERRIDE;
+
   NativeWidgetViews* active_widget_;
 
   DISALLOW_COPY_AND_ASSIGN(DesktopWindowView);
diff --git a/views/widget/native_widget_delegate.h b/views/widget/native_widget_delegate.h
index 41b4246..20456fb 100644
--- a/views/widget/native_widget_delegate.h
+++ b/views/widget/native_widget_delegate.h
@@ -49,6 +49,9 @@ class NativeWidgetDelegate {
   virtual void OnNativeFocus(gfx::NativeView focused_view) = 0;
   virtual void OnNativeBlur(gfx::NativeView focused_view) = 0;
 
+  // Called when the window is shown/hidden.
+  virtual void OnNativeWidgetVisibilityChanged(bool visible) = 0;
+
   // Called when the native widget is created.
   virtual void OnNativeWidgetCreated() = 0;
 
diff --git a/views/widget/native_widget_gtk.cc b/views/widget/native_widget_gtk.cc
index e474f71..14a4f31 100644
--- a/views/widget/native_widget_gtk.cc
+++ b/views/widget/native_widget_gtk.cc
@@ -1196,6 +1196,7 @@ void NativeWidgetGtk::Show() {
     gtk_widget_show(widget_);
     if (widget_->window)
       gdk_window_raise(widget_->window);
+    delegate_->OnNativeWidgetVisibilityChanged(true);
   }
 }
 
@@ -1204,6 +1205,7 @@ void NativeWidgetGtk::Hide() {
     gtk_widget_hide(widget_);
     if (widget_->window)
       gdk_window_lower(widget_->window);
+    delegate_->OnNativeWidgetVisibilityChanged(false);
   }
 }
 
diff --git a/views/widget/native_widget_view.cc b/views/widget/native_widget_view.cc
index 8983ec6..9a2af69 100644
--- a/views/widget/native_widget_view.cc
+++ b/views/widget/native_widget_view.cc
@@ -118,6 +118,11 @@ bool NativeWidgetView::OnMouseWheel(const MouseWheelEvent& event) {
   return delegate()->OnMouseEvent(event);
 }
 
+void NativeWidgetView::VisibilityChanged(View* starting_from,
+                                         bool visible) {
+  delegate()->OnNativeWidgetVisibilityChanged(visible);
+}
+
 void NativeWidgetView::OnFocus() {
   // TODO(beng): check if we have to do this.
   //delegate()->OnNativeFocus(NULL);
diff --git a/views/widget/native_widget_view.h b/views/widget/native_widget_view.h
index a0e7fd4..c99d5fc 100644
--- a/views/widget/native_widget_view.h
+++ b/views/widget/native_widget_view.h
@@ -60,6 +60,7 @@ class NativeWidgetView : public View {
   virtual bool OnKeyPressed(const KeyEvent& event) OVERRIDE;
   virtual bool OnKeyReleased(const KeyEvent& event) OVERRIDE;
   virtual bool OnMouseWheel(const MouseWheelEvent& event) OVERRIDE;
+  virtual void VisibilityChanged(View* starting_from, bool is_visible) OVERRIDE;
   virtual void OnFocus() OVERRIDE;
   virtual void OnBlur() OVERRIDE;
   virtual std::string GetClassName() const OVERRIDE;
diff --git a/views/widget/native_widget_views.cc b/views/widget/native_widget_views.cc
index 2f1476d..0a03e77 100644
--- a/views/widget/native_widget_views.cc
+++ b/views/widget/native_widget_views.cc
@@ -307,6 +307,7 @@ bool NativeWidgetViews::IsVisible() const {
 }
 
 void NativeWidgetViews::Activate() {
+  // Enable WidgetObserverTest.ActivationChange when this is implemented.
   NOTIMPLEMENTED();
 }
 
@@ -358,7 +359,7 @@ void NativeWidgetViews::Minimize() {
 }
 
 bool NativeWidgetViews::IsMaximized() const {
-  NOTIMPLEMENTED();
+  // NOTIMPLEMENTED();
   return false;
 }
 
diff --git a/views/widget/native_widget_win.cc b/views/widget/native_widget_win.cc
index 684edec..98c080c 100644
--- a/views/widget/native_widget_win.cc
+++ b/views/widget/native_widget_win.cc
@@ -2065,6 +2065,10 @@ void NativeWidgetWin::OnWindowPosChanging(WINDOWPOS* window_pos) {
 void NativeWidgetWin::OnWindowPosChanged(WINDOWPOS* window_pos) {
   if (DidClientAreaSizeChange(window_pos))
     ClientAreaSizeChanged();
+  if (window_pos->flags & SWP_SHOWWINDOW)
+    delegate_->OnNativeWidgetVisibilityChanged(true);
+  else if (window_pos->flags & SWP_HIDEWINDOW)
+    delegate_->OnNativeWidgetVisibilityChanged(false);
   SetMsgHandled(FALSE);
 }
 
diff --git a/views/widget/widget.cc b/views/widget/widget.cc
index dcca28c..047cf05 100644
--- a/views/widget/widget.cc
+++ b/views/widget/widget.cc
@@ -293,6 +293,18 @@ gfx::NativeWindow Widget::GetNativeWindow() const {
   return native_widget_->GetNativeWindow();
 }
 
+void Widget::AddObserver(Widget::Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void Widget::RemoveObserver(Widget::Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+bool Widget::HasObserver(Widget::Observer* observer) {
+  return observers_.HasObserver(observer);
+}
+
 bool Widget::GetAccelerator(int cmd_id, ui::Accelerator* accelerator) {
   return false;
 }
@@ -732,9 +744,8 @@ void Widget::OnNativeWidgetActivationChanged(bool active) {
   if (!active)
     SaveWindowPosition();
 
-  // TODO(beng): merge these two.
-  widget_delegate_->OnWidgetActivated(active);
-  widget_delegate_->OnWindowActivationChanged(active);
+  FOR_EACH_OBSERVER(Observer, observers_,
+                    OnWidgetActivationChanged(this, active));
 }
 
 void Widget::OnNativeFocus(gfx::NativeView focused_view) {
@@ -749,6 +760,11 @@ void Widget::OnNativeBlur(gfx::NativeView focused_view) {
       focused_view);
 }
 
+void Widget::OnNativeWidgetVisibilityChanged(bool visible) {
+  FOR_EACH_OBSERVER(Observer, observers_,
+                    OnWidgetVisibilityChanged(this, visible));
+}
+
 void Widget::OnNativeWidgetCreated() {
   if (GetTopLevelWidget() == this) {
     // Only the top level Widget in a native widget hierarchy has a focus
@@ -766,6 +782,7 @@ void Widget::OnNativeWidgetCreated() {
 }
 
 void Widget::OnNativeWidgetDestroying() {
+  FOR_EACH_OBSERVER(Observer, observers_, OnWidgetClosing(this));
   if (non_client_view_)
     non_client_view_->WindowClosing();
   widget_delegate_->WindowClosing();
diff --git a/views/widget/widget.h b/views/widget/widget.h
index bc16530..99e706f 100644
--- a/views/widget/widget.h
+++ b/views/widget/widget.h
@@ -9,6 +9,7 @@
 #include <stack>
 
 #include "base/memory/scoped_ptr.h"
+#include "base/observer_list.h"
 #include "ui/base/accessibility/accessibility_types.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/rect.h"
@@ -89,6 +90,14 @@ class RootView;
 class Widget : public internal::NativeWidgetDelegate,
                public FocusTraversable {
  public:
+  // Observers can listen to various events on the Widgets.
+  class Observer {
+   public:
+    virtual void OnWidgetClosing(Widget* widget) {}
+    virtual void OnWidgetVisibilityChanged(Widget* widget, bool visible) {}
+    virtual void OnWidgetActivationChanged(Widget* widget, bool active) {}
+  };
+
   typedef std::set<Widget*> Widgets;
 
   enum FrameType {
@@ -221,6 +230,11 @@ class Widget : public internal::NativeWidgetDelegate,
   // TYPE_WINDOW.
   gfx::NativeWindow GetNativeWindow() const;
 
+  // Add/remove observer.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+  bool HasObserver(Observer* observer);
+
   // Returns the accelerator given a command id. Returns false if there is
   // no accelerator associated with a given id, which is a common condition.
   virtual bool GetAccelerator(int cmd_id, ui::Accelerator* accelerator);
@@ -521,6 +535,7 @@ class Widget : public internal::NativeWidgetDelegate,
   virtual void OnNativeWidgetActivationChanged(bool active) OVERRIDE;
   virtual void OnNativeFocus(gfx::NativeView focused_view) OVERRIDE;
   virtual void OnNativeBlur(gfx::NativeView focused_view) OVERRIDE;
+  virtual void OnNativeWidgetVisibilityChanged(bool visible) OVERRIDE;
   virtual void OnNativeWidgetCreated() OVERRIDE;
   virtual void OnNativeWidgetDestroying() OVERRIDE;
   virtual void OnNativeWidgetDestroyed() OVERRIDE;
@@ -591,6 +606,8 @@ class Widget : public internal::NativeWidgetDelegate,
 
   internal::NativeWidgetPrivate* native_widget_;
 
+  ObserverList<Observer> observers_;
+
   // Non-owned pointer to the Widget's delegate.  May be NULL if no delegate is
   // being used.
   WidgetDelegate* widget_delegate_;
diff --git a/views/widget/widget_delegate.cc b/views/widget/widget_delegate.cc
index f2f16b7..b302254 100644
--- a/views/widget/widget_delegate.cc
+++ b/views/widget/widget_delegate.cc
@@ -18,9 +18,6 @@ namespace views {
 WidgetDelegate::WidgetDelegate() : default_contents_view_(NULL) {
 }
 
-void WidgetDelegate::OnWidgetActivated(bool active) {
-}
-
 void WidgetDelegate::OnWidgetMove() {
 }
 
diff --git a/views/widget/widget_delegate.h b/views/widget/widget_delegate.h
index 6a6b254..8dc21bc 100644
--- a/views/widget/widget_delegate.h
+++ b/views/widget/widget_delegate.h
@@ -31,11 +31,6 @@ class WidgetDelegate {
  public:
   WidgetDelegate();
 
-  // Called whenever the widget is activated or deactivated.
-  // TODO(beng): This should be consolidated with
-  //             WindowDelegate::OnWindowActivationChanged().
-  virtual void OnWidgetActivated(bool active);
-
   // Called whenever the widget's position changes.
   virtual void OnWidgetMove();
 
@@ -127,9 +122,6 @@ class WidgetDelegate {
   // this time if necessary.
   virtual void DeleteDelegate() {}
 
-  // Called when the window's activation state changes.
-  virtual void OnWindowActivationChanged(bool active) {}
-
   // Called when the user begins/ends to change the bounds of the window.
   virtual void OnWindowBeginUserBoundsChange() {}
   virtual void OnWindowEndUserBoundsChange() {}
diff --git a/views/widget/widget_unittest.cc b/views/widget/widget_unittest.cc
index a6e219d..4710348 100644
--- a/views/widget/widget_unittest.cc
+++ b/views/widget/widget_unittest.cc
@@ -398,5 +398,120 @@ TEST_F(WidgetOwnershipTest,
   EXPECT_TRUE(state.native_widget_deleted);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Widget observer tests.
+//
+
+class WidgetObserverTest : public WidgetTest,
+                                  Widget::Observer {
+ public:
+  WidgetObserverTest()
+      : active_(NULL),
+        widget_closed_(NULL),
+        widget_activated_(NULL),
+        widget_shown_(NULL),
+        widget_hidden_(NULL) {
+  }
+
+  virtual ~WidgetObserverTest() {}
+
+  virtual void OnWidgetClosing(Widget* widget) OVERRIDE {
+    if (active_ == widget)
+      active_ = NULL;
+    widget_closed_ = widget;
+  }
+
+  virtual void OnWidgetActivationChanged(Widget* widget,
+                                         bool active) OVERRIDE {
+    if (active) {
+      widget_activated_ = widget;
+      active_ = widget;
+    } else
+      widget_deactivated_ = widget;
+  }
+
+  virtual void OnWidgetVisibilityChanged(Widget* widget,
+                                         bool visible) OVERRIDE {
+    if (visible)
+      widget_shown_ = widget;
+    else
+      widget_hidden_ = widget;
+  }
+
+  void reset() {
+    active_ = NULL;
+    widget_closed_ = NULL;
+    widget_activated_ = NULL;
+    widget_deactivated_ = NULL;
+    widget_shown_ = NULL;
+    widget_hidden_ = NULL;
+  }
+
+  Widget* NewWidget() {
+    Widget* widget = CreateChildNativeWidgetViews();
+    widget->AddObserver(this);
+    return widget;
+  }
+
+  const Widget* active() const { return active_; }
+  const Widget* widget_closed() const { return widget_closed_; }
+  const Widget* widget_activated() const { return widget_activated_; }
+  const Widget* widget_deactivated() const { return widget_deactivated_; }
+  const Widget* widget_shown() const { return widget_shown_; }
+  const Widget* widget_hidden() const { return widget_hidden_; }
+
+ private:
+
+  Widget* active_;
+
+  Widget* widget_closed_;
+  Widget* widget_activated_;
+  Widget* widget_deactivated_;
+  Widget* widget_shown_;
+  Widget* widget_hidden_;
+};
+
+// TODO: This test should be enabled when NativeWidgetViews::Activate is
+// implemented.
+TEST_F(WidgetObserverTest, DISABLED_ActivationChange) {
+  Widget* toplevel = CreateTopLevelPlatformWidget();
+  views_delegate.set_default_parent_view(toplevel->GetRootView());
+
+  Widget* child1 = NewWidget();
+  Widget* child2 = NewWidget();
+
+  reset();
+
+  child1->Activate();
+  EXPECT_EQ(child1, widget_activated());
+
+  child2->Activate();
+  EXPECT_EQ(child1, widget_deactivated());
+  EXPECT_EQ(child2, widget_activated());
+  EXPECT_EQ(child2, active());
+}
+
+TEST_F(WidgetObserverTest, VisibilityChange) {
+  Widget* toplevel = CreateTopLevelPlatformWidget();
+  views_delegate.set_default_parent_view(toplevel->GetRootView());
+
+  Widget* child1 = NewWidget();
+  Widget* child2 = NewWidget();
+
+  reset();
+
+  child1->Hide();
+  EXPECT_EQ(child1, widget_hidden());
+
+  child2->Hide();
+  EXPECT_EQ(child2, widget_hidden());
+
+  child1->Show();
+  EXPECT_EQ(child1, widget_shown());
+
+  child2->Show();
+  EXPECT_EQ(child2, widget_shown());
+}
+
 }  // namespace
 }  // namespace views
-- 
cgit v1.1