Fix position of web notification bubble and arrow

ash/system/tray/tray_bubble_view.cc

Index: ash/system/tray/tray_bubble_view.cc
diff --git a/ash/system/tray/tray_bubble_view.cc b/ash/system/tray/tray_bubble_view.cc
index 295e70399a94e230af69abf0716d25e61c3771e9..e5cf35646fdd403131a888aade15507a89898938 100644
--- a/ash/system/tray/tray_bubble_view.cc
+++ b/ash/system/tray/tray_bubble_view.cc
@@ -8,6 +8,7 @@
 #include "ash/shell_window_ids.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/wm/shelf_layout_manager.h"
+#include "ash/wm/window_animations.h"
 #include "grit/ash_strings.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -37,10 +38,13 @@ const int kArrowHeight = 10;
 const int kArrowWidth = 20;
 
 // Inset the arrow a bit from the edge.
-const int kArrowMinOffset = kArrowWidth / 2 + 4;
+const int kArrowEdgeMargin = 12;
+const int kArrowMinOffset = kArrowWidth / 2 + kArrowEdgeMargin;
 
 const SkColor kShadowColor = SkColorSetARGB(0xff, 0, 0, 0);
 
+const int kAnimationDurationForPopupMS = 200;
+
 void DrawBlurredShadowAroundView(gfx::Canvas* canvas,
                                  int top,
                                  int bottom,
@@ -72,27 +76,18 @@ class TrayBubbleBorder : public views::BubbleBorder {
   TrayBubbleBorder(views::View* owner,
                    views::View* anchor,
                    views::BubbleBorder::ArrowLocation arrow_location,
-                   int arrow_offset)
-      : views::BubbleBorder(arrow_location,
-                            views::BubbleBorder::NO_SHADOW),
+                   int arrow_offset,
+                   const SkColor& arrow_color)
+      : views::BubbleBorder(arrow_location, views::BubbleBorder::NO_SHADOW),
         owner_(owner),
         anchor_(anchor),
         tray_arrow_offset_(arrow_offset) {
     set_alignment(views::BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+    set_background_color(arrow_color);
   }
 
   virtual ~TrayBubbleBorder() {}
 
- private:
-  views::Background* FindAppropriateBackground(views::View* view,
-                                               const gfx::Point& point) const {
-    views::Background* background = NULL;
-    views::View* target = view->GetEventHandlerForPoint(point);
-    for (; target && !background; target = target->parent())
-      background = target->background();
-    return background;
-  }

[jennyz, 2012/07/31 02:29:47]

I wonder if the arrow color will be right after removing this code from sadrul's
cl: https://chromiumcodereview.appspot.com/10566003

[stevenjb, 2012/07/31 16:35:28]

I think sadrul's CL predates the current design which always has a blue section
at the bottom of the uber tray; I tested these changes pretty thoroughly.
Unfortunately the code above did not work for web notifications, and would be
difficult to maintain when the bubble layout changes; setting the color
explicitly seemed easier to maintain.

-
   // Overridden from views::BubbleBorder.
   // Override views::BubbleBorder to set the bubble on top of the anchor when
   // it has no arrow.
@@ -117,11 +112,25 @@ class TrayBubbleBorder : public views::BubbleBorder {
     return gfx::Rect(x, y, border_size.width(), border_size.height());
   }
 
+  // TrayBubbleView supports dynamically updated bubbles. This does not

[msw, 2012/07/31 14:41:56]

FYI: Check out chrome/browser/ui/views/page_info_bubble_view.cc
PageInfoBubbleView grows via animation, and it may offer some inspiration.

[stevenjb, 2012/07/31 16:35:28]

I'll take a look. As discussed, I plan to write unit tests with the existing
bubble code to see if I can break it, then fix that code and/or this code so
that we don't need the customization. Added issue 139813 and added it to the
comment.

+  // behave well with BubbleFrameView which expects arrow_location to be
+  // unmirrored during initial layout (when ClientView is constructed),
+  // then mirrored after SizeToContents() gets called.
+  // So, instead of mirroring the arrow in CreateNonClientFrameView,
+  // mirror it here instead.
+  virtual void GetInsets(gfx::Insets* insets) const OVERRIDE {
+    ArrowLocation arrow_loc = arrow_location();
+    if (base::i18n::IsRTL())
+      arrow_loc = horizontal_mirror(arrow_loc);
+    return GetInsetsForArrowLocation(insets, arrow_loc);

[jennyz, 2012/07/31 02:29:47]

It seems make more sense for bubble infrastructure code to support this, instead
of client code to deal with it. I guess it probably not easy to do, so this is a
work around?

[stevenjb, 2012/07/31 16:35:28]

Yes, this is a work around until we have more/better tests defining the Bubble
behavior (I didn't want to risk breaking for other bubbles without the
additional tests). Created an issue and added it to the comment above.

+  }
+
   // Overridden from views::Border.
   virtual void Paint(const views::View& view,
                      gfx::Canvas* canvas) const OVERRIDE {
     gfx::Insets inset;
-    GetInsets(&inset);
+    // Get the unmirrored insets for the arrow location.

[jennyz, 2012/07/31 02:29:47]

Can you add some comments to explain why we need to use unmirrored insets here?

[stevenjb, 2012/07/31 16:35:28]

Added a little to the comment here, mostly see the issue referenced above.

+    GetInsetsForArrowLocation(&inset, arrow_location());
     DrawBlurredShadowAroundView(
         canvas, 0, owner_->height(), owner_->width(), inset);
 
@@ -137,22 +146,28 @@ class TrayBubbleBorder : public views::BubbleBorder {
     gfx::Point arrow_reference;
 
     // Draw the arrow after drawing child borders, so that the arrow can cover
-    // the its overlap section with child border.
+    // its overlap section with child border.
     SkPath path;
     path.incReserve(4);
     if (arrow_location() == views::BubbleBorder::BOTTOM_RIGHT ||
         arrow_location() == views::BubbleBorder::BOTTOM_LEFT) {
-      // Do not let the arrow too close to the edge of the bubble and
-      // and the edge of the anchor.
-      int tip_x = base::i18n::IsRTL() ?
-          std::min(std::max(tray_arrow_offset_, kArrowMinOffset),
-                   std::min(owner_->width(), anchor_->width())
-                       - kArrowMinOffset) :
-          std::min(std::max(owner_->width() - tray_arrow_offset_,
-                            owner_->width() -
-                                std::min(owner_->width(), anchor_->width()) +
-                                    kArrowMinOffset),
-                   owner_->width() - kArrowMinOffset);
+      // Note: tray_arrow_offset_ is relative to the anchor widget.
+      int tip_x;
+      if (tray_arrow_offset_ ==
+          internal::TrayBubbleView::InitParams::kArrowDefaultOffset) {
+        if (arrow_location() == views::BubbleBorder::BOTTOM_LEFT)
+          tip_x = kArrowMinOffset;
+        else
+          tip_x = owner_->width() - kArrowMinOffset;
+      } else {
+        gfx::Point pt(tray_arrow_offset_, 0);
+        views::View::ConvertPointToScreen(
+            anchor_->GetWidget()->GetRootView(), &pt);
+        views::View::ConvertPointFromScreen(
+            owner_->GetWidget()->GetRootView(), &pt);
+        tip_x = std::min(pt.x(), owner_->width() - kArrowMinOffset);
+        tip_x = std::max(tip_x, kArrowMinOffset);
+      }
       int left_base_x = tip_x - kArrowWidth / 2;
       int left_base_y = y;
       int tip_y = left_base_y + kArrowHeight;
@@ -162,9 +177,20 @@ class TrayBubbleBorder : public views::BubbleBorder {
                   SkIntToScalar(left_base_y));
       arrow_reference.SetPoint(tip_x, left_base_y - kArrowHeight);
     } else {
-      int tip_y = y - tray_arrow_offset_;
-      tip_y = std::min(std::max(kArrowMinOffset, tip_y),
-                       owner_->height() - kArrowMinOffset);
+      int tip_y;
+      if (tray_arrow_offset_ ==
+          internal::TrayBubbleView::InitParams::kArrowDefaultOffset) {
+        tip_y = owner_->height() - kArrowMinOffset;
+      } else {
+        int pty = y - tray_arrow_offset_;
+        gfx::Point pt(0, pty);
+        views::View::ConvertPointToScreen(
+            anchor_->GetWidget()->GetRootView(), &pt);
+        views::View::ConvertPointFromScreen(
+            owner_->GetWidget()->GetRootView(), &pt);
+        tip_y = std::min(pt.y(), owner_->height() - kArrowMinOffset);
+        tip_y = std::max(tip_y, kArrowMinOffset);
+      }
       int top_base_y = tip_y - kArrowWidth / 2;
       int top_base_x, tip_x;
       if (arrow_location() == views::BubbleBorder::LEFT_BOTTOM) {
@@ -184,12 +210,9 @@ class TrayBubbleBorder : public views::BubbleBorder {
                   SkIntToScalar(top_base_y + kArrowWidth));
     }
 
-    views::Background* background = FindAppropriateBackground(owner_,
-                                                              arrow_reference);
-
     SkPaint paint;
     paint.setStyle(SkPaint::kFill_Style);
-    paint.setColor(background ? background->get_color() : kBackgroundColor);
+    paint.setColor(background_color());
     canvas->DrawPath(path, paint);
 
     // Now draw the arrow border.
@@ -210,17 +233,50 @@ class TrayBubbleBorder : public views::BubbleBorder {
 
 namespace internal {
 
+// static
+const int TrayBubbleView::InitParams::kArrowDefaultOffset = -1;
+
+TrayBubbleView::InitParams::InitParams(AnchorType anchor_type,
+                                       ShelfAlignment shelf_alignment)
+    : anchor_type(anchor_type),
+      shelf_alignment(shelf_alignment),
+      bubble_width(kTrayPopupWidth),
+      max_height(0),
+      can_activate(false),
+      arrow_offset(kArrowDefaultOffset),
+      arrow_color(kHeaderBackgroundColorDark) {
+}
+
+TrayBubbleView* TrayBubbleView::Create(views::View* anchor,
+                                       Host* host,
+                                       const InitParams& init_params) {
+  // Set arrow_location here so that it can be passed correctly to the
+  // BubbleView constructor.
+  views::BubbleBorder::ArrowLocation arrow_location;
+  if (init_params.anchor_type == ANCHOR_TYPE_TRAY) {
+    if (init_params.shelf_alignment == SHELF_ALIGNMENT_BOTTOM) {
+      arrow_location = base::i18n::IsRTL() ?
+          views::BubbleBorder::BOTTOM_LEFT : views::BubbleBorder::BOTTOM_RIGHT;
+    } else if (init_params.shelf_alignment == SHELF_ALIGNMENT_LEFT) {
+      arrow_location = views::BubbleBorder::LEFT_BOTTOM;
+    } else {
+      arrow_location = views::BubbleBorder::RIGHT_BOTTOM;
+    }
+  } else {
+    arrow_location = views::BubbleBorder::NONE;
+  }
+
+  return new TrayBubbleView(init_params, arrow_location, anchor, host);
+}
+
 TrayBubbleView::TrayBubbleView(
-    views::View* anchor,
+    const InitParams& init_params,
     views::BubbleBorder::ArrowLocation arrow_location,
-    Host* host,
-    bool can_activate,
-    int bubble_width)
+    views::View* anchor,
+    Host* host)
     : views::BubbleDelegateView(anchor, arrow_location),
-      host_(host),
-      can_activate_(can_activate),
-      max_height_(0),
-      bubble_width_(bubble_width) {
+      params_(init_params),
+      host_(host) {
   set_margins(gfx::Insets());
   set_parent_window(Shell::GetContainer(
       anchor->GetWidget()->GetNativeWindow()->GetRootWindow(),
@@ -236,22 +292,13 @@ TrayBubbleView::~TrayBubbleView() {
     host_->BubbleViewDestroyed();
 }
 
-void TrayBubbleView::SetBubbleBorder(int arrow_offset) {
-  DCHECK(GetWidget());
-  TrayBubbleBorder* bubble_border = new TrayBubbleBorder(
-      this, anchor_view(), arrow_location(), arrow_offset);
-  GetBubbleFrameView()->SetBubbleBorder(bubble_border);
-  // Recalculate size with new border.
-  SizeToContents();
-}
-
 void TrayBubbleView::UpdateBubble() {
   SizeToContents();
   GetWidget()->GetRootView()->SchedulePaint();
 }
 
 void TrayBubbleView::SetMaxHeight(int height) {

[jennyz, 2012/07/31 02:29:47]

This function seems not called in the new code path? Is this intentional? If
yes, we should remove it.

[stevenjb, 2012/07/31 16:35:28]

Still called in SystemTrayBubble::UpdateView()

-  max_height_ = height;
+  params_.max_height = height;
   if (GetWidget())
     SizeToContents();
 }
@@ -266,8 +313,36 @@ void TrayBubbleView::Init() {
 
 gfx::Rect TrayBubbleView::GetAnchorRect() {
   gfx::Rect rect;
-  if (host_)
-    rect = host_->GetAnchorRect();
+
+  if (anchor_widget()->IsVisible()) {
+    rect = anchor_widget()->GetWindowBoundsInScreen();
+    if (params_.anchor_type == ANCHOR_TYPE_TRAY) {
+      if (params_.shelf_alignment == SHELF_ALIGNMENT_BOTTOM) {
+        bool rtl = base::i18n::IsRTL();
+        rect.Inset(
+            rtl ? kPaddingFromRightEdgeOfScreenBottomAlignment : 0,
+            0,
+            rtl ? 0 : kPaddingFromRightEdgeOfScreenBottomAlignment,
+            kPaddingFromBottomOfScreenBottomAlignment);
+      } else if (params_.shelf_alignment == SHELF_ALIGNMENT_LEFT) {
+        rect.Inset(0, 0, kPaddingFromInnerEdgeOfLauncherVerticalAlignment,
+                   kPaddingFromBottomOfScreenVerticalAlignment);
+      } else {
+        rect.Inset(kPaddingFromInnerEdgeOfLauncherVerticalAlignment,
+                   0, 0, kPaddingFromBottomOfScreenVerticalAlignment);
+      }
+    } else if (params_.anchor_type == ANCHOR_TYPE_BUBBLE) {
+      // Invert the offsets to align with the bubble below.
+      if (params_.shelf_alignment == SHELF_ALIGNMENT_LEFT) {
+        rect.Inset(kPaddingFromInnerEdgeOfLauncherVerticalAlignment,
+                   0, 0, kPaddingFromBottomOfScreenVerticalAlignment);
+      } else if (params_.shelf_alignment == SHELF_ALIGNMENT_RIGHT) {
+        rect.Inset(0, 0, kPaddingFromInnerEdgeOfLauncherVerticalAlignment,
+                   kPaddingFromBottomOfScreenVerticalAlignment);
+      }
+    }
+  }
+
   // TODO(jennyz): May need to add left/right alignment in the following code.
   if (rect.IsEmpty()) {
     rect = gfx::Screen::GetPrimaryDisplay().bounds();
@@ -287,15 +362,25 @@ gfx::Rect TrayBubbleView::GetBubbleBounds() {
 }
 
 bool TrayBubbleView::CanActivate() const {
-  return can_activate_;
+  return params_.can_activate;
+}
+
+// Overridden to create BubbleFrameView and set the border to TrayBubbleBorder
+// (instead of creating a default BubbleBorder and replacing it).
+views::NonClientFrameView* TrayBubbleView::CreateNonClientFrameView(
+    views::Widget* widget) {
+  TrayBubbleBorder* bubble_border = new TrayBubbleBorder(
+      this, anchor_view(),
+      arrow_location(), params_.arrow_offset, params_.arrow_color);
+  return new views::BubbleFrameView(margins(), bubble_border);
 }
 
 gfx::Size TrayBubbleView::GetPreferredSize() {
   gfx::Size size = views::BubbleDelegateView::GetPreferredSize();
   int height = size.height();
-  if (max_height_ != 0 && height > max_height_)
-    height = max_height_;
-  return gfx::Size(bubble_width_, height);
+  if (params_.max_height != 0 && height > params_.max_height)
+    height = params_.max_height;
+  return gfx::Size(params_.bubble_width, height);
 }
 
 void TrayBubbleView::OnMouseEntered(const views::MouseEvent& event) {
@@ -309,7 +394,7 @@ void TrayBubbleView::OnMouseExited(const views::MouseEvent& event) {
 }
 
 void TrayBubbleView::GetAccessibleState(ui::AccessibleViewState* state) {
-  if (can_activate_) {
+  if (params_.can_activate) {
     state->role = ui::AccessibilityTypes::ROLE_WINDOW;
     state->name = l10n_util::GetStringUTF16(
         IDS_ASH_STATUS_TRAY_ACCESSIBLE_NAME);
@@ -340,10 +425,28 @@ TrayBubbleView::Host::~Host() {
   Shell::GetInstance()->RemoveEnvEventFilter(this);
 }
 
-void TrayBubbleView::Host::InitializeHost(views::Widget* widget,
-                                          views::View* tray_view) {
+void TrayBubbleView::Host::InitializeAndShowBubble(views::Widget* widget,
+                                                   TrayBubbleView* bubble_view,
+                                                   views::View* tray_view) {
   widget_ = widget;
   tray_view_ = tray_view;
+
+  // Must occur after call to BubbleDelegateView::CreateBubble().
+  bubble_view->SetAlignment(views::BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+
+  // Setup animation.
+  ash::SetWindowVisibilityAnimationType(
+      widget->GetNativeWindow(),
+      ash::WINDOW_VISIBILITY_ANIMATION_TYPE_FADE);
+  ash::SetWindowVisibilityAnimationTransition(
+      widget->GetNativeWindow(),
+      ash::ANIMATE_BOTH);
+  ash::SetWindowVisibilityAnimationDuration(
+      widget->GetNativeWindow(),
+      base::TimeDelta::FromMilliseconds(kAnimationDurationForPopupMS));
+
+  bubble_view->Show();
+  bubble_view->UpdateBubble();
 }
 
 bool TrayBubbleView::Host::PreHandleKeyEvent(aura::Window* target,