Clean-up: Replaces Font with FontList in ui/message_center.

Clean-up: Replaces Font with FontList in ui/message_center.

This is a part of the plan to support gfx::FontList in UI components. 
https://docs.google.com/a/chromium.org/document/d/1D_25fp9B8b9aZJORfAjDIFq61NWvUquZ5xmKH-VcC4k/edit

BUG=265485
TEST=Ran message_center_unittests

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

[Yuki, 2013-09-10 08:49:30]

Mukai san, could you review this CL as an owner of ui/message_center ?

[Jun Mukai, 2013-09-10 08:59:41]

+dewittj

also please build&run message_center_unittests to make sure your CL doesn't
cause breaks (and write TEST= line too)

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
File ui/message_center/views/bounded_label.h (right):

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
ui/message_center/views/bounded_label.h:18: }  // namespace gfx
you can omit "// namespace ~" comment in this size of section in the header.

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
ui/message_center/views/bounded_label.h:26: }  // namespace test
ditto

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/notif...
File ui/message_center/views/notification_view.cc (right):

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/notif...
ui/message_center/views/notification_view.cc:511: default_label_font_list);
should this be |font_list| instead?

[Yuki, 2013-09-10 09:16:34]

Sorry for not writing TEST= line.
I'd already run message_center_unittests.

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
File ui/message_center/views/bounded_label.h (right):

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
ui/message_center/views/bounded_label.h:18: }  // namespace gfx
On 2013/09/10 08:59:41, Jun Mukai wrote:
> you can omit "// namespace ~" comment in this size of section in the header.

I know some of people omit it, but I was not able to find if it's okay or not in
the style guide, or in what cases we should omit it if it's okay.

As I read the style guide, it's at least okay to always put an ending comment. 
Considering there is no description how to omit it, probably it's recommended to
always write it.

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
ui/message_center/views/bounded_label.h:26: }  // namespace test
On 2013/09/10 08:59:41, Jun Mukai wrote:
> ditto

Done.

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/notif...
File ui/message_center/views/notification_view.cc (right):

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/notif...
ui/message_center/views/notification_view.cc:511: default_label_font_list);
On 2013/09/10 08:59:41, Jun Mukai wrote:
> should this be |font_list| instead?

No, the original code expected the default font.
So, this should be |default_label_font_list|.
|font_list| is larger by 2 px.

[Jun Mukai, 2013-09-10 17:29:46]

lgtm

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
File ui/message_center/views/bounded_label.h (right):

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
ui/message_center/views/bounded_label.h:18: }  // namespace gfx
On 2013/09/10 09:16:34, Yuki wrote:
> On 2013/09/10 08:59:41, Jun Mukai wrote:
> > you can omit "// namespace ~" comment in this size of section in the header.
> 
> I know some of people omit it, but I was not able to find if it's okay or not
in
> the style guide, or in what cases we should omit it if it's okay.
> 
> As I read the style guide, it's at least okay to always put an ending comment.

> Considering there is no description how to omit it, probably it's recommended
to
> always write it.

If the coding style doesn't say specifically, usually follow the convention on
that module. As I quickly checked, ui/message_center tends to omit this thing,
see message_center.h or notification_list.h or views/message_popup_collection.h
(except for views/message_center_view.h)

It's still up to you though, you can go ahead with this, but I'd prefer to omit.

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/notif...
File ui/message_center/views/notification_view.cc (right):

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/notif...
ui/message_center/views/notification_view.cc:511: default_label_font_list);
On 2013/09/10 09:16:34, Yuki wrote:
> On 2013/09/10 08:59:41, Jun Mukai wrote:
> > should this be |font_list| instead?
> 
> No, the original code expected the default font.
> So, this should be |default_label_font_list|.
> |font_list| is larger by 2 px.

You're right. thanks.

[dewittj, 2013-09-10 20:33:11]

I noticed there are no changes for cocoa.  Do we need to refactor
ui/message_center/cocoa to use fontlists too?

[Yuki, 2013-09-11 12:53:18]

I don't think we need to refactor cocoa part.

I'm not 100% sure, but Cocoa seems not supporting specifying multiple fonts.  As
long as Cocoa doesn't support multiple fonts specification, we don't need any
font list support in Cocoa part.  It's simply meaningless.

Views, which is used on Chrome OS, needs to support font list because Chrome OS
supports multiple fonts specification.

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
File ui/message_center/views/bounded_label.h (right):

https://codereview.chromium.org/24084003/diff/1/ui/message_center/views/bound...
ui/message_center/views/bounded_label.h:18: }  // namespace gfx
On 2013/09/10 17:29:46, Jun Mukai wrote:
> On 2013/09/10 09:16:34, Yuki wrote:
> > On 2013/09/10 08:59:41, Jun Mukai wrote:
> > > you can omit "// namespace ~" comment in this size of section in the
header.
> > 
> > I know some of people omit it, but I was not able to find if it's okay or
not
> in
> > the style guide, or in what cases we should omit it if it's okay.
> > 
> > As I read the style guide, it's at least okay to always put an ending
comment.
> 
> > Considering there is no description how to omit it, probably it's
recommended
> to
> > always write it.
> 
> If the coding style doesn't say specifically, usually follow the convention on
> that module. As I quickly checked, ui/message_center tends to omit this thing,
> see message_center.h or notification_list.h or
views/message_popup_collection.h
> (except for views/message_center_view.h)
> 
> It's still up to you though, you can go ahead with this, but I'd prefer to
omit.

Done.

[Yuki, 2013-09-13 06:49:41]

Ping?
Justin, do you have any other concerns?

[dewittj, 2013-09-13 20:00:24]

lgtm

[commit-bot: I haz the power, 2013-09-16 07:02:41]

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

[commit-bot: I haz the power, 2013-09-16 09:37:12]

Change committed as 223321