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

Issue 261743002: Improve process output watcher's handling of multi-byte UTF8 characters (Closed)

Created:
6 years, 7 months ago by tbarzic
Modified:
6 years, 7 months ago
Reviewers:
rginda
CC:
chromium-reviews, stevenjb+watch_chromium.org, oshima+watch_chromium.org
Visibility:
Public.

Description

Improve process output watcher's handling of multi-byte UTF8 characters Speciffically, handle case when a non ASCII UTF8 character is read partially from the process output. Instead of reporting read character bytes imediatelly, cache them until the rest of the character is read from the process output. BUG=278340 TEST=in crosh (see comment #1 in the bug): $python >>> print(u"\u20ac" * 10000) Verify a series of EURO signs is displayed (without "unknown" characters) Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270799

Patch Set 1 #

Patch Set 2 : some nits #

Patch Set 3 : . #

Patch Set 4 : . #

Patch Set 5 : . #

Patch Set 6 : . #

Patch Set 7 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+283 lines, -60 lines) Patch
M chromeos/process_proxy/process_output_watcher.h View 1 2 3 4 5 3 chunks +16 lines, -8 lines 0 comments Download
M chromeos/process_proxy/process_output_watcher.cc View 1 2 3 4 5 6 5 chunks +91 lines, -13 lines 0 comments Download
M chromeos/process_proxy/process_output_watcher_unittest.cc View 1 2 3 8 chunks +176 lines, -37 lines 0 comments Download
M chromeos/process_proxy/process_proxy_unittest.cc View 1 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
tbarzic
can you please take a look?
6 years, 7 months ago (2014-05-01 16:00:23 UTC) #1
tbarzic
On 2014/05/01 16:00:23, tbarzic wrote: > can you please take a look? ping
6 years, 7 months ago (2014-05-06 21:38:19 UTC) #2
rginda
lgtm Sorry for the delay, I missed the mail about this one.
6 years, 7 months ago (2014-05-14 20:34:33 UTC) #3
tbarzic
On 2014/05/14 20:34:33, rginda wrote: > lgtm > > Sorry for the delay, I missed ...
6 years, 7 months ago (2014-05-14 20:36:48 UTC) #4
tbarzic
The CQ bit was checked by tbarzic@chromium.org
6 years, 7 months ago (2014-05-14 20:41:42 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tbarzic@chromium.org/261743002/100001
6 years, 7 months ago (2014-05-14 20:42:06 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-15 00:18:37 UTC) #7
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-15 01:43:00 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chromeos_clang_dbg/builds/16756)
6 years, 7 months ago (2014-05-15 01:43:00 UTC) #9
tbarzic
The CQ bit was checked by tbarzic@chromium.org
6 years, 7 months ago (2014-05-15 17:45:23 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tbarzic@chromium.org/261743002/120001
6 years, 7 months ago (2014-05-15 17:47:26 UTC) #11
commit-bot: I haz the power
6 years, 7 months ago (2014-05-15 20:34:40 UTC) #12
Message was sent while issue was closed.
Change committed as 270799

Powered by Google App Engine
This is Rietveld 408576698