The password bubble should fade out on click on the web page.

The timeout is now obsolete. The bubble stays infinitely if there is no user input.

BUG=394287

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

Cr-Commit-Position: refs/heads/master@{#292887}

chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc

@@ -22,6 +22,8 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/combobox_model.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/compositor/layer_animation_observer.h"
+#include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/text_utils.h"
 #include "ui/views/controls/button/blue_button.h"
 #include "ui/views/controls/button/label_button.h"
@@ -36,8 +38,8 @@
 
 namespace {
 
-// The number of seconds the inactive bubble should stay alive.
-const int kBubbleCloseDelay = 15;
+// The number of seconds the bubble needs to fade out.
+const int kBubbleFadeDelay = 2;
 
 const int kDesiredBubbleWidth = 370;
 
@@ -534,7 +536,7 @@ class ManagePasswordsBubbleView::WebContentMouseHandler
 
   virtual void OnMouseEvent(ui::MouseEvent* event) OVERRIDE {
     if (event->type() == ui::ET_MOUSE_PRESSED)
-      bubble_->OnWebContentClicked();
+      bubble_->StartFadingOut();
   }
 
  private:
@@ -548,6 +550,26 @@ class ManagePasswordsBubbleView::WebContentMouseHandler
   DISALLOW_COPY_AND_ASSIGN(WebContentMouseHandler);
 };
 
+// ManagePasswordsBubbleView::FadeOutObserver ---------------------------------
+
+// The class notifies the bubble when it faded out completely.
+class ManagePasswordsBubbleView::FadeOutObserver
+    : public ui::ImplicitAnimationObserver {
+ public:
+  explicit FadeOutObserver(ManagePasswordsBubbleView* bubble)
+      : bubble_(bubble) {
+  }
+
+  virtual void OnImplicitAnimationsCompleted() OVERRIDE {
+    bubble_->OnBubbleDisappeared();
+  }
+
+ private:
+  ManagePasswordsBubbleView* bubble_;
+
+  DISALLOW_COPY_AND_ASSIGN(FadeOutObserver);
+};
+
 // ManagePasswordsBubbleView --------------------------------------------------
 
 // static
@@ -590,7 +612,6 @@ void ManagePasswordsBubbleView::ShowBubble(content::WebContents* web_contents,
     manage_passwords_bubble_->GetWidget()->ShowInactive();
   else
     manage_passwords_bubble_->GetWidget()->Show();
-  manage_passwords_bubble_->StartTimerIfNecessary();
 }
 
 // static
@@ -652,6 +673,8 @@ void ManagePasswordsBubbleView::AdjustForFullscreen(
 }
 
 void ManagePasswordsBubbleView::Close() {
+  fadeout_observer_.reset();
+  mouse_handler_.reset();
   GetWidget()->Close();
 }
 
@@ -672,7 +695,7 @@ void ManagePasswordsBubbleView::WindowClosing() {
 void ManagePasswordsBubbleView::OnWidgetActivationChanged(views::Widget* widget,
                                                           bool active) {
   if (active && widget == GetWidget())
-    timer_.Stop();
+    CancelFadingOut();
   BubbleDelegateView::OnWidgetActivationChanged(widget, active);
 }
 
@@ -681,11 +704,7 @@ views::View* ManagePasswordsBubbleView::GetInitiallyFocusedView() {
 }
 
 void ManagePasswordsBubbleView::OnMouseEntered(const ui::MouseEvent& event) {
-  timer_.Stop();
-}
-
-void ManagePasswordsBubbleView::OnMouseExited(const ui::MouseEvent& event) {
-  StartTimerIfNecessary();
+  CancelFadingOut();
 }
 
 void ManagePasswordsBubbleView::Refresh() {
@@ -704,9 +723,7 @@ void ManagePasswordsBubbleView::Refresh() {
     AddChildView(new ManageView(this));
   }
   GetLayoutManager()->Layout(this);
-  // If we refresh the existing bubble we may want to restart the timer.
-  if (GetWidget())
-    StartTimerIfNecessary();
+  CancelFadingOut();
 }
 
 void ManagePasswordsBubbleView::NotifyNeverForThisSiteClicked() {
@@ -729,16 +746,26 @@ void ManagePasswordsBubbleView::NotifyUndoNeverForThisSite() {
   Refresh();
 }
 
-void ManagePasswordsBubbleView::StartTimerIfNecessary() {
-  // Active bubble will stay visible until it loses focus.
-  if (GetWidget()->IsActive())
+void ManagePasswordsBubbleView::StartFadingOut() {
+  if (fadeout_observer_)
     return;
-  timer_.Start(FROM_HERE,
-               base::TimeDelta::FromSeconds(kBubbleCloseDelay),
-               this,
-               &ManagePasswordsBubbleView::Close);
+  aura::Window* window = GetWidget()->GetNativeView();
+  ui::ScopedLayerAnimationSettings animator(window->layer()->GetAnimator());
+  fadeout_observer_.reset(new FadeOutObserver(this));
+  animator.AddObserver(fadeout_observer_.get());
+  animator.SetTransitionDuration(
+      base::TimeDelta::FromSeconds(kBubbleFadeDelay));
+  window->layer()->SetOpacity(0);
+}
+
+void ManagePasswordsBubbleView::CancelFadingOut() {
+  if (!fadeout_observer_)
+    return;
+  fadeout_observer_.reset();
+  aura::Window* window = GetWidget()->GetNativeView();
+  window->layer()->SetOpacity(1);
 }
 
-void ManagePasswordsBubbleView::OnWebContentClicked() {
+void ManagePasswordsBubbleView::OnBubbleDisappeared() {
   Close();
 }

chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h

@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_UI_VIEWS_PASSWORDS_MANAGE_PASSWORDS_BUBBLE_VIEW_H_
 
 #include "base/basictypes.h"
-#include "base/timer/timer.h"
 #include "chrome/browser/ui/passwords/manage_passwords_bubble.h"
 #include "chrome/browser/ui/passwords/save_password_refusal_combobox_model.h"
 #include "ui/views/bubble/bubble_delegate.h"
@@ -175,8 +174,8 @@ class ManagePasswordsBubbleView : public ManagePasswordsBubble,
     return initially_focused_view_;
   }
 
-  bool IsTimerRunning() const {
-    return timer_.IsRunning();
+  bool IsFadingAway() const {
+    return fadeout_observer_;
   }
 
  private:
@@ -210,9 +209,6 @@ class ManagePasswordsBubbleView : public ManagePasswordsBubble,
   // undo the action and refresh to PendingView.
   void NotifyUndoNeverForThisSite();
 
-  // Starts a timer which will close the bubble if it's inactive.
-  void StartTimerIfNecessary();
-
   // views::BubbleDelegateView:
   virtual void Init() OVERRIDE;
   virtual void WindowClosing() OVERRIDE;
@@ -224,10 +220,15 @@ class ManagePasswordsBubbleView : public ManagePasswordsBubble,
 
   // views::View methods.
   virtual void OnMouseEntered(const ui::MouseEvent& event) OVERRIDE;
-  virtual void OnMouseExited(const ui::MouseEvent& event) OVERRIDE;
 
-  // Called from WebContentMouseHandler when user clicks the web view.
-  void OnWebContentClicked();
+  // Starts animating the bubble.
+  void StartFadingOut();
+
+  // Cancel fading out if it's active.
+  void CancelFadingOut();
+
+  // Called when the bubble completely faded out.
+  void OnBubbleDisappeared();
 
   void set_initially_focused_view(views::View* view) {
     DCHECK(!initially_focused_view_);
@@ -248,12 +249,14 @@ class ManagePasswordsBubbleView : public ManagePasswordsBubble,
 
   views::View* initially_focused_view_;
 
-  // Timer used to close the bubble after timeout.
-  base::OneShotTimer<ManagePasswordsBubbleView> timer_;
-
+  // A helper to intercept mouse click events on the web contents.
   class WebContentMouseHandler;
   scoped_ptr<WebContentMouseHandler> mouse_handler_;
 
+  // A helper to get a notification when the bubble fades out completely.
+  class FadeOutObserver;
+  scoped_ptr<FadeOutObserver> fadeout_observer_;
+
   DISALLOW_COPY_AND_ASSIGN(ManagePasswordsBubbleView);
 };
 

chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc

@@ -15,12 +15,52 @@
 #include "chrome/test/base/interactive_test_utils.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
+#include "content/public/browser/web_contents.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/aura/window.h"
 
 namespace {
 
 const char kDisplayDispositionMetric[] = "PasswordBubble.DisplayDisposition";
 
+// Listens to WebContents and invokes a callback on the mouse down event.
+class WebContentMouseClickHandler : public ui::EventHandler {
+ public:
+  explicit WebContentMouseClickHandler(content::WebContents* web_contents,
+                                       const base::Closure& callback)
+      : web_contents_(web_contents),
+        callback_(callback),
+        was_called_(false) {
+    web_contents_->GetNativeView()->AddPreTargetHandler(this);
+  }
+
+  virtual ~WebContentMouseClickHandler() {
+    web_contents_->GetNativeView()->RemovePreTargetHandler(this);
+  }
+
+  virtual void OnMouseEvent(ui::MouseEvent* event) OVERRIDE {
+    if (event->type() == ui::ET_MOUSE_PRESSED) {
+      callback_.Run();
+      was_called_ = true;
+    }
+  }
+
+  bool was_called() const { return was_called_; }
+
+ private:
+  content::WebContents* web_contents_;
+  base::Closure callback_;
+  bool was_called_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebContentMouseClickHandler);
+};
+
+void CheckBubbleAnimation() {
+  EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing());
+  EXPECT_TRUE(ManagePasswordsBubbleView::manage_password_bubble()->
+      IsFadingAway());
+}
+
 }  // namespace
 
 namespace metrics_util = password_manager::metrics_util;
@@ -52,7 +92,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, BasicOpenAndClose) {
   EXPECT_TRUE(bubble->initially_focused_view());
   EXPECT_EQ(bubble->initially_focused_view(),
             bubble->GetFocusManager()->GetFocusedView());
-  EXPECT_FALSE(bubble->IsTimerRunning());
+  EXPECT_FALSE(bubble->IsFadingAway());
   ManagePasswordsBubbleView::CloseBubble();
   EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing());
 
@@ -80,6 +120,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandControlsBubble) {
   EXPECT_TRUE(bubble->initially_focused_view());
   EXPECT_EQ(bubble->initially_focused_view(),
             bubble->GetFocusManager()->GetFocusedView());
+  EXPECT_FALSE(bubble->IsFadingAway());
   ManagePasswordsBubbleView::CloseBubble();
   EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing());
 
@@ -120,8 +161,8 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest,
   // Bubble can be active if user clicks it.
   EXPECT_TRUE(ManagePasswordsBubbleView::manage_password_bubble()->
       CanActivate());
-  EXPECT_TRUE(ManagePasswordsBubbleView::manage_password_bubble()->
-      IsTimerRunning());
+  EXPECT_FALSE(ManagePasswordsBubbleView::manage_password_bubble()->
+      IsFadingAway());
 
   scoped_ptr<base::HistogramSamples> samples(
       GetSamples(kDisplayDispositionMetric));
@@ -180,13 +221,18 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest,
                 metrics_util::MANUAL_MANAGE_PASSWORDS));
 }
 
-IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CloseOnClick) {
+IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, FadeOnClick) {
   ManagePasswordsBubbleView::ShowBubble(
       browser()->tab_strip_model()->GetActiveWebContents(),
       ManagePasswordsBubble::AUTOMATIC);
   EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing());
   EXPECT_FALSE(ManagePasswordsBubbleView::manage_password_bubble()->
       GetFocusManager()->GetFocusedView());
+  // We have to check the animation in the process of handling the mouse down
+  // event. Otherwise, animation may finish too quickly.
+  WebContentMouseClickHandler observer(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      base::Bind(&CheckBubbleAnimation));
   ui_test_utils::ClickOnView(browser(), VIEW_ID_TAB_CONTAINER);
-  EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing());
+  EXPECT_TRUE(observer.was_called());
 }