Fix position of web notification bubble and arrow

ui/views/bubble/bubble_border.cc

Index: ui/views/bubble/bubble_border.cc
diff --git a/ui/views/bubble/bubble_border.cc b/ui/views/bubble/bubble_border.cc
index c24f367b2debe1d2d0b65312476e8d1f17167f5d..e292ca8d5b2894236f11e66c61ac7421361f2966 100644
--- a/ui/views/bubble/bubble_border.cc
+++ b/ui/views/bubble/bubble_border.cc
@@ -58,7 +58,8 @@ BubbleBorder::BubbleBorder(ArrowLocation arrow_location, Shadow shadow)
     : override_arrow_offset_(0),
       arrow_location_(arrow_location),
       alignment_(ALIGN_ARROW_TO_MID_ANCHOR),
-      background_color_(SK_ColorWHITE) {
+      background_color_(SK_ColorWHITE),
+      rtl_mirrored_(true) {
   images_ = GetBorderImages(shadow);
 
   // Calculate horizontal and vertical insets for arrow by ensuring that
@@ -174,11 +175,22 @@ gfx::Rect BubbleBorder::GetBounds(const gfx::Rect& position_relative_to,
 }
 
 void BubbleBorder::GetInsets(gfx::Insets* insets) const {
+  // Insets will get mirrored in the View code for normal layout. If

[msw, 2012/07/23 22:20:49]

Don't you need to update arrow_location() to conditionally suppress flipping the
actual arrow location for |rtl_mirrored_|? Otherwise, it seems like this will
keep the insets but still flip the arrow.

[stevenjb, 2012/07/23 23:32:06]

See comment below.

+  // rtl_mirrored_ is false, invert the layout here so that when they are
+  // mirrored in the View code the insets are correct.
+  ArrowLocation arrow_loc = arrow_location_;
+  if (!rtl_mirrored_ && base::i18n::IsRTL())
+    arrow_loc = horizontal_mirror(arrow_loc);
+  return GetInsetsForArrowLocation(insets, arrow_loc);
+}
+
+void BubbleBorder::GetInsetsForArrowLocation(gfx::Insets* insets,

[msw, 2012/07/23 22:20:49]

Can you merge this function back into GetInsets() and just build the conditional
arrow mirroring logic into a new GetArrowLocation() that replaces
arrow_location()?

[stevenjb, 2012/07/23 23:32:06]

The problem is that most of the code in BubbleBorder needs arrow_location to be
mirrored (as set in BubbleDelegateView::CreateNonClientFrameView, previously set
in BubbleFrameView::BubbleFrameView). TrayBubbleBorder::Paint however needs it
not to be mirrored.
 

[msw, 2012/07/24 00:32:44]

I guess I'm unfamiliar with the use case, but this sounds odd to me. Why should
the arrow be mirrored everywhere except in that one instance? See my suggestion
there to de-duplicate the arrow painting code.

+                                             ArrowLocation arrow_loc) const {
   int top = images_->top->height();
   int bottom = images_->bottom->height();
   int left = images_->left->width();
   int right = images_->right->width();
-  switch (arrow_location_) {
+  switch (arrow_loc) {
     case TOP_LEFT:
     case TOP_RIGHT:
       top = std::max(top, images_->top_arrow->height());