Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(13)

Issue 18805: Correct sqlite wrapper behavior on systems where wchar_t is UTF-32, (Closed)

Created:
10 years, 6 months ago by Paweł Hajdan Jr.
Modified:
8 years, 2 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Correct sqlite wrapper behavior on systems where wchar_t is UTF-32, for example Linux. The problem was that old code assumed wstring is UTF-16, which resulted in string corruption on Linux. I actually tested it on browser/history unit tests, see http://codereview.chromium.org/18758.

Patch Set 1 #

Total comments: 6

Patch Set 2 : work in progress. Switched to utf-8, but breaks Windows. #

Patch Set 3 : sync with trunk #

Patch Set 4 : applied Brett's fix #

Total comments: 11

Patch Set 5 : minor fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -70 lines) Patch
M chrome/browser/history/download_database.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/history/starred_url_database.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/history/url_database.cc View 1 2 3 4 3 chunks +11 lines, -8 lines 0 comments Download
M chrome/browser/history/visitsegment_database.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/importer/firefox3_importer.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/meta_table_helper.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/webdata/web_database.cc View 1 2 4 chunks +11 lines, -11 lines 0 comments Download
M chrome/common/sqlite_utils.h View 1 6 chunks +11 lines, -16 lines 0 comments Download
M chrome/common/sqlite_utils.cc View 1 2 3 4 6 chunks +11 lines, -25 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Paweł Hajdan Jr.
10 years, 6 months ago (2009-01-26 17:57:17 UTC) #1
Evan Martin
This looks great to me, but please wait for Brett to sign off on it ...
10 years, 6 months ago (2009-01-26 18:24:11 UTC) #2
brettw
http://codereview.chromium.org/18805/diff/1/9 File chrome/common/sqlite_utils.cc (right): http://codereview.chromium.org/18805/diff/1/9#newcode199 Line 199: int SQLStatement::prepare16(sqlite3* db, const char16* sql, int sql_len) ...
10 years, 6 months ago (2009-01-26 19:00:34 UTC) #3
Paweł Hajdan Jr.
I removed unused functions, switched to UTF-8, and applied Brett's fix for crashing windows unit_tests.exe ...
10 years, 6 months ago (2009-01-30 10:13:18 UTC) #4
Dean McNamee
I have a few random comments, that aren't really part of your review, but I ...
10 years, 6 months ago (2009-01-30 10:48:35 UTC) #5
brettw
LGTM if dean is happy with my suggestions. http://codereview.chromium.org/18805/diff/605/423 File chrome/browser/history/url_database.cc (right): http://codereview.chromium.org/18805/diff/605/423#newcode244 Line 244: ...
10 years, 6 months ago (2009-01-30 16:01:31 UTC) #6
Paweł Hajdan Jr.
I think I fixed these issues. Evan, what do you think?
10 years, 6 months ago (2009-01-30 18:59:15 UTC) #7
Evan Martin
10 years, 6 months ago (2009-01-30 19:01:48 UTC) #8
LGTM

Powered by Google App Engine
This is Rietveld 408576698