Fix position of web notification bubble and arrow

ui/views/bubble/bubble_border.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/bubble/bubble_border.h"
 
 #include <algorithm>  // for std::max
 
 #include "base/logging.h"
 #include "grit/ui_resources.h"
@@ skipping 40 matching lines @@
 struct BubbleBorder::BorderImages* BubbleBorder::normal_images_ = NULL;
 struct BubbleBorder::BorderImages* BubbleBorder::shadow_images_ = NULL;
 
 // The height inside the arrow image, in pixels.
 static const int kArrowInteriorHeight = 7;
 
 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
   // the widest arrow and corner images will have enough room to avoid overlap
   int offset_x =
       (std::max(images_->top_arrow->width(),
                 images_->bottom_arrow->width()) / 2) +
       std::max(std::max(images_->top_left->width(),
                         images_->top_right->width()),
                std::max(images_->bottom_left->width(),
@@ skipping 95 matching lines @@
 
     case FLOAT:
       y += h / 2 - border_size.height() / 2;
       break;
   }
 
   return gfx::Rect(x, y, border_size.width(), border_size.height());
 }
 
 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());
       break;
 
     case BOTTOM_LEFT:
     case BOTTOM_RIGHT:
       bottom = std::max(bottom, images_->bottom_arrow->height());
       break;
 
@@ skipping 308 matching lines @@
   paint.setColor(border_->background_color());
   SkPath path;
   gfx::Rect bounds(view->GetContentsBounds());
   bounds.Inset(-border_->border_thickness(), -border_->border_thickness());
   SkScalar radius = SkIntToScalar(BubbleBorder::GetCornerRadius());
   path.addRoundRect(gfx::RectToSkRect(bounds), radius, radius);
   canvas->DrawPath(path, paint);
 }
 
 }  // namespace views