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

Issue 23483015: Clean-up: Replaces Font with FontList in TooltipManager. (Closed)

Created:
7 years, 3 months ago by Yuki
Modified:
7 years, 3 months ago
Reviewers:
sadrul, sky
CC:
chromium-reviews, tfarina, ben+watch_chromium.org
Visibility:
Public.

Description

Clean-up: Replaces Font with FontList in TooltipManager. This is a part of refactoring to replace all use of gfx::Font with gfx::FontList. Also this replacement solves a possible vertical alignment issue of text rendering. See "Background & Problem" section in the following doc why this refactoring is needed. https://docs.google.com/a/chromium.org/document/d/1D_25fp9B8b9aZJORfAjDIFq61NWvUquZ5xmKH-VcC4k/edit#heading=h.olbd4u4vk3h6 NOTE: a) Existing Font::GetStringWidth() is now obsolete and should be replaced by gfx::GetStringWidth() in ui/gfx/text_utils.h . b) 'const' has internal linkage so it doesn't require anonymous namespace (in ui/views/widget/tooltip_manager.cc). BUG=265485 TEST=Run unittests. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220509

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -22 lines) Patch
M chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M ui/views/widget/tooltip_manager.h View 2 chunks +2 lines, -2 lines 0 comments Download
M ui/views/widget/tooltip_manager.cc View 3 chunks +5 lines, -8 lines 0 comments Download
M ui/views/widget/tooltip_manager_aura.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M ui/views/widget/tooltip_manager_win.cc View 2 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Yuki
Could you guys review this CL as owners of ui/views/ and chrome/browser/ui/views/bookmarks/ ? Thanks in ...
7 years, 3 months ago (2013-08-29 12:32:40 UTC) #1
sky
LGTM
7 years, 3 months ago (2013-08-29 15:44:29 UTC) #2
sadrul
LGTM
7 years, 3 months ago (2013-08-29 16:54:59 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yukishiino@chromium.org/23483015/1
7 years, 3 months ago (2013-08-30 02:05:04 UTC) #4
commit-bot: I haz the power
7 years, 3 months ago (2013-08-30 05:36:52 UTC) #5
Message was sent while issue was closed.
Change committed as 220509

Powered by Google App Engine
This is Rietveld 408576698