Remove close_on_deactivate=false for tray bubbles (2nd)

Actually we want to close those bubbles on deactivation (like, opening
settings page or opening the chat window for a notification). To deal
with the race condition of close-on-deactivate and button action,
a static delay is introduced.

The first attempt (r184572) has been reverted due to a crash bug.
It seems that the crash happens because of missing cleanup of system
notification bubbles.

BUG=179992, 177075
TEST=on goobuntu, open the systme tray, wait for the power notifications, then click 'settings' and see no crash.


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

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

5 files changed, 29 insertions, 7 deletions

diff --git a/ash/system/tray/system_tray.cc b/ash/system/tray/system_tray.cc
index 0b17757..3623c16 100644
--- a/ash/system/tray/system_tray.cc
+++ b/ash/system/tray/system_tray.cc
@@ -364,7 +364,6 @@ void SystemTray::ShowItems(const std::vector<SystemTrayItem*>& items,
                                            kTrayPopupMinWidth,
                                            kTrayPopupMaxWidth);
     init_params.can_activate = can_activate;
-    init_params.close_on_deactivate = false;
     if (detailed) {
       // This is the case where a volume control or brightness control bubble
       // is created.
@@ -477,6 +476,8 @@ void SystemTray::HideBubbleWithView(const TrayBubbleView* bubble_view) {
   } else if (notification_bubble_.get() &&
              bubble_view == notification_bubble_->bubble_view()) {
     DestroyNotificationBubble();
+  } else {
+    UpdateNotificationBubble();
   }
 }
 
diff --git a/ash/system/tray/tray_background_view.h b/ash/system/tray/tray_background_view.h
index 8b136655..17aa5a9 100644
--- a/ash/system/tray/tray_background_view.h
+++ b/ash/system/tray/tray_background_view.h
@@ -87,7 +87,7 @@ class ASH_EXPORT TrayBackgroundView : public ActionableView,
   virtual string16 GetAccessibleNameForTray() = 0;
 
   // Hides the bubble associated with |bubble_view|. Called when the widget
-  // is closed.
+  // is closed. |bubble_view| may be already destroyed.
   virtual void HideBubbleWithView(const views::TrayBubbleView* bubble_view) = 0;
 
   // Called by the bubble wrapper when a click event occurs outside the bubble.
diff --git a/ash/system/tray/tray_bubble_wrapper.cc b/ash/system/tray/tray_bubble_wrapper.cc
index 75651c2..70980ea 100644
--- a/ash/system/tray/tray_bubble_wrapper.cc
+++ b/ash/system/tray/tray_bubble_wrapper.cc
@@ -7,6 +7,9 @@
 #include "ash/system/tray/tray_background_view.h"
 #include "ash/system/tray/tray_event_filter.h"
 #include "ash/wm/window_properties.h"
+#include "base/bind.h"
+#include "base/message_loop.h"
+#include "base/time.h"
 #include "ui/views/bubble/tray_bubble_view.h"
 #include "ui/views/widget/widget.h"
 
@@ -17,6 +20,7 @@ TrayBubbleWrapper::TrayBubbleWrapper(TrayBackgroundView* tray,
                                      views::TrayBubbleView* bubble_view)
     : tray_(tray),
       bubble_view_(bubble_view) {
+  DCHECK(tray_);
   bubble_widget_ = views::BubbleDelegateView::CreateBubble(bubble_view_);
   bubble_widget_->AddObserver(this);
   bubble_widget_->GetNativeView()->
@@ -40,7 +44,17 @@ TrayBubbleWrapper::~TrayBubbleWrapper() {
 void TrayBubbleWrapper::OnWidgetDestroying(views::Widget* widget) {
   CHECK_EQ(bubble_widget_, widget);
   bubble_widget_ = NULL;
-  tray_->HideBubbleWithView(bubble_view_);  // May destroy |bubble_view_|
+
+  // Do not call HideBubbleWithView directly but post the task to ensure that
+  // HideBubbleWithView is called after the click event on the tray button is
+  // handled. See crbug.com/177075 and crbug.com/169940
+  MessageLoopForUI::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&TrayBubbleWrapper::HideBubbleWithView, AsWeakPtr()));
+}
+
+void TrayBubbleWrapper::HideBubbleWithView() {
+  tray_->HideBubbleWithView(bubble_view_);
 }
 
 }  // namespace internal
diff --git a/ash/system/tray/tray_bubble_wrapper.h b/ash/system/tray/tray_bubble_wrapper.h
index baa5416..562b196 100644
--- a/ash/system/tray/tray_bubble_wrapper.h
+++ b/ash/system/tray/tray_bubble_wrapper.h
@@ -7,6 +7,7 @@
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "ui/views/widget/widget_observer.h"
 
 namespace views {
@@ -20,8 +21,8 @@ class TrayBackgroundView;
 class TrayEventFilter;
 
 // Creates and manages the Widget and EventFilter components of a bubble.
-
-class TrayBubbleWrapper : public views::WidgetObserver {
+class TrayBubbleWrapper : public views::WidgetObserver,
+                          public base::SupportsWeakPtr<TrayBubbleWrapper> {
  public:
   TrayBubbleWrapper(TrayBackgroundView* tray,
                     views::TrayBubbleView* bubble_view);
@@ -36,8 +37,15 @@ class TrayBubbleWrapper : public views::WidgetObserver {
   const views::Widget* bubble_widget() const { return bubble_widget_; }
 
  private:
+  void HideBubbleWithView();
+
+  // unowned. |tray_| owns this and it should outlive.
   TrayBackgroundView* tray_;
-  views::TrayBubbleView* bubble_view_;  // unowned
+
+  // unowned.
+  views::TrayBubbleView* bubble_view_;
+
+  // unowned.
   views::Widget* bubble_widget_;
 
   DISALLOW_COPY_AND_ASSIGN(TrayBubbleWrapper);
diff --git a/ash/system/web_notification/web_notification_tray.cc b/ash/system/web_notification/web_notification_tray.cc
index cfc1e1c..2e05fa4 100644
--- a/ash/system/web_notification/web_notification_tray.cc
+++ b/ash/system/web_notification/web_notification_tray.cc
@@ -61,7 +61,6 @@ class WebNotificationBubbleWrapper {
         tray->GetAnchorAlignment();
     views::TrayBubbleView::InitParams init_params =
         bubble->GetInitParams(anchor_alignment);
-    init_params.close_on_deactivate = false;
     views::View* anchor = tray->tray_container();
     if (anchor_alignment == views::TrayBubbleView::ANCHOR_ALIGNMENT_BOTTOM) {
       gfx::Point bounds(anchor->width() / 2, 0);