Fix some infobar drawing glitches:

  * Glass mode popups used the wrong separator color, making the arrow look weird
  * The stroke/fill were not being drawn quite correctly due to some subtle AA-related issues

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/6875017

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

16 files changed, 76 insertions, 49 deletions

diff --git a/chrome/browser/external_tab_container_win.cc b/chrome/browser/external_tab_container_win.cc
index a6fdfab..4894c2d 100644
--- a/chrome/browser/external_tab_container_win.cc
+++ b/chrome/browser/external_tab_container_win.cc
@@ -44,6 +44,7 @@
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/base/view_prop.h"
 #include "views/layout/grid_layout.h"
 #include "views/widget/root_view.h"
@@ -909,6 +910,10 @@ scoped_refptr<ExternalTabContainer> ExternalTabContainer::RemovePendingTab(
   return NULL;
 }
 
+SkColor ExternalTabContainer::GetInfoBarSeparatorColor() const {
+  return ResourceBundle::toolbar_separator_color;
+}
+
 void ExternalTabContainer::InfoBarContainerHeightChanged(bool is_animating) {
   if (external_tab_view_)
     external_tab_view_->Layout();
diff --git a/chrome/browser/external_tab_container_win.h b/chrome/browser/external_tab_container_win.h
index 1a2af72..5747b4d 100644
--- a/chrome/browser/external_tab_container_win.h
+++ b/chrome/browser/external_tab_container_win.h
@@ -205,6 +205,7 @@ class ExternalTabContainer : public TabContentsDelegate,
   }
 
   // InfoBarContainer::Delegate overrides
+  virtual SkColor GetInfoBarSeparatorColor() const OVERRIDE;
   virtual void InfoBarContainerHeightChanged(bool is_animating) OVERRIDE;
   virtual bool DrawInfoBarArrows(int* x) const OVERRIDE;
 
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 31cf854..c8d13b8 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -1653,6 +1653,13 @@ void BrowserView::GetAccessibleState(ui::AccessibleViewState* state) {
   state->role = ui::AccessibilityTypes::ROLE_CLIENT;
 }
 
+SkColor BrowserView::GetInfoBarSeparatorColor() const {
+  // NOTE: Keep this in sync with ToolbarView::OnPaint()!
+  return (IsTabStripVisible() ||
+          !frame_->GetWindow()->non_client_view()->UseNativeFrame()) ?
+      ResourceBundle::toolbar_separator_color : SK_ColorBLACK;
+}
+
 void BrowserView::InfoBarContainerHeightChanged(bool is_animating) {
   ToolbarSizeChanged(is_animating);
 }
diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h
index 0232918..285c9a6 100644
--- a/chrome/browser/ui/views/frame/browser_view.h
+++ b/chrome/browser/ui/views/frame/browser_view.h
@@ -374,6 +374,7 @@ class BrowserView : public BrowserBubbleHost,
   virtual gfx::Size GetMinimumSize() OVERRIDE;
 
   // InfoBarContainer::Delegate overrides
+  virtual SkColor GetInfoBarSeparatorColor() const OVERRIDE;
   virtual void InfoBarContainerHeightChanged(bool is_animating) OVERRIDE;
   virtual bool DrawInfoBarArrows(int* x) const OVERRIDE;
 
diff --git a/chrome/browser/ui/views/infobars/infobar.cc b/chrome/browser/ui/views/infobars/infobar.cc
index 1cc03ba..1d32362 100644
--- a/chrome/browser/ui/views/infobars/infobar.cc
+++ b/chrome/browser/ui/views/infobars/infobar.cc
@@ -65,10 +65,6 @@ int InfoBar::OffsetY(const gfx::Size& prefsize) const {
       (bar_target_height_ - bar_height_);
 }
 
