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

Issue 39208: POSIX: Rewrite IPC's interaction with FileDescriptor (Closed)

Created:
11 years, 9 months ago by agl
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

POSIX: Rewrite IPC's interaction with FileDescriptor The FileDescriptor API is clearly too hard to use. It's the only IPC data type which is non-POD and serialising an invalid file descriptor is fatal to Chrome on POSIX. The use of Maybe is possibly non-obvious to non-functional programmers. This patch merges Maybe and FileDescriptor so that serialising invalid file descriptors is permitted and results in -1 at the other end. (Serialising /closed/ a file descriptor is still fatal.) Also, it adds a pointer in base/file_descriptor.h to instructions for its use with IPC. Although it's generally bad practice to mention IPC in base, in this case I cannot find another suitable location.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -50 lines) Patch
M base/file_descriptor_posix.h View 1 chunk +4 lines, -0 lines 1 comment Download
M chrome/browser/renderer_host/render_widget_helper.h View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/renderer_host/render_widget_helper.cc View 1 chunk +5 lines, -5 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.h View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/ipc_message_utils.h View 4 chunks +33 lines, -33 lines 1 comment Download
M chrome/common/render_messages_internal.h View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/renderer/render_process.cc View 1 chunk +4 lines, -4 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
agl
darin: review hclam: cc only
11 years, 9 months ago (2009-03-05 19:26:28 UTC) #1
darin (slow to review)
seems like nice cleanup to me. LGTM http://codereview.chromium.org/39208/diff/1/8 File chrome/common/ipc_message_utils.h (right): http://codereview.chromium.org/39208/diff/1/8#newcode730 Line 730: } ...
11 years, 9 months ago (2009-03-05 19:42:12 UTC) #2
Alpha Left Google
11 years, 9 months ago (2009-03-05 20:39:38 UTC) #3
http://codereview.chromium.org/39208/diff/1/2
File base/file_descriptor_posix.h (right):

http://codereview.chromium.org/39208/diff/1/2#newcode27
Line 27: int fd;
Should this be type base::PlatformFile?

Powered by Google App Engine
This is Rietveld 408576698