Clean-up: Avoids unnecessary font list creation in views::Tab.

Clean-up: Avoids unnecessary font list creation in views::Tab.

There is no need to derive(create) a new font list in views::Tab.  Uses BaseFont and BoldFont.

TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245250

[Yuki, 2014-01-16 14:25:17]

Could you review this CL?

[sadrul, 2014-01-16 14:29:40]

lgtm

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
File ui/views/controls/tabbed_pane/tabbed_pane.cc (right):

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
ui/views/controls/tabbed_pane/tabbed_pane.cc:167:
title_->SetFontList(rb.GetFontList(ui::ResourceBundle::BaseFont));
Just so I understand: this (and the change in line 175) are no-ops, right?

[Yuki, 2014-01-16 14:53:27]

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
File ui/views/controls/tabbed_pane/tabbed_pane.cc (right):

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
ui/views/controls/tabbed_pane/tabbed_pane.cc:167:
title_->SetFontList(rb.GetFontList(ui::ResourceBundle::BaseFont));
On 2014/01/16 14:29:41, sadrul wrote:
> Just so I understand: this (and the change in line 175) are no-ops, right?

My understanding is that
1. When Tab::SetState(TAB_ACTIVE) is called,
   we do title_->SetFontList(BoldFont);
2. When Tab::SetState(TAB_INACTIVE) is called after 1.,
   we *reset* title_->SetFontList(BaseFont);

So, 2. is not no-op.  It resets to the base (non-bold) font list.  It's
necessary.

[sadrul, 2014-01-16 15:17:31]

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
File ui/views/controls/tabbed_pane/tabbed_pane.cc (right):

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
ui/views/controls/tabbed_pane/tabbed_pane.cc:167:
title_->SetFontList(rb.GetFontList(ui::ResourceBundle::BaseFont));
On 2014/01/16 14:53:27, Yuki wrote:
> On 2014/01/16 14:29:41, sadrul wrote:
> > Just so I understand: this (and the change in line 175) are no-ops, right?
> 
> My understanding is that
> 1. When Tab::SetState(TAB_ACTIVE) is called,
>    we do title_->SetFontList(BoldFont);
> 2. When Tab::SetState(TAB_INACTIVE) is called after 1.,
>    we *reset* title_->SetFontList(BaseFont);
> 
> So, 2. is not no-op.  It resets to the base (non-bold) font list.  It's
> necessary.

I was not asking whether it was necessary to call SetFontList. I was asking if
the change from gfx::FontList() to GetFontList(Base) is a no-op (I remember in
an earlier CL someone mentioned that they are equivalent).

[sadrul, 2014-01-16 15:19:18]

On 2014/01/16 15:17:31, sadrul wrote:
>
https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
> File ui/views/controls/tabbed_pane/tabbed_pane.cc (right):
> 
>
https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
> ui/views/controls/tabbed_pane/tabbed_pane.cc:167:
> title_->SetFontList(rb.GetFontList(ui::ResourceBundle::BaseFont));
> On 2014/01/16 14:53:27, Yuki wrote:
> > On 2014/01/16 14:29:41, sadrul wrote:
> > > Just so I understand: this (and the change in line 175) are no-ops, right?
> > 
> > My understanding is that
> > 1. When Tab::SetState(TAB_ACTIVE) is called,
> >    we do title_->SetFontList(BoldFont);
> > 2. When Tab::SetState(TAB_INACTIVE) is called after 1.,
> >    we *reset* title_->SetFontList(BaseFont);
> > 
> > So, 2. is not no-op.  It resets to the base (non-bold) font list.  It's
> > necessary.
> 
> I was not asking whether it was necessary to call SetFontList. I was asking if
> the change from gfx::FontList() to GetFontList(Base) is a no-op (I remember in
> an earlier CL someone mentioned that they are equivalent).

To clarify, I am not suggesting that you undo this change (if they are indeed
equivalent).

[Yuki, 2014-01-16 15:31:02]

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
File ui/views/controls/tabbed_pane/tabbed_pane.cc (right):

https://codereview.chromium.org/137533011/diff/1/ui/views/controls/tabbed_pan...
ui/views/controls/tabbed_pane/tabbed_pane.cc:167:
title_->SetFontList(rb.GetFontList(ui::ResourceBundle::BaseFont));
On 2014/01/16 15:17:31, sadrul wrote:
> On 2014/01/16 14:53:27, Yuki wrote:
> > On 2014/01/16 14:29:41, sadrul wrote:
> > > Just so I understand: this (and the change in line 175) are no-ops, right?
> > 
> > My understanding is that
> > 1. When Tab::SetState(TAB_ACTIVE) is called,
> >    we do title_->SetFontList(BoldFont);
> > 2. When Tab::SetState(TAB_INACTIVE) is called after 1.,
> >    we *reset* title_->SetFontList(BaseFont);
> > 
> > So, 2. is not no-op.  It resets to the base (non-bold) font list.  It's
> > necessary.
> 
> I was not asking whether it was necessary to call SetFontList. I was asking if
> the change from gfx::FontList() to GetFontList(Base) is a no-op (I remember in
> an earlier CL someone mentioned that they are equivalent).

Yes, they're equivalent.  The default instance of FontList is the same as
BaseFont, except that ui::ResourceBundle::GetFontList() does not create a new
instance of FontList.

[commit-bot: I haz the power, 2014-01-16 16:12:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yukishiino@chromium.org/137533011/1

[commit-bot: I haz the power, 2014-01-16 18:09:17]

Change committed as 245250