Fix up extension badge text drawing.

Fall abck to the system default font if the preferred font isn't available.

Also, only create the SkPaint struct once, not on every paint.

Also, make some more things const.

BUG=25693
TEST=badge still looks good, that crash goes away

Review URL: http://codereview.chromium.org/341045

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

11 files changed, 90 insertions, 54 deletions

diff --git a/app/gfx/font.h b/app/gfx/font.h
index 4bea194..ccc1a64 100644
--- a/app/gfx/font.h
+++ b/app/gfx/font.h
@@ -97,7 +97,7 @@ class Font {
   // Font Name.
   // It is actually a font family name, because Skia expects a family name
   // and not a font name.
-  std::wstring FontName();
+  const std::wstring& FontName() const;
 
   // Font Size.
   int FontSize();
@@ -175,6 +175,7 @@ class Font {
     int ave_char_width() const { return ave_char_width_; }
     int style() const { return style_; }
     int dlu_base_x() const { return dlu_base_x_; }
+    const std::wstring& font_name() const { return font_name_; }
 
    private:
     const HFONT hfont_;
@@ -184,6 +185,7 @@ class Font {
     const int style_;
     // Constants used in converting dialog units to pixels.
     const int dlu_base_x_;
+    std::wstring font_name_;
 
     DISALLOW_COPY_AND_ASSIGN(HFontRef);
   };
diff --git a/app/gfx/font_mac.mm b/app/gfx/font_mac.mm
index 4d0a48e..56cc6bc 100644
--- a/app/gfx/font_mac.mm
+++ b/app/gfx/font_mac.mm
@@ -74,7 +74,7 @@ int Font::style() const {
   return style_;
 }
 
-std::wstring Font::FontName() {
+const std::wstring& Font::FontName() const {
   return font_name_;
 }
 
diff --git a/app/gfx/font_skia.cc b/app/gfx/font_skia.cc
index a283306..8cf0280 100755
--- a/app/gfx/font_skia.cc
+++ b/app/gfx/font_skia.cc
@@ -261,7 +261,7 @@ int Font::style() const {
   return style_;
 }
 
-std::wstring Font::FontName() {
+const std::wstring& Font::FontName() const {
   return font_family_;
 }
 
diff --git a/app/gfx/font_win.cc b/app/gfx/font_win.cc
index 5e22efe..1f69afa 100644
--- a/app/gfx/font_win.cc
+++ b/app/gfx/font_win.cc
@@ -111,10 +111,8 @@ Font::HFontRef* Font::GetBaseFontRef() {
   return base_font_ref_;
 }
 
-std::wstring Font::FontName() {
-  LOGFONT font_info;
-  GetObject(hfont(), sizeof(LOGFONT), &font_info);
-  return (std::wstring(font_info.lfFaceName));
+const std::wstring& Font::FontName() const {
+  return font_ref_->font_name();
 }
 
 int Font::FontSize() {
@@ -145,6 +143,10 @@ Font::HFontRef::HFontRef(HFONT hfont,
       style_(style),
       dlu_base_x_(dlu_base_x) {
   DLOG_ASSERT(hfont);
+
+  LOGFONT font_info;
+  GetObject(hfont_, sizeof(LOGFONT), &font_info);
+  font_name_ = std::wstring(font_info.lfFaceName);
 }
 
 Font::HFontRef::~HFontRef() {
diff --git a/app/resource_bundle.cc b/app/resource_bundle.cc
index 28674a8..a2ebdf0 100644
--- a/app/resource_bundle.cc
+++ b/app/resource_bundle.cc
@@ -176,7 +176,7 @@ void ResourceBundle::LoadFontsIfNecessary() {
   }
 }
 
-gfx::Font ResourceBundle::GetFont(FontStyle style) {
+const gfx::Font& ResourceBundle::GetFont(FontStyle style) {
   LoadFontsIfNecessary();
   switch (style) {
     case SmallFont:
diff --git a/app/resource_bundle.h b/app/resource_bundle.h
index f2fdb58..a59cbfd 100644
--- a/app/resource_bundle.h
+++ b/app/resource_bundle.h
@@ -113,7 +113,7 @@ class ResourceBundle {
   string16 GetLocalizedString(int message_id);
 
   // Returns the font for the specified style.
-  gfx::Font GetFont(FontStyle style);
+  const gfx::Font& GetFont(FontStyle style);
 
 #if defined(OS_WIN)
   // Loads and returns an icon from the theme dll.
diff --git a/chrome/browser/gtk/tabs/tab_renderer_gtk.cc b/chrome/browser/gtk/tabs/tab_renderer_gtk.cc
index e41f6a5..50502fc 100644
--- a/chrome/browser/gtk/tabs/tab_renderer_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_renderer_gtk.cc
@@ -1039,7 +1039,7 @@ void TabRendererGtk::InitResources() {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   // Force the font size to 9pt, which matches Windows' default font size
   // (taken from the system).
-  gfx::Font base_font = rb.GetFont(ResourceBundle::BaseFont);
+  const gfx::Font& base_font = rb.GetFont(ResourceBundle::BaseFont);
   title_font_ = new gfx::Font(gfx::Font::CreateFont(base_font.FontName(), 9));
   title_font_height_ = title_font_->height();
 
diff --git a/chrome/browser/views/first_run_bubble.cc b/chrome/browser/views/first_run_bubble.cc
index af628cb..a0233be4 100644
--- a/chrome/browser/views/first_run_bubble.cc
+++ b/chrome/browser/views/first_run_bubble.cc
@@ -109,7 +109,7 @@ FirstRunBubbleView::FirstRunBubbleView(FirstRunBubble* bubble_window,
       label3_(NULL),
       keep_button_(NULL),
       change_button_(NULL) {
-  gfx::Font& font =
+  const gfx::Font& font =
       ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::MediumFont);
 
   label1_ = new views::Label(l10n_util::GetString(IDS_FR_BUBBLE_TITLE));
@@ -261,7 +261,7 @@ FirstRunOEMBubbleView::FirstRunOEMBubbleView(FirstRunBubble* bubble_window,
       label3_(NULL),
       close_button_(NULL) {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-  gfx::Font& font = rb.GetFont(ResourceBundle::MediumFont);
+  const gfx::Font& font = rb.GetFont(ResourceBundle::MediumFont);
 
   label1_ = new views::Label(l10n_util::GetString(IDS_FR_OEM_BUBBLE_TITLE_1));
   label1_->SetFont(font.DeriveFont(3, gfx::Font::BOLD));
diff --git a/chrome/browser/views/options/languages_page_view.cc b/chrome/browser/views/options/languages_page_view.cc
index 54b4466..768fc98 100644
--- a/chrome/browser/views/options/languages_page_view.cc
+++ b/chrome/browser/views/options/languages_page_view.cc
@@ -139,7 +139,7 @@ void AddLanguageWindowView::Layout() {
 
 gfx::Size AddLanguageWindowView::GetPreferredSize() {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-  gfx::Font font = rb.GetFont(ResourceBundle::BaseFont);
+  const gfx::Font& font = rb.GetFont(ResourceBundle::BaseFont);
   return gfx::Size(font.ave_char_width() * kDefaultWindowWidthChars,
                    font.height() * kDefaultWindowHeightLines);
 }
diff --git a/chrome/browser/views/options/options_group_view.cc b/chrome/browser/views/options/options_group_view.cc
index 5b93bb2..f0c2974 100644
--- a/chrome/browser/views/options/options_group_view.cc
+++ b/chrome/browser/views/options/options_group_view.cc
@@ -36,7 +36,7 @@ OptionsGroupView::OptionsGroupView(views::View* contents,
       show_separator_(show_separator),
       highlighted_(false) {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-  gfx::Font title_font =
+  const gfx::Font& title_font =
       rb.GetFont(ResourceBundle::BaseFont).DeriveFont(0, gfx::Font::BOLD);
   title_label_->SetFont(title_font);
   SkColor title_color = gfx::NativeTheme::instance()->GetThemeColorWithDefault(
@@ -92,7 +92,7 @@ void OptionsGroupView::Init() {
   SetLayoutManager(layout);
 
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-  gfx::Font font = rb.GetFont(ResourceBundle::BaseFont);
+  const gfx::Font& font = rb.GetFont(ResourceBundle::BaseFont);
   std::wstring left_column_chars =
       l10n_util::GetString(IDS_OPTIONS_DIALOG_LEFT_COLUMN_WIDTH_CHARS);
   int left_column_width =
diff --git a/chrome/common/extensions/extension_action.cc b/chrome/common/extensions/extension_action.cc
index 4a46a81..47e2439 100755
--- a/chrome/common/extensions/extension_action.cc
+++ b/chrome/common/extensions/extension_action.cc
@@ -5,14 +5,81 @@
 #include "chrome/common/extensions/extension_action.h"
 
 #include "app/gfx/canvas.h"
+#include "app/gfx/font.h"
 #include "app/resource_bundle.h"
 #include "base/gfx/rect.h"
+#include "base/string_util.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "grit/app_resources.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkTypeface.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
 
+namespace {
+
+// Different platforms need slightly different constants to look good.
+#if defined(OS_LINUX)
+const int kTextSize = 9;
+const int kBottomMargin = 0;
+const int kPadding = 2;
+const int kTopTextPadding = 0;
+const int kBadgeHeight = 11;
+const int kMaxTextWidth = 23;
+// The minimum width for center-aligning the badge.
+const int kCenterAlignThreshold = 20;
+#else
+const int kTextSize = 8;
+const int kBottomMargin = 5;
+const int kPadding = 2;
+// The padding between the top of the badge and the top of the text.
+const int kTopTextPadding = 1;
+const int kBadgeHeight = 11;
+const int kMaxTextWidth = 23;
+// The minimum width for center-aligning the badge.
+const int kCenterAlignThreshold = 20;
+#endif
+
+#if defined(OS_MACOSX)
+const char kPreferredTypeface[] = "Helvetica";
+#else
+const char kPreferredTypeface[] = "Arial";
+#endif
+
+SkPaint* GetTextPaint() {
+  static SkPaint* text_paint = NULL;
+  if (!text_paint) {
+    text_paint = new SkPaint;
+    text_paint->setAntiAlias(true);
+
+    text_paint->setTextAlign(SkPaint::kLeft_Align);
+    text_paint->setTextSize(SkIntToScalar(kTextSize));
+
+    SkTypeface* typeface = SkTypeface::CreateFromName(
+        kPreferredTypeface, SkTypeface::kBold);
+    // Skia doesn't do any font fallback---if the user is missing the font then
+    // typeface will be NULL. If we don't do manual fallback then we'll crash.
+    if (typeface) {
+      text_paint->setFakeBoldText(true);
+    } else {
+      // Fall back to the system font. We don't bold it because we aren't sure
+      // how it will look.
+      // For the most part this code path will only be hit on Linux systems
+      // that don't have Arial.
+      ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+      const gfx::Font& base_font = rb.GetFont(ResourceBundle::BaseFont);
+      typeface = SkTypeface::CreateFromName(
+          WideToUTF8(base_font.FontName()).c_str(), SkTypeface::kNormal);
+    }
+
+    text_paint->setTypeface(typeface);
+    // |text_paint| adds its own ref. Release the ref from CreateFontName.
+    typeface->unref();
+  }
+  return text_paint;
+}
+
+}  // namespace
+
 const int ExtensionAction::kDefaultTabId = -1;
 
 void ExtensionAction::ClearAllValuesForTab(int tab_id) {
@@ -41,48 +108,13 @@ void ExtensionAction::PaintBadge(gfx::Canvas* canvas,
   if (SkColorGetA(background_color) == 0x00)
     background_color = SkColorSetARGB(255, 218, 0, 24);  // default badge color
 
-  // Different platforms need slightly different constants to look good.
-#if defined(OS_LINUX)
-  const int kTextSize = 9;
-  const int kBottomMargin = 0;
-  const int kPadding = 2;
-  const int kTopTextPadding = 0;
-  const int kBadgeHeight = 11;
-  const int kMaxTextWidth = 23;
-  // The minimum width for center-aligning the badge.
-  const int kCenterAlignThreshold = 20;
-#else
-  const int kTextSize = 8;
-  const int kBottomMargin = 5;
-  const int kPadding = 2;
-  // The padding between the top of the badge and the top of the text.
-  const int kTopTextPadding = 1;
-  const int kBadgeHeight = 11;
-  const int kMaxTextWidth = 23;
-  // The minimum width for center-aligning the badge.
-  const int kCenterAlignThreshold = 20;
-#endif
-
-#if defined(OS_MACOSX)
-  const char kTypeFaceName[] = "Helvetica";
-#else
-  const char kTypeFaceName[] = "Arial";
-#endif
-
   canvas->save();
 
-  SkTypeface* typeface = SkTypeface::CreateFromName(kTypeFaceName,
-                                                    SkTypeface::kBold);
-  SkPaint text_paint;
-  text_paint.setAntiAlias(true);
-  text_paint.setColor(text_color);
-  text_paint.setFakeBoldText(true);
-  text_paint.setTextAlign(SkPaint::kLeft_Align);
-  text_paint.setTextSize(SkIntToScalar(kTextSize));
-  text_paint.setTypeface(typeface);
+  SkPaint* text_paint = GetTextPaint();
+  text_paint->setColor(text_color);
 
   // Calculate text width. We clamp it to a max size.
-  SkScalar text_width = text_paint.measureText(text.c_str(), text.size());
+  SkScalar text_width = text_paint->measureText(text.c_str(), text.size());
   text_width = SkIntToScalar(
       std::min(kMaxTextWidth, SkScalarFloor(text_width)));
 
@@ -137,6 +169,6 @@ void ExtensionAction::PaintBadge(gfx::Canvas* canvas,
   canvas->drawText(text.c_str(), text.size(),
                    rect.fLeft + (rect.width() - text_width) / 2,
                    rect.fTop + kTextSize + kTopTextPadding,
-                   text_paint);
+                   *text_paint);
   canvas->restore();
 }