aura: Fix WidgetFocusChangeListener::OnNativeFocusChange().

In Aura, we were passing the newly-focused view through
views::WidgetFocusManager::OnWidgetFocusEvent()'s
"focused_before" parameter.  This made us not close
extension popups when they lost the focus.

(Note that a popup still doesn't get closed upon clicking
outside of the popup on the desktop, presumably because the
popup retains the focus in Ash.)

BUG=132697
TEST=added, also manual testing


Review URL: https://chromiumcodereview.appspot.com/10562025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142858 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 175 insertions, 23 deletions

diff --git a/ui/aura/demo/demo_main.cc b/ui/aura/demo/demo_main.cc
index 530dc52..da57329a 100644
--- a/ui/aura/demo/demo_main.cc
+++ b/ui/aura/demo/demo_main.cc
@@ -39,7 +39,7 @@ class DemoWindowDelegate : public aura::WindowDelegate {
   }
   virtual void OnBoundsChanged(const gfx::Rect& old_bounds,
                                const gfx::Rect& new_bounds) OVERRIDE {}
-  virtual void OnFocus() OVERRIDE {}
+  virtual void OnFocus(aura::Window* old_focused_window) OVERRIDE {}
   virtual void OnBlur() OVERRIDE {}
   virtual bool OnKeyEvent(aura::KeyEvent* event) OVERRIDE {
     return false;
diff --git a/ui/aura/focus_manager.cc b/ui/aura/focus_manager.cc
index 475c0e3..fc963ec 100644
--- a/ui/aura/focus_manager.cc
+++ b/ui/aura/focus_manager.cc
@@ -48,7 +48,7 @@ void FocusManager::SetFocusedWindow(Window* focused_window,
   if (old_focused_window && old_focused_window->delegate())
     old_focused_window->delegate()->OnBlur();
   if (focused_window_ && focused_window_->delegate())
-    focused_window_->delegate()->OnFocus();
+    focused_window_->delegate()->OnFocus(old_focused_window);
   if (focused_window_) {
     FOR_EACH_OBSERVER(FocusChangeObserver, observers_,
                       OnWindowFocused(focused_window));
diff --git a/ui/aura/test/test_window_delegate.cc b/ui/aura/test/test_window_delegate.cc
index 3da06e3..4be062d 100644
--- a/ui/aura/test/test_window_delegate.cc
+++ b/ui/aura/test/test_window_delegate.cc
@@ -31,7 +31,7 @@ void TestWindowDelegate::OnBoundsChanged(const gfx::Rect& old_bounds,
                                          const gfx::Rect& new_bounds) {
 }
 
-void TestWindowDelegate::OnFocus() {
+void TestWindowDelegate::OnFocus(Window* old_focused_window) {
 }
 
 void TestWindowDelegate::OnBlur() {
diff --git a/ui/aura/test/test_window_delegate.h b/ui/aura/test/test_window_delegate.h
index bdf70d8..4eb8e2d 100644
--- a/ui/aura/test/test_window_delegate.h
+++ b/ui/aura/test/test_window_delegate.h
@@ -28,7 +28,7 @@ class TestWindowDelegate : public WindowDelegate {
   virtual gfx::Size GetMinimumSize() const OVERRIDE;
   virtual void OnBoundsChanged(const gfx::Rect& old_bounds,
                                const gfx::Rect& new_bounds) OVERRIDE;
-  virtual void OnFocus() OVERRIDE;
+  virtual void OnFocus(Window* old_focused_window) OVERRIDE;
   virtual void OnBlur() OVERRIDE;
   virtual bool OnKeyEvent(KeyEvent* event) OVERRIDE;
   virtual gfx::NativeCursor GetCursor(const gfx::Point& point) OVERRIDE;
diff --git a/ui/aura/window_delegate.h b/ui/aura/window_delegate.h
index 801d2cb..7908a19 100644
--- a/ui/aura/window_delegate.h
+++ b/ui/aura/window_delegate.h
@@ -37,7 +37,7 @@ class AURA_EXPORT WindowDelegate {
                                const gfx::Rect& new_bounds) = 0;
 
   // Sent to the Window's delegate when the Window gains or loses focus.
-  virtual void OnFocus() = 0;
+  virtual void OnFocus(aura::Window* old_focused_window) = 0;
   virtual void OnBlur() = 0;
 
   virtual bool OnKeyEvent(KeyEvent* event) = 0;
diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc
index b684d3b..96abaf6 100644
--- a/ui/aura/window_unittest.cc
+++ b/ui/aura/window_unittest.cc
@@ -192,6 +192,25 @@ class CaptureWindowDelegateImpl : public TestWindowDelegate {
   DISALLOW_COPY_AND_ASSIGN(CaptureWindowDelegateImpl);
 };
 
+// aura::WindowDelegate that tracks the window that was reported as having the
+// focus before us.
+class FocusDelegate : public TestWindowDelegate {
+ public:
+  FocusDelegate() : previous_focused_window_(NULL) {
+  }
+
+  aura::Window* previous_focused_window() const {
+    return previous_focused_window_;
+  }
+
+  virtual void OnFocus(aura::Window* old_focused_window) {
+    previous_focused_window_ = old_focused_window;
+  }
+
+ private:
+  aura::Window* previous_focused_window_;
+};
+
 // Keeps track of the location of the gesture.
 class GestureTrackPositionDelegate : public TestWindowDelegate {
  public:
@@ -1355,6 +1374,27 @@ TEST_F(WindowTest, FocusedWindowTest) {
   EXPECT_TRUE(parent->HasFocus());
 }
 
+// Tests that the previously-focused window is passed to
+// WindowDelegate::OnFocus().
+TEST_F(WindowTest, OldFocusedWindowTest) {
+  const gfx::Rect kBounds(0, 0, 100, 100);
+
+  FocusDelegate delegate1;
+  scoped_ptr<Window> window1(
+      CreateTestWindowWithDelegate(&delegate1, 0, kBounds, NULL));
+  window1->Focus();
+  ASSERT_TRUE(window1->HasFocus());
+  EXPECT_TRUE(delegate1.previous_focused_window() == NULL);
+
+  FocusDelegate delegate2;
+  scoped_ptr<Window> window2(
+      CreateTestWindowWithDelegate(&delegate2, 1, kBounds, NULL));
+  window2->Focus();
+  ASSERT_TRUE(window2->HasFocus());
+  EXPECT_FALSE(window1->HasFocus());
+  EXPECT_EQ(window1.get(), delegate2.previous_focused_window());
+}
+
 namespace {
 DEFINE_WINDOW_PROPERTY_KEY(int, kIntKey, -2);
 DEFINE_WINDOW_PROPERTY_KEY(const char*, kStringKey, "squeamish");
diff --git a/ui/views/focus/focus_manager_test.cc b/ui/views/focus/focus_manager_test.cc
index da15b27..4344f12 100644
--- a/ui/views/focus/focus_manager_test.cc
+++ b/ui/views/focus/focus_manager_test.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -14,7 +14,8 @@ namespace views {
 
 FocusManagerTest::FocusManagerTest()
     : contents_view_(new View),
-      focus_change_listener_(NULL) {
+      focus_change_listener_(NULL),
+      widget_focus_change_listener_(NULL) {
 }
 
 FocusManagerTest::~FocusManagerTest() {
@@ -38,6 +39,10 @@ void FocusManagerTest::SetUp() {
 void FocusManagerTest::TearDown() {
   if (focus_change_listener_)
     GetFocusManager()->RemoveFocusChangeListener(focus_change_listener_);
+  if (widget_focus_change_listener_) {
+    WidgetFocusManager::GetInstance()->RemoveFocusChangeListener(
+        widget_focus_change_listener_);
+  }
   GetWidget()->Close();
 
   // Flush the message loop to make application verifiers happy.
@@ -72,6 +77,13 @@ void FocusManagerTest::AddFocusChangeListener(FocusChangeListener* listener) {
   GetFocusManager()->AddFocusChangeListener(listener);
 }
 
+void FocusManagerTest::AddWidgetFocusChangeListener(
+    WidgetFocusChangeListener* listener) {
+  ASSERT_FALSE(widget_focus_change_listener_);
+  widget_focus_change_listener_ = listener;
+  WidgetFocusManager::GetInstance()->AddFocusChangeListener(listener);
+}
+
 #if defined(OS_WIN) && !defined(USE_AURA)
 void FocusManagerTest::SimulateActivateWindow() {
   SendMessage(GetWidget()->GetNativeWindow(), WM_ACTIVATE, WA_ACTIVE, NULL);
@@ -111,4 +123,23 @@ void TestFocusChangeListener::ClearFocusChanges() {
   focus_changes_.clear();
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// TestWidgetFocusChangeListener
+
+TestWidgetFocusChangeListener::TestWidgetFocusChangeListener() {
+}
+
+TestWidgetFocusChangeListener::~TestWidgetFocusChangeListener() {
+}
+
+void TestWidgetFocusChangeListener::ClearFocusChanges() {
+  focus_changes_.clear();
+}
+
+void TestWidgetFocusChangeListener::OnNativeFocusChange(
+    gfx::NativeView focused_before,
+    gfx::NativeView focused_now) {
+  focus_changes_.push_back(NativeViewPair(focused_before, focused_now));
+}
+
 }  // namespace views
diff --git a/ui/views/focus/focus_manager_test.h b/ui/views/focus/focus_manager_test.h
index 188ad23..f2c0102 100644
--- a/ui/views/focus/focus_manager_test.h
+++ b/ui/views/focus/focus_manager_test.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -6,6 +6,7 @@
 #define UI_VIEWS_FOCUS_FOCUS_MANAGER_TEST_H_
 
 #include "ui/views/focus/focus_manager.h"
+#include "ui/views/focus/widget_focus_manager.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget_delegate.h"
 
@@ -37,6 +38,7 @@ class FocusManagerTest : public ViewsTestBase,
   virtual void InitContentView();
 
   void AddFocusChangeListener(FocusChangeListener* listener);
+  void AddWidgetFocusChangeListener(WidgetFocusChangeListener* listener);
 
 #if defined(OS_WIN) && !defined(USE_AURA)
   // Mocks activating/deactivating the window.
@@ -50,6 +52,7 @@ class FocusManagerTest : public ViewsTestBase,
  private:
   View* contents_view_;
   FocusChangeListener* focus_change_listener_;
+  WidgetFocusChangeListener* widget_focus_change_listener_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusManagerTest);
 };
@@ -78,6 +81,31 @@ class TestFocusChangeListener : public FocusChangeListener {
   DISALLOW_COPY_AND_ASSIGN(TestFocusChangeListener);
 };
 
+typedef std::pair<gfx::NativeView, gfx::NativeView> NativeViewPair;
+
+// Use to record widget focus change notifications.
+class TestWidgetFocusChangeListener : public WidgetFocusChangeListener {
+ public:
+  TestWidgetFocusChangeListener();
+  virtual ~TestWidgetFocusChangeListener();
+
+  const std::vector<NativeViewPair>& focus_changes() const {
+    return focus_changes_;
+  }
+  void ClearFocusChanges();
+
+  // Overridden from WidgetFocusChangeListener:
+  virtual void OnNativeFocusChange(gfx::NativeView focused_before,
+                                   gfx::NativeView focused_now) OVERRIDE;
+
+ private:
+  // Pairs of (focused_before, focused_now) parameters we've received via calls
+  // to OnNativeFocusChange(), in oldest-to-newest-received order.
+  std::vector<NativeViewPair> focus_changes_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestWidgetFocusChangeListener);
+};
+
 }  // namespace views
 
 #endif  // UI_VIEWS_FOCUS_FOCUS_MANAGER_TEST_H_
diff --git a/ui/views/focus/focus_manager_unittest.cc b/ui/views/focus/focus_manager_unittest.cc
index b121e44..c545bb8 100644
--- a/ui/views/focus/focus_manager_unittest.cc
+++ b/ui/views/focus/focus_manager_unittest.cc
@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <utility>
+#include <vector>
+
 #include "base/utf_string_conversions.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/keycodes/keyboard_codes.h"
@@ -10,6 +13,7 @@
 #include "ui/views/focus/accelerator_handler.h"
 #include "ui/views/focus/focus_manager_factory.h"
 #include "ui/views/focus/focus_manager_test.h"
+#include "ui/views/focus/widget_focus_manager.h"
 #include "ui/views/widget/widget.h"
 
 #if !defined(USE_AURA)
@@ -115,6 +119,41 @@ TEST_F(FocusManagerTest, FocusChangeListener) {
   EXPECT_TRUE(listener.focus_changes()[0] == ViewPair(view2, null_view));
 }
 
+TEST_F(FocusManagerTest, WidgetFocusChangeListener) {
+  TestWidgetFocusChangeListener widget_listener;
+  AddWidgetFocusChangeListener(&widget_listener);
+
+  Widget::InitParams params;
+  params.type = views::Widget::InitParams::TYPE_WINDOW;
+  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.bounds = gfx::Rect(10, 10, 100, 100);
+  params.parent_widget = GetWidget();
+
+  scoped_ptr<Widget> widget1(new Widget);
+  widget1->Init(params);
+  widget1->Show();
+
+  scoped_ptr<Widget> widget2(new Widget);
+  widget2->Init(params);
+  widget2->Show();
+
+  widget_listener.ClearFocusChanges();
+  gfx::NativeView native_view1 = widget1->GetNativeView();
+  GetWidget()->FocusNativeView(native_view1);
+  ASSERT_EQ(2, static_cast<int>(widget_listener.focus_changes().size()));
+  EXPECT_EQ(native_view1, widget_listener.focus_changes()[0].second);
+  EXPECT_EQ(native_view1, widget_listener.focus_changes()[1].second);
+
+  widget_listener.ClearFocusChanges();
+  gfx::NativeView native_view2 = widget2->GetNativeView();
+  GetWidget()->FocusNativeView(native_view2);
+  ASSERT_EQ(2, static_cast<int>(widget_listener.focus_changes().size()));
+  EXPECT_EQ(NativeViewPair(native_view1, native_view2),
+            widget_listener.focus_changes()[0]);
+  EXPECT_EQ(NativeViewPair(native_view1, native_view2),
+            widget_listener.focus_changes()[1]);
+}
+
 #if !defined(USE_AURA)
 class TestTextfield : public Textfield {
  public:
diff --git a/ui/views/widget/native_widget_aura.cc b/ui/views/widget/native_widget_aura.cc
index b169f74..76761ec 100644
--- a/ui/views/widget/native_widget_aura.cc
+++ b/ui/views/widget/native_widget_aura.cc
@@ -149,6 +149,7 @@ NativeWidgetAura::NativeWidgetAura(internal::NativeWidgetDelegate* delegate)
       ownership_(Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET),
       ALLOW_THIS_IN_INITIALIZER_LIST(close_widget_factory_(this)),
       can_activate_(true),
+      destroying_(false),
       cursor_(gfx::kNullCursor),
       saved_window_state_(ui::SHOW_STATE_DEFAULT) {
 }
@@ -700,17 +701,26 @@ void NativeWidgetAura::OnBoundsChanged(const gfx::Rect& old_bounds,
     delegate_->OnNativeWidgetSizeChanged(new_bounds.size());
 }
 
-void NativeWidgetAura::OnFocus() {
+void NativeWidgetAura::OnFocus(aura::Window* old_focused_window) {
   // In aura, it is possible for child native widgets to take input and focus,
   // this differs from the behavior on windows.
   GetWidget()->GetInputMethod()->OnFocus();
-  delegate_->OnNativeFocus(window_);
+  delegate_->OnNativeFocus(old_focused_window);
 }
 
 void NativeWidgetAura::OnBlur() {
-  // Not only top level native widget can take input and focus, child
-  // widgets are allowed also.
-  GetWidget()->GetInputMethod()->OnBlur();
+  // GetInputMethod() recreates the input method if it's previously been
+  // destroyed.  If we get called during destruction, the input method will be
+  // gone, and creating a new one and telling it that we lost the focus will
+  // trigger a DCHECK (the new input method doesn't think that we have the focus
+  // and doesn't expect a blur).  OnBlur() shouldn't be called during
+  // destruction unless WIDGET_OWNS_NATIVE_WIDGET is set (which is just the case
+  // in tests).
+  if (!destroying_)
+    GetWidget()->GetInputMethod()->OnBlur();
+  else
+    DCHECK_EQ(ownership_, Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET);
+
   delegate_->OnNativeBlur(window_->GetFocusManager()->GetFocusedWindow());
 }
 
@@ -902,6 +912,7 @@ int NativeWidgetAura::OnPerformDrop(const aura::DropTargetEvent& event) {
 // NativeWidgetAura, protected:
 
 NativeWidgetAura::~NativeWidgetAura() {
+  destroying_ = true;
   if (ownership_ == Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET)
     delete delegate_;
   else
diff --git a/ui/views/widget/native_widget_aura.h b/ui/views/widget/native_widget_aura.h
index 57dbe27..f4c177f 100644
--- a/ui/views/widget/native_widget_aura.h
+++ b/ui/views/widget/native_widget_aura.h
@@ -131,7 +131,7 @@ class VIEWS_EXPORT NativeWidgetAura : public internal::NativeWidgetPrivate,
   virtual gfx::Size GetMinimumSize() const OVERRIDE;
   virtual void OnBoundsChanged(const gfx::Rect& old_bounds,
                                const gfx::Rect& new_bounds) OVERRIDE;
-  virtual void OnFocus() OVERRIDE;
+  virtual void OnFocus(aura::Window* old_focused_window) OVERRIDE;
   virtual void OnBlur() OVERRIDE;
   virtual bool OnKeyEvent(aura::KeyEvent* event) OVERRIDE;
   virtual gfx::NativeCursor GetCursor(const gfx::Point& point) OVERRIDE;
@@ -189,6 +189,9 @@ class VIEWS_EXPORT NativeWidgetAura : public internal::NativeWidgetPrivate,
   // Can we be made active?
   bool can_activate_;
 
+  // Are we in the destructor?
+  bool destroying_;
+
   gfx::NativeCursor cursor_;
 
   // The saved window state for exiting full screen state.
diff --git a/ui/views/widget/native_widget_win.cc b/ui/views/widget/native_widget_win.cc
index 402d53f..2fcf057 100644
--- a/ui/views/widget/native_widget_win.cc
+++ b/ui/views/widget/native_widget_win.cc
@@ -2022,8 +2022,8 @@ LRESULT NativeWidgetWin::OnSetCursor(UINT message,
   return 0;
 }
 
-void NativeWidgetWin::OnSetFocus(HWND focused_window) {
-  delegate_->OnNativeFocus(focused_window);
+void NativeWidgetWin::OnSetFocus(HWND old_focused_window) {
+  delegate_->OnNativeFocus(old_focused_window);
   InputMethod* input_method = GetWidget()->GetInputMethodDirect();
   if (input_method)
     input_method->OnFocus();
diff --git a/ui/views/widget/native_widget_win.h b/ui/views/widget/native_widget_win.h
index 868c24b..b02b3b2 100644
--- a/ui/views/widget/native_widget_win.h
+++ b/ui/views/widget/native_widget_win.h
@@ -427,7 +427,7 @@ class VIEWS_EXPORT NativeWidgetWin : public ui::WindowImpl,
   virtual LRESULT OnPowerBroadcast(DWORD power_event, DWORD data);
   virtual LRESULT OnReflectedMessage(UINT msg, WPARAM w_param, LPARAM l_param);
   virtual LRESULT OnSetCursor(UINT message, WPARAM w_param, LPARAM l_param);
-  virtual void OnSetFocus(HWND focused_window);
+  virtual void OnSetFocus(HWND old_focused_window);
   virtual LRESULT OnSetText(const wchar_t* text);
   virtual void OnSettingChange(UINT flags, const wchar_t* section);
   virtual void OnSize(UINT param, const CSize& size);
diff --git a/ui/views/widget/widget.cc b/ui/views/widget/widget.cc
index 0bc0906..31748ed 100644
--- a/ui/views/widget/widget.cc
+++ b/ui/views/widget/widget.cc
@@ -941,14 +941,14 @@ void Widget::OnNativeWidgetActivationChanged(bool active) {
                     OnWidgetActivationChanged(this, active));
 }
 
-void Widget::OnNativeFocus(gfx::NativeView focused_view) {
-  WidgetFocusManager::GetInstance()->OnWidgetFocusEvent(focused_view,
+void Widget::OnNativeFocus(gfx::NativeView old_focused_view) {
+  WidgetFocusManager::GetInstance()->OnWidgetFocusEvent(old_focused_view,
                                                         GetNativeView());
 }
 
-void Widget::OnNativeBlur(gfx::NativeView focused_view) {
+void Widget::OnNativeBlur(gfx::NativeView new_focused_view) {
   WidgetFocusManager::GetInstance()->OnWidgetFocusEvent(GetNativeView(),
-                                                        focused_view);
+                                                        new_focused_view);
 }
 
 void Widget::OnNativeWidgetVisibilityChanged(bool visible) {
diff --git a/ui/views/widget/widget.h b/ui/views/widget/widget.h
index 92d1b28..9ee8792 100644
--- a/ui/views/widget/widget.h
+++ b/ui/views/widget/widget.h
@@ -630,8 +630,8 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
   virtual bool IsInactiveRenderingDisabled() const OVERRIDE;
   virtual void EnableInactiveRendering() OVERRIDE;
   virtual void OnNativeWidgetActivationChanged(bool active) OVERRIDE;
-  virtual void OnNativeFocus(gfx::NativeView focused_view) OVERRIDE;
-  virtual void OnNativeBlur(gfx::NativeView focused_view) OVERRIDE;
+  virtual void OnNativeFocus(gfx::NativeView old_focused_view) OVERRIDE;
+  virtual void OnNativeBlur(gfx::NativeView new_focused_view) OVERRIDE;
   virtual void OnNativeWidgetVisibilityChanged(bool visible) OVERRIDE;
   virtual void OnNativeWidgetCreated() OVERRIDE;
   virtual void OnNativeWidgetDestroying() OVERRIDE;