-bool InfoBar::DrawInfoBarArrows(int* x) const {
-  return container_ && container_->DrawInfoBarArrows(x);
-}
-
 void InfoBar::AnimationEnded(const ui::Animation* animation) {
   RecalculateHeight();
   MaybeDelete();
diff --git a/chrome/browser/ui/views/infobars/infobar.h b/chrome/browser/ui/views/infobars/infobar.h
index 6f1769a..ed9250e 100644
--- a/chrome/browser/ui/views/infobars/infobar.h
+++ b/chrome/browser/ui/views/infobars/infobar.h
@@ -60,9 +60,7 @@ class InfoBar : public ui::AnimationDelegate {
   // out) as we animate open and closed.
   int OffsetY(const gfx::Size& prefsize) const;
 
-  // Passthrough to the container function of the same name.
-  bool DrawInfoBarArrows(int* x) const;
-
+  const InfoBarContainer* container() const { return container_; }
   ui::SlideAnimation* animation() { return animation_.get(); }
   const ui::SlideAnimation* animation() const { return animation_.get(); }
   int bar_height() const { return bar_height_; }
diff --git a/chrome/browser/ui/views/infobars/infobar_background.cc b/chrome/browser/ui/views/infobars/infobar_background.cc
index 43dff3a..b6f94f8 100644
--- a/chrome/browser/ui/views/infobars/infobar_background.cc
+++ b/chrome/browser/ui/views/infobars/infobar_background.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/ui/views/infobars/infobar_background.h"
 
 #include "chrome/browser/ui/views/infobars/infobar_view.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/canvas_skia_paint.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
@@ -15,7 +14,8 @@
 const int InfoBarBackground::kSeparatorLineHeight = 1;
 
 InfoBarBackground::InfoBarBackground(InfoBarDelegate::Type infobar_type)
-    : top_color_(GetTopColor(infobar_type)),
+    : separator_color_(SK_ColorBLACK),
+      top_color_(GetTopColor(infobar_type)),
       bottom_color_(GetBottomColor(infobar_type)) {
 }
 
@@ -56,8 +56,8 @@ void InfoBarBackground::Paint(gfx::Canvas* canvas, views::View* view) const {
   SkPaint paint;
   paint.setStrokeWidth(1.0);
   paint.setStyle(SkPaint::kFill_Style);
+  paint.setStrokeCap(SkPaint::kRound_Cap);
   paint.setShader(gradient_shader);
-  paint.setAntiAlias(true);
   gradient_shader->unref();
 
   InfoBarView* infobar = static_cast<InfoBarView*>(view);
@@ -65,13 +65,13 @@ void InfoBarBackground::Paint(gfx::Canvas* canvas, views::View* view) const {
   canvas_skia->drawPath(*infobar->fill_path(), paint);
 
   paint.setShader(NULL);
-  paint.setColor(SkColorSetA(ResourceBundle::toolbar_separator_color,
+  paint.setColor(SkColorSetA(separator_color_,
                              SkColorGetA(gradient_colors[0])));
   paint.setStyle(SkPaint::kStroke_Style);
   canvas_skia->drawPath(*infobar->stroke_path(), paint);
 
-  // Now draw at the bottom.
-  canvas->FillRectInt(ResourceBundle::toolbar_separator_color, 0,
+  // Now draw the separator at the bottom.
+  canvas->FillRectInt(separator_color_, 0,
                       view->height() - kSeparatorLineHeight, view->width(),
                       kSeparatorLineHeight);
 }
diff --git a/chrome/browser/ui/views/infobars/infobar_background.h b/chrome/browser/ui/views/infobars/infobar_background.h
index e23fed2..f948129 100644
--- a/chrome/browser/ui/views/infobars/infobar_background.h
+++ b/chrome/browser/ui/views/infobars/infobar_background.h
@@ -16,6 +16,7 @@ class InfoBarBackground : public views::Background {
   explicit InfoBarBackground(InfoBarDelegate::Type infobar_type);
   virtual ~InfoBarBackground();
 
+  void set_separator_color(SkColor color) { separator_color_ = color; }
   static SkColor GetTopColor(InfoBarDelegate::Type infobar_type);
   static SkColor GetBottomColor(InfoBarDelegate::Type infobar_type);
 
@@ -23,6 +24,7 @@ class InfoBarBackground : public views::Background {
   // views::Background:
   virtual void Paint(gfx::Canvas* canvas, views::View* view) const;
 
+  SkColor separator_color_;
   SkColor top_color_;
   SkColor bottom_color_;
 
diff --git a/chrome/browser/ui/views/infobars/infobar_container.cc b/chrome/browser/ui/views/infobars/infobar_container.cc
index c4408bc..1fe7436 100644
--- a/chrome/browser/ui/views/infobars/infobar_container.cc
+++ b/chrome/browser/ui/views/infobars/infobar_container.cc
@@ -82,10 +82,6 @@ void InfoBarContainer::OnInfoBarHeightChanged(bool is_animating) {
     delegate_->InfoBarContainerHeightChanged(is_animating);
 }
 
-bool InfoBarContainer::DrawInfoBarArrows(int* x) const {
-  return delegate_ && delegate_->DrawInfoBarArrows(x);
-}
-
 void InfoBarContainer::RemoveDelegate(InfoBarDelegate* delegate) {
   tab_contents_->RemoveInfoBar(delegate);
 }
diff --git a/chrome/browser/ui/views/infobars/infobar_container.h b/chrome/browser/ui/views/infobars/infobar_container.h
index 2ed3fcc..0c0a129 100644
--- a/chrome/browser/ui/views/infobars/infobar_container.h
+++ b/chrome/browser/ui/views/infobars/infobar_container.h
@@ -11,6 +11,7 @@
 #include "base/compiler_specific.h"
 #include "content/common/notification_observer.h"
 #include "content/common/notification_registrar.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 class InfoBar;
 class InfoBarDelegate;
@@ -27,6 +28,9 @@ class InfoBarContainer : public NotificationObserver {
  public:
   class Delegate {
    public:
+    // The separator color may vary depending on where the container is hosted.
+    virtual SkColor GetInfoBarSeparatorColor() const = 0;
+
     // The delegate is notified each time the infobar container changes height.
     virtual void InfoBarContainerHeightChanged(bool is_animating) = 0;
 
@@ -56,9 +60,6 @@ class InfoBarContainer : public NotificationObserver {
   // e.g. re-layout.
   void OnInfoBarHeightChanged(bool is_animating);
 
-  // Passthrough to the delegate function of the same name.
-  bool DrawInfoBarArrows(int* x) const;
-
   // Remove the specified InfoBarDelegate from the selected TabContents. This
   // will notify us back and cause us to close the InfoBar.  This is called from
   // the InfoBar's close button handler.
@@ -69,6 +70,8 @@ class InfoBarContainer : public NotificationObserver {
   // hidden.
   void RemoveInfoBar(InfoBar* infobar);
 
+  const Delegate* delegate() const { return delegate_; }
+
  protected:
   // Subclasses must call this during destruction, so that we can remove
   // infobars (which will call the pure virtual functions below) while the
diff --git a/chrome/browser/ui/views/infobars/infobar_view.cc b/chrome/browser/ui/views/infobars/infobar_view.cc
index f43fc60..4a8af9320 100644
--- a/chrome/browser/ui/views/infobars/infobar_view.cc
+++ b/chrome/browser/ui/views/infobars/infobar_view.cc
@@ -51,9 +51,7 @@ InfoBarView::InfoBarView(InfoBarDelegate* delegate)
       fill_path_(new SkPath),
       stroke_path_(new SkPath) {
   set_parent_owned(false);  // InfoBar deletes itself at the appropriate time.
-
-  InfoBarDelegate::Type infobar_type = delegate->GetInfoBarType();
-  set_background(new InfoBarBackground(infobar_type));
+  set_background(new InfoBarBackground(delegate->GetInfoBarType()));
 }
 
 InfoBarView::~InfoBarView() {
@@ -148,28 +146,33 @@ void InfoBarView::Layout() {
   fill_path_->rewind();
   int arrow_bottom = std::max(arrow_height() - 1, 0);
   SkScalar arrow_bottom_scalar = SkIntToScalar(arrow_bottom);
+  const InfoBarContainer::Delegate* delegate = container_delegate();
   int arrow_x;
-  if (DrawInfoBarArrows(&arrow_x) && arrow_bottom) {
-    stroke_path_->moveTo(SkIntToScalar(arrow_x - arrow_bottom),
-                         arrow_bottom_scalar);
-    stroke_path_->rLineTo(arrow_bottom_scalar, -arrow_bottom_scalar);
-    stroke_path_->rLineTo(arrow_bottom_scalar, arrow_bottom_scalar);
-
-    // Without extending the fill downward by a pixel, Skia doesn't seem to want
-    // to fill over the divider above the bar portion.
-    *fill_path_ = *stroke_path_;
-    fill_path_->rLineTo(0.0, 1.0);
-    fill_path_->rLineTo(-arrow_bottom_scalar * 2, 0.0);
-    fill_path_->close();
-
-    // Fill and stroke have different opinions about how to treat paths.
-    // Because in Skia integral coordinates represent pixel boundaries,
-    // offsetting the path makes it go exactly through pixel centers; this
-    // results in lines that are exactly where we expect, instead of having odd
-    // "off by one" issues.  Were we to do this for |fill_path|, however, which
-    // tries to fill "inside" the path (using some questionable math), we'd get
-    // a fill at a very different place than we'd want.
-    stroke_path_->offset(SK_ScalarHalf, SK_ScalarHalf);
+  if (delegate) {
+    static_cast<InfoBarBackground*>(background())->set_separator_color(
+        delegate->GetInfoBarSeparatorColor());
+    if (delegate->DrawInfoBarArrows(&arrow_x) && arrow_bottom) {
+      stroke_path_->moveTo(SkIntToScalar(arrow_x - arrow_bottom),
+                           arrow_bottom_scalar);
+      stroke_path_->rLineTo(arrow_bottom_scalar, -arrow_bottom_scalar);
+      stroke_path_->rLineTo(arrow_bottom_scalar, arrow_bottom_scalar);
+
+      // Without extending the fill downward by a pixel, Skia doesn't seem to
+      // want to fill over the divider above the bar portion.
+      *fill_path_ = *stroke_path_;
+      fill_path_->rLineTo(0.0, 1.0);
+      fill_path_->rLineTo(-arrow_bottom_scalar * 2, 0.0);
+      fill_path_->close();
+
+      // Fill and stroke have different opinions about how to treat paths.
+      // Because in Skia integral coordinates represent pixel boundaries,
+      // offsetting the path makes it go exactly through pixel centers; this
+      // results in lines that are exactly where we expect, instead of having
+      // odd "off by one" issues.  Were we to do this for |fill_path|, however,
+      // which tries to fill "inside" the path (using some questionable math),
+      // we'd get a fill at a very different place than we'd want.
+      stroke_path_->offset(SK_ScalarHalf, SK_ScalarHalf);
+    }
   }
   if (bar_height()) {
     fill_path_->addRect(0.0, SkIntToScalar(arrow_height()),
@@ -295,6 +298,11 @@ int InfoBarView::EndX() const {
   return close_button_->x() - kCloseButtonSpacing;
 }
 
+const InfoBarContainer::Delegate* InfoBarView::container_delegate() const {
+  const InfoBarContainer* infobar_container = container();
+  return infobar_container ? infobar_container->delegate() : NULL;
+}
+
 void InfoBarView::PlatformSpecificHide(bool animate) {
   if (!animate)
     return;
diff --git a/chrome/browser/ui/views/infobars/infobar_view.h b/chrome/browser/ui/views/infobars/infobar_view.h
index 3b4c948..56bebc8 100644
--- a/chrome/browser/ui/views/infobars/infobar_view.h
+++ b/chrome/browser/ui/views/infobars/infobar_view.h
@@ -8,6 +8,8 @@
 
 #include "base/task.h"
 #include "chrome/browser/ui/views/infobars/infobar.h"
+#include "chrome/browser/ui/views/infobars/infobar_background.h"
+#include "chrome/browser/ui/views/infobars/infobar_container.h"
 #include "views/controls/button/button.h"
 #include "views/focus/focus_manager.h"
 
@@ -80,6 +82,9 @@ class InfoBarView : public InfoBar,
   int StartX() const;
   int EndX() const;
 
+  // Convenience getter.
+  const InfoBarContainer::Delegate* container_delegate() const;
+
  private:
   static const int kHorizontalPadding;
 
diff --git a/chrome/browser/ui/views/infobars/translate_infobar_base.cc b/chrome/browser/ui/views/infobars/translate_infobar_base.cc
index 7337098..f687e11 100644
--- a/chrome/browser/ui/views/infobars/translate_infobar_base.cc
+++ b/chrome/browser/ui/views/infobars/translate_infobar_base.cc
@@ -46,7 +46,6 @@ const int TranslateInfoBarBase::kButtonInLabelSpacing = 5;
 
 TranslateInfoBarBase::TranslateInfoBarBase(TranslateInfoBarDelegate* delegate)
     : InfoBarView(delegate),
-      normal_background_(InfoBarDelegate::PAGE_ACTION_TYPE),
       error_background_(InfoBarDelegate::WARNING_TYPE) {
 }
 
@@ -94,6 +93,12 @@ TranslateInfoBarDelegate* TranslateInfoBarBase::GetDelegate() {
 }
 
 void TranslateInfoBarBase::OnPaintBackground(gfx::Canvas* canvas) {
+  // We need to set the separator color for |error_background_| like
+  // InfoBarView::Layout() does for the normal background.
+  const InfoBarContainer::Delegate* delegate = container_delegate();
+  if (delegate)
+    error_background_.set_separator_color(delegate->GetInfoBarSeparatorColor());
+
   // If we're not animating, simply paint the background for the current state.
   if (!background_color_animation_->is_animating()) {
     GetBackground().Paint(canvas, this);
@@ -101,7 +106,7 @@ void TranslateInfoBarBase::OnPaintBackground(gfx::Canvas* canvas) {
   }
 
   FadeBackground(canvas, 1.0 - background_color_animation_->GetCurrentValue(),
-                 normal_background_);
+                 *background());
   FadeBackground(canvas, background_color_animation_->GetCurrentValue(),
                  error_background_);
 }
@@ -114,7 +119,7 @@ void TranslateInfoBarBase::AnimationProgressed(const ui::Animation* animation) {
 }
 
 const views::Background& TranslateInfoBarBase::GetBackground() {
-  return GetDelegate()->IsError() ? error_background_ : normal_background_;
+  return GetDelegate()->IsError() ? error_background_ : *background();
 }
 
 void TranslateInfoBarBase::FadeBackground(gfx::Canvas* canvas,
diff --git a/chrome/browser/ui/views/infobars/translate_infobar_base.h b/chrome/browser/ui/views/infobars/translate_infobar_base.h
index 11df83e..4a6923d 100644
--- a/chrome/browser/ui/views/infobars/translate_infobar_base.h
+++ b/chrome/browser/ui/views/infobars/translate_infobar_base.h
@@ -53,7 +53,6 @@ class TranslateInfoBarBase : public TranslateInfoBarView,
                       double animation_value,
                       const views::Background& background);
 
-  InfoBarBackground normal_background_;
   InfoBarBackground error_background_;
   scoped_ptr<ui::SlideAnimation> background_color_animation_;
 
diff --git a/chrome/browser/ui/views/toolbar_view.cc b/chrome/browser/ui/views/toolbar_view.cc
index 8cf2bca..4873458 100644
--- a/chrome/browser/ui/views/toolbar_view.cc
+++ b/chrome/browser/ui/views/toolbar_view.cc
@@ -17,7 +17,6 @@
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/browser_actions_container.h"
 #include "chrome/browser/ui/views/event_utils.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/upgrade_detector.h"
 #include "chrome/common/pref_names.h"
 #include "content/common/notification_service.h"
@@ -556,6 +555,7 @@ void ToolbarView::OnPaint(gfx::Canvas* canvas) {
   // For glass, we need to draw a black line below the location bar to separate
   // it from the content area.  For non-glass, the NonClientView draws the
   // toolbar background below the location bar for us.
+  // NOTE: Keep this in sync with BrowserView::GetInfoBarSeparatorColor()!
   if (GetWindow()->non_client_view()->UseNativeFrame())
     canvas->FillRectInt(SK_ColorBLACK, 0, height() - 1, width(), 1);
 }
diff --git a/views/view.h b/views/view.h
index 5f98564..8fe4de8 100644
--- a/views/view.h
+++ b/views/view.h
@@ -534,6 +534,7 @@ class View : public AcceleratorTarget {
   // The background object is owned by this object and may be NULL.
   void set_background(Background* b) { background_.reset(b); }
   const Background* background() const { return background_.get(); }
+  Background* background() { return background_.get(); }
 
   // The border object is owned by this object and may be NULL.
   void set_border(Border* b) { border_.reset(b); }