Fix position of web notification bubble and arrow

ash/system/tray/system_tray.cc

Index: ash/system/tray/system_tray.cc
diff --git a/ash/system/tray/system_tray.cc b/ash/system/tray/system_tray.cc
index 8b3cb3b373328e3db24342a14d9ad4a9c5f086ef..2a8d8e536cd46b5832fd0438212fae9114d5ca5c 100644
--- a/ash/system/tray/system_tray.cc
+++ b/ash/system/tray/system_tray.cc
@@ -30,7 +30,6 @@
 #include "ash/system/user/login_status.h"
 #include "ash/system/user/tray_user.h"
 #include "ash/wm/shelf_layout_manager.h"
-#include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/timer.h"
 #include "base/utf_string_conversions.h"
@@ -144,6 +143,7 @@ class SystemTrayLayerAnimationObserver : public ui::LayerAnimationObserver {
 
 using internal::SystemTrayBubble;
 using internal::SystemTrayLayerAnimationObserver;
+using internal::TrayBubbleView;
 
 SystemTray::SystemTray()
     : items_(),
@@ -256,7 +256,8 @@ void SystemTray::RemoveTrayItem(SystemTrayItem* item) {
 }
 
 void SystemTray::ShowDefaultView(BubbleCreationType creation_type) {
-  ShowDefaultViewWithOffset(creation_type, -1);
+  ShowDefaultViewWithOffset(creation_type,
+                            TrayBubbleView::InitParams::kArrowDefaultOffset);
 }
 
 void SystemTray::ShowDetailedView(SystemTrayItem* item,
@@ -366,35 +367,24 @@ void SystemTray::RemoveBubble(SystemTrayBubble* bubble) {
 }
 
 int SystemTray::GetTrayXOffset(SystemTrayItem* item) const {
+  // Don't attempt to align the arrow if the shelf is on the left or right.
+  if (shelf_alignment() != SHELF_ALIGNMENT_BOTTOM)
+    return TrayBubbleView::InitParams::kArrowDefaultOffset;
+
   std::map<SystemTrayItem*, views::View*>::const_iterator it =
       tray_item_map_.find(item);
   if (it == tray_item_map_.end())
-    return -1;
+    return TrayBubbleView::InitParams::kArrowDefaultOffset;
 
   const views::View* item_view = it->second;
-  gfx::Rect item_bounds = item_view->bounds();
-  if (!item_bounds.IsEmpty()) {
-    int x_offset = item_bounds.x() + item_bounds.width() / 2;
-    return base::i18n::IsRTL() ? x_offset : tray_container_->width() - x_offset;
-  }
+  gfx::Rect rect(item_view->width() / 2, 0, 0, 0);

[msw, 2012/07/27 20:32:04]

nit: use a Point and ConvertPointToWidget; assign the converted value to the
same local point.

[stevenjb, 2012/07/30 20:09:03]

Done.

+  gfx::Rect item_bounds = item_view->ConvertRectToWidget(rect);
+  if (!item_view->bounds().IsEmpty())

[msw, 2012/07/27 20:32:04]

Invert the check and make this an early return of kArrowDefaultOffset
(potentially combine with line 376/377 above). If the bounds are empty,
converting the point is unnecessary.

[stevenjb, 2012/07/30 20:09:03]

Good call, done.

+    return item_bounds.x();
 
-  // The bounds of item could be still empty.  It could happen in the case that
-  // the view appears for the first time in the current session, because the
-  // bounds is not calculated yet. In that case, we want to guess the offset
-  // from the position of its parent.
-  int x_offset = 0;
-  for (int i = 0; i < tray_container_->child_count(); ++i) {
-    const views::View* child = tray_container_->child_at(i);
-    if (child == item_view)
-      return base::i18n::IsRTL() ?
-          x_offset : tray_container_->width() - x_offset;
-
-    if (!child->visible())
-      continue;
-    x_offset = child->bounds().right();
-  }
-
-  return -1;
+  // The bounds of item could be still empty if it does not have a visible
+  // tray view. In that case, use the default (minimum) offset.
+  return TrayBubbleView::InitParams::kArrowDefaultOffset;
 }
 
 void SystemTray::ShowDefaultViewWithOffset(BubbleCreationType creation_type,
@@ -423,17 +413,13 @@ void SystemTray::ShowItems(const std::vector<SystemTrayItem*>& items,
     ash::SystemTrayDelegate* delegate =
         ash::Shell::GetInstance()->tray_delegate();
     views::View* anchor = tray_container_;
-    SystemTrayBubble::InitParams init_params(
-        SystemTrayBubble::ANCHOR_TYPE_TRAY,
-        shelf_alignment());
-    init_params.anchor = anchor;
+    TrayBubbleView::InitParams init_params(TrayBubbleView::ANCHOR_TYPE_TRAY,
+                                           shelf_alignment());
     init_params.can_activate = can_activate;
-    init_params.login_status = delegate->GetUserLoginStatus();
-    if (arrow_offset >= 0)
-      init_params.arrow_offset = arrow_offset;
     if (detailed)
       init_params.max_height = default_bubble_height_;
-    bubble_->InitView(init_params);
+    init_params.arrow_offset = arrow_offset;
+    bubble_->InitView(anchor, init_params, delegate->GetUserLoginStatus());
   }
   // Save height of default view for creating detailed views directly.
   if (!detailed)
@@ -479,22 +465,20 @@ void SystemTray::UpdateNotificationBubble() {
         this, notification_items_, SystemTrayBubble::BUBBLE_TYPE_NOTIFICATION));
   }
   views::View* anchor;
-  SystemTrayBubble::AnchorType anchor_type;
+  TrayBubbleView::AnchorType anchor_type;
   if (bubble_.get()) {
     anchor = bubble_->bubble_view();
-    anchor_type = SystemTrayBubble::ANCHOR_TYPE_BUBBLE;
+    anchor_type = TrayBubbleView::ANCHOR_TYPE_BUBBLE;
   } else {
     anchor = tray_container_;
-    anchor_type = SystemTrayBubble::ANCHOR_TYPE_TRAY;
+    anchor_type = TrayBubbleView::ANCHOR_TYPE_TRAY;
   }
-  SystemTrayBubble::InitParams init_params(anchor_type, shelf_alignment());
-  init_params.anchor = anchor;
-  init_params.login_status =
-      ash::Shell::GetInstance()->tray_delegate()->GetUserLoginStatus();
-  int arrow_offset = GetTrayXOffset(notification_items_[0]);
-  if (arrow_offset >= 0)
-    init_params.arrow_offset = arrow_offset;
-  notification_bubble_->InitView(init_params);
+  TrayBubbleView::InitParams init_params(anchor_type, shelf_alignment());
+  init_params.arrow_offset = GetTrayXOffset(notification_items_[0]);
+  init_params.arrow_color = kBackgroundColor;
+  user::LoginStatus login_status =
+      Shell::GetInstance()->tray_delegate()->GetUserLoginStatus();
+  notification_bubble_->InitView(anchor, init_params, login_status);
   if (hide_notifications_)
     notification_bubble_->SetVisible(false);
 }
