Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(334)

Issue 117903006: Refactor: Makes menus use gfx::FontList instead of gfx::Font. (Closed)

Created:
6 years, 11 months ago by Yuki
Modified:
6 years, 11 months ago
Reviewers:
sky
CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, tfarina, dcheng, oshima+watch_chromium.org, kalyank, stevenjb+watch_chromium.org, ben+ash_chromium.org
Visibility:
Public.

Description

Refactor: Makes menus use gfx::FontList instead of gfx::Font. As part of effort to support multiple fonts, this CL changes methods which are now taking gfx::Font so they will take gfx::FontList instead. See https://docs.google.com/a/chromium.org/document/d/1D_25fp9B8b9aZJORfAjDIFq61NWvUquZ5xmKH-VcC4k/view BUG=265485 TEST=Run unit_tests, ui_unittests, views_unittests Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244684

Patch Set 1 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+136 lines, -134 lines) Patch
M ash/shelf/shelf_view.cc View 2 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/status/network_menu.cc View 2 chunks +6 lines, -7 lines 0 comments Download
M chrome/browser/ui/cocoa/wrench_menu/wrench_menu_controller.mm View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc View 5 chunks +9 lines, -8 lines 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model_unittest.cc View 4 chunks +27 lines, -27 lines 0 comments Download
M chrome/browser/ui/views/toolbar/wrench_menu.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/toolbar/wrench_menu.cc View 4 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/network_dropdown.cc View 2 chunks +4 lines, -5 lines 0 comments Download
M ui/base/cocoa/menu_controller.mm View 1 chunk +4 lines, -3 lines 0 comments Download
M ui/base/cocoa/menu_controller_unittest.mm View 2 chunks +14 lines, -12 lines 0 comments Download
M ui/base/models/menu_model.h View 2 chunks +4 lines, -4 lines 0 comments Download
M ui/base/models/menu_model.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/menu/menu_config.h View 2 chunks +3 lines, -3 lines 0 comments Download
M ui/views/controls/menu/menu_config_views.cc View 1 chunk +0 lines, -1 line 2 comments Download
M ui/views/controls/menu/menu_config_win.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/menu/menu_delegate.h View 2 chunks +2 lines, -2 lines 0 comments Download
M ui/views/controls/menu/menu_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/menu/menu_item_view.h View 2 chunks +3 lines, -3 lines 0 comments Download
M ui/views/controls/menu/menu_item_view.cc View 8 chunks +29 lines, -31 lines 0 comments Download
M ui/views/controls/menu/menu_model_adapter.h View 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/menu/menu_model_adapter.cc View 1 chunk +5 lines, -5 lines 0 comments Download
M ui/views/controls/menu/menu_model_adapter_unittest.cc View 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Yuki
Hi Scott, Could you kindly review this CL? The purpose of this CL is, in ...
6 years, 11 months ago (2014-01-06 12:05:21 UTC) #1
sky
Before you continue with this 330975 needs to be addressed.
6 years, 11 months ago (2014-01-06 19:25:44 UTC) #2
Yuki
On 2014/01/06 19:25:44, sky wrote: > Before you continue with this 330975 needs to be ...
6 years, 11 months ago (2014-01-07 12:59:32 UTC) #3
sky
LGTM https://codereview.chromium.org/117903006/diff/190001/ui/views/controls/menu/menu_config_views.cc File ui/views/controls/menu/menu_config_views.cc (left): https://codereview.chromium.org/117903006/diff/190001/ui/views/controls/menu/menu_config_views.cc#oldcode46 ui/views/controls/menu/menu_config_views.cc:46: font = rb.GetFont(ResourceBundle::BaseFont); I'm assuming this is the ...
6 years, 11 months ago (2014-01-07 17:14:32 UTC) #4
Yuki
Thanks for the review. https://codereview.chromium.org/117903006/diff/190001/ui/views/controls/menu/menu_config_views.cc File ui/views/controls/menu/menu_config_views.cc (left): https://codereview.chromium.org/117903006/diff/190001/ui/views/controls/menu/menu_config_views.cc#oldcode46 ui/views/controls/menu/menu_config_views.cc:46: font = rb.GetFont(ResourceBundle::BaseFont); On 2014/01/07 ...
6 years, 11 months ago (2014-01-08 06:35:02 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yukishiino@chromium.org/117903006/190001
6 years, 11 months ago (2014-01-14 04:50:27 UTC) #6
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) content_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=245923
6 years, 11 months ago (2014-01-14 07:38:11 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yukishiino@chromium.org/117903006/190001
6 years, 11 months ago (2014-01-14 08:30:41 UTC) #8
commit-bot: I haz the power
6 years, 11 months ago (2014-01-14 13:49:04 UTC) #9
Message was sent while issue was closed.
Change committed as 244684

Powered by Google App Engine
This is Rietveld 408576698