Renames gfx::FontList::Derive* family.

ui/app_list/views/speech_view.cc

Index: ui/app_list/views/speech_view.cc
diff --git a/ui/app_list/views/speech_view.cc b/ui/app_list/views/speech_view.cc
index c6bad7c4549b49784b51a01cca22d7698f78adab..e53fdf1a13bd80038e38a722223510d30f7aa95c 100644
--- a/ui/app_list/views/speech_view.cc
+++ b/ui/app_list/views/speech_view.cc
@@ -27,7 +27,6 @@ namespace {
 const int kShadowOffset = 1;
 const int kShadowBlur = 4;
 const int kSpeechViewMaxHeight = 300;
-const int kTextSize = 20;
 const int kMicButtonMargin = 12;
 const int kTextMargin = 32;
 const int kIndicatorRadiusMax = 100;
@@ -116,10 +115,11 @@ SpeechView::SpeechView(AppListViewDelegate* delegate)
   container->AddChildView(mic_button_);
 
   // TODO(mukai): use BoundedLabel to cap 2 lines.
-  speech_result_ = new views::Label();
+  ui::ResourceBundle* bundle = &ui::ResourceBundle::GetSharedInstance();

[xiyuan, 2014/01/30 03:49:27]

Thought "ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();" is
more popular in current code base.

[Jun Mukai, 2014/01/30 04:02:37]

+1 to xiyuan

[Yuki, 2014/01/30 05:30:35]

Our policy is to use a const reference or a pointer (to a mutable object). 
ui::ResourceBundle is violating this guideline.  GetSharedInstance() should
return ui::ResourceBundle* instead of ui::ResourceBundle&.

This is the reason why I used ui::ResourceBundle* here.
I personally don't insist this point, but there are not few reviewers who highly
recommend a pointer to a mutable object.

[Jun Mukai, 2014/01/30 06:00:56]

I don't want to argue here and I'm really happy if you want to refactor
ResourceBundle.
However, for some reasons I don't know, ui::ResourceBundle is widely used as
non-const reference right now, all among our code base.
see;
https://code.google.com/p/chromium/codesearch#search/&q=ui::ResourceBundle&am...

This situation isn't ideal, but that doesn't justify using a pointer here by
itself.

Anyways, I think there are two ways. Stop this CL and do your refactoring of
ResourceBundle first, or move this with const reference and revisit the issue of
ResourceBundle::SharedInstance() later.
Either is fine.

[Yuki, 2014/01/30 06:29:56]

Done.

+  speech_result_ = new views::Label(
+      base::string16(), bundle->GetFontList(ui::ResourceBundle::LargeFont));

[Jun Mukai, 2014/01/30 04:02:37]

Not sure how large is LargeFont. Could you give me the data?

[Jun Mukai, 2014/01/30 04:10:10]

note that the spec says that this text is 20sp. I don't think we have to follow
this exactly but want to choose the closest one.

[xiyuan, 2014/01/30 05:18:36]

My impression is that ChromeOS base font is defined in IDS_UI_FONT_FAMILY_CROS,
which is "Noto Sans UI,ui-sans, 12px" for en. LargeFont is base font size +
kLargeFontSizeDelta (which is 8). It should be 20px for en.

But be warned that I might be totally wrong. :) Let's see what Yuki responds.

[Yuki, 2014/01/30 05:30:35]

The current definition of font sizes are as follows.  They may change in future
though.

SmallFont = -1px relative to the default font size
MediumFont = +3px relative to the default font size
LargeFont = +8px relative to the default font size

Typical default font size would be 12px or 13px, but could be a different size
depending on platforms and user's settings.

The current definition on CrOS is 12px in English UI and 13px in most other
languages.  On Windows, it depends on user's font settings.
https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/strings/ap...

[Jun Mukai, 2014/01/30 06:00:56]

okay, go ahead.

   speech_result_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  const gfx::FontList& font_list = speech_result_->font_list();
-  speech_result_->SetFontList(font_list.DeriveFontListWithSize(kTextSize));
+
   speech_result_->SetMultiLine(true);
   container->AddChildView(speech_result_);
 
@@ -138,12 +138,12 @@ SpeechView::~SpeechView() {
 }
 
 void SpeechView::Reset() {
-  ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+  ui::ResourceBundle* bundle = &ui::ResourceBundle::GetSharedInstance();

[Jun Mukai, 2014/01/30 04:02:37]

ditto

[Yuki, 2014/01/30 06:29:56]

Done.

   speech_result_->SetText(l10n_util::GetStringUTF16(
       IDS_APP_LIST_SPEECH_HINT_TEXT));
   speech_result_->SetEnabledColor(kHintTextColor);
-  mic_button_->SetImage(views::Button::STATE_NORMAL, bundle.GetImageSkiaNamed(
-      IDR_APP_LIST_SPEECH_MIC_ON));
+  mic_button_->SetImage(views::Button::STATE_NORMAL,
+                        bundle->GetImageSkiaNamed(IDR_APP_LIST_SPEECH_MIC_ON));
 }
 
 int SpeechView::GetIndicatorRadius(uint8 level) {
@@ -214,9 +214,9 @@ void SpeechView::OnSpeechRecognitionStateChanged(
   else if (new_state == SPEECH_RECOGNITION_IN_SPEECH)
     resource_id = IDR_APP_LIST_SPEECH_MIC_RECORDING;
 
-  ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
-  mic_button_->SetImage(views::Button::STATE_NORMAL, bundle.GetImageSkiaNamed(
-      resource_id));
+  ui::ResourceBundle* bundle = &ui::ResourceBundle::GetSharedInstance();
+  mic_button_->SetImage(views::Button::STATE_NORMAL,
+                        bundle->GetImageSkiaNamed(resource_id));
 }
 
 }  // namespace app_list