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

Issue 15732022: Move latin1_string_conversions to base (Closed)

Created:
7 years, 6 months ago by abarth-chromium
Modified:
7 years, 6 months ago
CC:
chromium-reviews, erikwright+watch_chromium.org, darin-cc_chromium.org, jshin+watch_chromium.org, brettw, jamesr
Visibility:
Public.

Description

Move latin1_string_conversions to base Unfortunately, we can't keep this function in webkit/glue because not everyone who links in Blink depends on webkit/glue. For example, cc uses WebString but doesn't depend on webkit/glue. IMHO, the best solution to all these constraints is to put this function in base. It's concerned with concepts that make sense in base (strings and character sets). The only thing odd about the function is that it accepts Latin-1 or UTF-16, which is because of the underlying string representation in Blink. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203003

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -67 lines) Patch
M base/base.gypi View 1 chunk +3 lines, -1 line 0 comments Download
A + base/strings/latin1_string_conversions.h View 2 chunks +8 lines, -9 lines 0 comments Download
A + base/strings/latin1_string_conversions.cc View 2 chunks +3 lines, -3 lines 0 comments Download
D webkit/glue/latin1_string_conversions.h View 1 chunk +0 lines, -33 lines 0 comments Download
D webkit/glue/latin1_string_conversions.cc View 1 chunk +0 lines, -19 lines 0 comments Download
M webkit/glue/webkit_glue.gypi View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
abarth-chromium
@darin: Would you be willing to review this CL?
7 years, 6 months ago (2013-05-29 01:35:03 UTC) #1
abarth-chromium
@brettw: Would you be willing to LGTM this CL? You said "ok" the idea in ...
7 years, 6 months ago (2013-05-29 18:32:22 UTC) #2
darin (slow to review)
LGTM
7 years, 6 months ago (2013-05-29 19:11:15 UTC) #3
abarth-chromium
Thanks!
7 years, 6 months ago (2013-05-29 20:24:55 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/abarth@chromium.org/15732022/1
7 years, 6 months ago (2013-05-29 20:25:14 UTC) #5
commit-bot: I haz the power
7 years, 6 months ago (2013-05-29 23:21:33 UTC) #6
Message was sent while issue was closed.
Change committed as 203003

Powered by Google App Engine
This is Rietveld 408576698