Issue 8392017: Change string16 to std::string in the gfx::Font() interface.

Issue 8392017: Change string16 to std::string in the gfx::Font() interface. (Closed)

Created:
9 years, 2 months ago by Elliot Glaysher

Modified:
9 years, 1 month ago

Reviewers:
Avi (use Gerrit), Ben Goodger (Google)

CC:
chromium-reviews, tfarina, dhollowa, Paweł Hajdan Jr.

Base URL:
http://git.chromium.org/chromium/src.git@master

Visibility:
Public.

Description

Change string16 to std::string in the gfx::Font() interface. Most consumers on all platforms (including windows!) were converting the font family/GetFontName() result to UTF8. (Windows was still having to convert for the SkTypeface calls.) On Linux, we're performing a UTF16ToUTF8 on each string print (and this is showing up in pprof output.) BUG=100803 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107659

Patch Set 1 #

Patch Set 2 : Fix windows compile #

Patch Set 3 : Fix mac, which already does a conversion. #

Patch Set 4 : Fix bad rebase #

Patch Set 5 : Fix ui_unittests #

Patch Set 6 : Fix mac unit tests #

Total comments: 1

Patch Set 7 : Add 'UTF-8' everywhere in the comments #

Created: 9 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+82 lines, -82 lines)			Patch
M	chrome/browser/printing/cloud_print/cloud_print_setup_flow.cc	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/cocoa/download/download_item_cell.mm	View	1 2 3 4 5	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac_unittest.mm	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/cocoa/status_bubble_mac.mm	View	1 2 3 4 5	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/common/badge_util.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	printing/print_settings_initializer.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	ui/gfx/font.h	View	1 2 3 4 5 6	2 chunks	+4 lines, -4 lines	0 comments	Download
M	ui/gfx/font.cc	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	ui/gfx/font_unittest.cc	View	1 2 3 4	5 chunks	+10 lines, -10 lines	0 comments	Download
M	ui/gfx/platform_font.h	View	1 2 3 4 5 6	2 chunks	+5 lines, -5 lines	0 comments	Download
M	ui/gfx/platform_font_mac.h	View		2 chunks	+5 lines, -5 lines	0 comments	Download
M	ui/gfx/platform_font_mac.mm	View	1 2	5 chunks	+7 lines, -7 lines	0 comments	Download
M	ui/gfx/platform_font_pango.h	View		4 chunks	+8 lines, -8 lines	0 comments	Download
M	ui/gfx/platform_font_pango.cc	View		11 chunks	+15 lines, -15 lines	0 comments	Download
M	ui/gfx/platform_font_win.h	View	1	5 chunks	+5 lines, -5 lines	0 comments	Download
M	ui/gfx/platform_font_win.cc	View	1	5 chunks	+6 lines, -6 lines	0 comments	Download
M	ui/gfx/render_text_win.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	views/controls/label_unittest.cc	View		2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Elliot Glaysher

ben: blame says you were the last person to really modify gfx::Font. avi: Most fixups ...

9 years, 1 month ago (2011-10-27 20:44:53 UTC) #1

Avi (use Gerrit)

lgtm http://codereview.chromium.org/8392017/diff/6002/ui/gfx/font.h File ui/gfx/font.h (right): http://codereview.chromium.org/8392017/diff/6002/ui/gfx/font.h#newcode46 ui/gfx/font.h:46: // Creates a font with the specified name ...

9 years, 1 month ago (2011-10-27 20:54:16 UTC) #2

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erg@chromium.org/8392017/9022

9 years, 1 month ago (2011-10-27 21:51:22 UTC) #4

commit-bot: I haz the power

Try job failure for 8392017-9022 (retry) on linux_rel for step "ui_tests". It's a second try, ...

9 years, 1 month ago (2011-10-27 22:39:50 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erg@chromium.org/8392017/9022

9 years, 1 month ago (2011-10-27 22:46:48 UTC) #6

Change committed as 107659

Expand Messages | Collapse Messages