@@ -502,7 +486,7 @@ void SystemTray::UpdateNotificationBubble() {
 void SystemTray::UpdateNotificationAnchor() {
   if (!notification_bubble_.get())
     return;
-  notification_bubble_->bubble_view()->UpdateAnchor();
+  notification_bubble_->bubble_view()->UpdateBubble();
   // Ensure that the notification buble is above the launcher/status area.
   notification_bubble_->bubble_view()->GetWidget()->StackAtTop();
 }
@@ -533,6 +517,13 @@ void SystemTray::SetShelfAlignment(ShelfAlignment alignment) {
   UpdateAfterShelfAlignmentChange(alignment);
   SetBorder();
   tray_container_->UpdateLayout(alignment);
+  // Destroy an existing bubble so that it is rebuilt correctly.

[msw, 2012/07/27 20:32:04]

nit: not sure if you meant to write "any".

[stevenjb, 2012/07/30 20:09:03]

Done.

+  bubble_.reset();
+  // Rebuild any notification bubble.
+  if (notification_bubble_.get()) {
+    notification_bubble_.reset();
+    UpdateNotificationBubble();
+  }
 }
 
 bool SystemTray::PerformAction(const views::Event& event) {
@@ -542,13 +533,15 @@ bool SystemTray::PerformAction(const views::Event& event) {
       bubble_->bubble_type() == SystemTrayBubble::BUBBLE_TYPE_DEFAULT) {
     bubble_->Close();
   } else {
-    int arrow_offset = -1;
+    int arrow_offset = TrayBubbleView::InitParams::kArrowDefaultOffset;
     if (event.IsMouseEvent() || event.type() == ui::ET_GESTURE_TAP) {
       const views::LocatedEvent& located_event =
           static_cast<const views::LocatedEvent&>(event);
-      if (shelf_alignment() == SHELF_ALIGNMENT_BOTTOM)
-        arrow_offset = base::i18n::IsRTL() ?
-            located_event.x() : tray_container_->width() - located_event.x();
+      if (shelf_alignment() == SHELF_ALIGNMENT_BOTTOM) {
+        gfx::Rect bounds(located_event.x(), 0, 0, 0);

[msw, 2012/07/27 20:32:04]

nit: use a Point and ConvertPointToWidget.

[stevenjb, 2012/07/30 20:09:03]

Done.

+        bounds = ConvertRectToWidget(bounds);
+        arrow_offset = bounds.x();
+      }
     }
     ShowDefaultViewWithOffset(BUBBLE_CREATE_NEW, arrow_offset);
   }