Ensure that the Bubble widget closes correctly when we fade it out on AURA.

Currently the BubbleDelegateView which manages the Bubble widget runs a slide animation when
the bubble fades out. The issue is that the desktop host on Windows AURA assumes that the regular
layer animation callbacks will continue to fire which is where the window is eventually closed.

Fix is to not run our own animation for AURA in BubbleDelegateView::StartFade function and instead
rely on the animation being provided by the underlying layer.

Fixes bug https://code.google.com/p/chromium/issues/detail?id=247171

BUG=247171
TEST=Covered by view_unittest TestBubbleVisibilityAndClose
R=msw@chromium.org, sky@chromium.org

Review URL: https://codereview.chromium.org/16912002

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

3 files changed, 98 insertions, 1 deletions

diff --git a/ui/views/bubble/bubble_delegate.cc b/ui/views/bubble/bubble_delegate.cc
index 706475f..a713e43 100644
--- a/ui/views/bubble/bubble_delegate.cc
+++ b/ui/views/bubble/bubble_delegate.cc
@@ -239,8 +239,17 @@ gfx::Rect BubbleDelegateView::GetAnchorRect() {
 }
 
 void BubbleDelegateView::StartFade(bool fade_in) {
+#if defined(USE_AURA)
+  // Use AURA's window layer animation instead of fading. This ensures that
+  // hosts which rely on the layer animation callbacks to close the window
+  // work correctly.
+  if (fade_in)
+    GetWidget()->Show();
+  else
+    GetWidget()->Close();
+#else
   fade_animation_.reset(new ui::SlideAnimation(this));
-  fade_animation_->SetSlideDuration(kHideFadeDurationMS);
+  fade_animation_->SetSlideDuration(GetFadeDuration());
   fade_animation_->Reset(fade_in ? 0.0 : 1.0);
   if (fade_in) {
     original_opacity_ = 0;
@@ -253,6 +262,7 @@ void BubbleDelegateView::StartFade(bool fade_in) {
     original_opacity_ = 255;
     fade_animation_->Hide();
   }
+#endif
 }
 
 void BubbleDelegateView::ResetFade() {
@@ -340,6 +350,10 @@ gfx::Rect BubbleDelegateView::GetBubbleBounds() {
       GetPreferredSize(), adjust_if_offscreen_);
 }
 
+int BubbleDelegateView::GetFadeDuration() {
+  return kHideFadeDurationMS;
+}
+
 void BubbleDelegateView::UpdateColorsFromTheme(const ui::NativeTheme* theme) {
   if (!color_explicitly_set_) {
     color_ = GetNativeTheme()->GetSystemColor(
diff --git a/ui/views/bubble/bubble_delegate.h b/ui/views/bubble/bubble_delegate.h
index ab76fa5..340cad7 100644
--- a/ui/views/bubble/bubble_delegate.h
+++ b/ui/views/bubble/bubble_delegate.h
@@ -117,6 +117,9 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   // Get bubble bounds from the anchor rect and client view's preferred size.
   virtual gfx::Rect GetBubbleBounds();
 
+  // Returns the duration in milliseconds for the fade animation.
+  virtual int GetFadeDuration();
+
   // View overrides:
   virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE;
   virtual void OnNativeThemeChanged(const ui::NativeTheme* theme) OVERRIDE;
diff --git a/ui/views/bubble/bubble_delegate_unittest.cc b/ui/views/bubble/bubble_delegate_unittest.cc
index 8c601acd..003e67f 100644
--- a/ui/views/bubble/bubble_delegate_unittest.cc
+++ b/ui/views/bubble/bubble_delegate_unittest.cc
@@ -2,11 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/run_loop.h"
 #include "ui/views/bubble/bubble_delegate.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/test/test_widget_observer.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_observer.h"
+
+#if defined(USE_AURA)
+#include "ui/aura/env.h"
+#endif
 
 namespace views {
 
@@ -19,6 +25,10 @@ class TestBubbleDelegateView : public BubbleDelegateView {
 
   virtual View* GetInitiallyFocusedView() OVERRIDE;
 
+ protected:
+  // BubbleDelegateView overrides.
+  virtual int GetFadeDuration() OVERRIDE;
+
  private:
   View* view_;
 };
@@ -36,6 +46,11 @@ View* TestBubbleDelegateView::GetInitiallyFocusedView() {
   return view_;
 }
 
+int TestBubbleDelegateView::GetFadeDuration() {
+  // We cannot return 0 as that causes the animation to never complete.
+  return 1;
+}
+
 }  // namespace
 
 typedef ViewsTestBase BubbleDelegateTest;
@@ -181,4 +196,69 @@ TEST_F(BubbleDelegateTest, DISABLED_InitiallyFocusedView) {
   bubble_widget->CloseNow();
 }
 
+// This class provides functionality to verify that the BubbleView shows up
+// when we call BubbleDelegateView::StartFade(true) and is destroyed when we
+// call BubbleDelegateView::StartFade(false).
+class BubbleWidgetClosingTest : public BubbleDelegateTest,
+                                public views::WidgetObserver {
+ public:
+  BubbleWidgetClosingTest()
+      : bubble_destroyed_(false) {
+#if defined(USE_AURA)
+    loop_.set_dispatcher(aura::Env::GetInstance()->GetDispatcher());
+#endif
+  }
+
+  virtual ~BubbleWidgetClosingTest() {}
+
+  void Observe(views::Widget* widget) {
+    widget->AddObserver(this);
+  }
+
+  // views::WidgetObserver overrides.
+  virtual void OnWidgetDestroyed(Widget* widget) {
+    bubble_destroyed_ = true;
+    widget->RemoveObserver(this);
+    loop_.Quit();
+  }
+
+  bool bubble_destroyed() const { return bubble_destroyed_; }
+
+  void RunNestedLoop() {
+    loop_.Run();
+  }
+
+ private:
+  bool bubble_destroyed_;
+  base::RunLoop loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(BubbleWidgetClosingTest);
+};
+
+TEST_F(BubbleWidgetClosingTest, TestBubbleVisibilityAndClose) {
+  // Create the anchor and parent widgets.
+  Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  scoped_ptr<Widget> anchor_widget(new Widget);
+  anchor_widget->Init(params);
+  anchor_widget->Show();
+
+  TestBubbleDelegateView* bubble_delegate =
+      new TestBubbleDelegateView(anchor_widget->GetContentsView());
+  Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
+  EXPECT_FALSE(bubble_widget->IsVisible());
+
+  bubble_delegate->StartFade(true);
+  EXPECT_TRUE(bubble_widget->IsVisible());
+
+  EXPECT_EQ(bubble_delegate->GetInitiallyFocusedView(),
+            bubble_widget->GetFocusManager()->GetFocusedView());
+
+  Observe(bubble_widget);
+
+  bubble_delegate->StartFade(false);
+  RunNestedLoop();
+  EXPECT_TRUE(bubble_destroyed());
+}
+
 }  // namespace views