Refactoring on Windows IPC channel.

Refactoring on Windows IPC channel.

This makes the Windows IPC channel interface the same as I've created for the Posix one:
http://codereview.chromium.org/9570001/

The core reading functions DispatchInputData, ProcessIncomingMessages, and the "hello" message handling are now identical. The follow-up step will be to merge these into a common IPCChannelReader base class and eliminate the duplicate code.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124848

[brettw, 2012-03-01 21:24:16]

The header file could use a little more documentation, but I want to write it in
the new base class (which wouldn't make a lot of sense here now).

https://chromiumcodereview.appspot.com/9568031/diff/1/ipc/ipc_channel_win.cc
File ipc/ipc_channel_win.cc (left):

https://chromiumcodereview.appspot.com/9568031/diff/1/ipc/ipc_channel_win.cc#...
ipc/ipc_channel_win.cc:264: if (input_state_.is_pending) {
This check moved to OnIOCompleted.

https://chromiumcodereview.appspot.com/9568031/diff/1/ipc/ipc_channel_win.cc#...
ipc/ipc_channel_win.cc:276: if (bytes_read == 0) {
This code moved to ReadData

https://chromiumcodereview.appspot.com/9568031/diff/1/ipc/ipc_channel_win.cc#...
ipc/ipc_channel_win.cc:302: const char* p, *end;
The rest of this loop moved to DispatchInputData

https://chromiumcodereview.appspot.com/9568031/diff/1/ipc/ipc_channel_win.cc
File ipc/ipc_channel_win.cc (right):

https://chromiumcodereview.appspot.com/9568031/diff/1/ipc/ipc_channel_win.cc#...
ipc/ipc_channel_win.cc:463: ok = AsyncReadComplete(bytes_transfered);
ProcessIncomingMessages doesn't need any parameters now that processing async
data is a two-step process.

[rvargas (doing something else), 2012-03-02 02:22:08]

LGTM

http://codereview.chromium.org/9568031/diff/1/ipc/ipc_channel_win.cc
File ipc/ipc_channel_win.cc (right):

http://codereview.chromium.org/9568031/diff/1/ipc/ipc_channel_win.cc#newcode129
ipc/ipc_channel_win.cc:129: // structured, but instead we go back to the message
loop. Our completion
nit: this comment needs some rewording.

http://codereview.chromium.org/9568031/diff/1/ipc/ipc_channel_win.cc#newcode459
ipc/ipc_channel_win.cc:459: input_state_.is_pending = false;
It may be better to move this block to AsyncReadComplete.

Maybe even the whole if/else... even if the else is not part of a Read... but it
can be an early return there, and it's only slightly out of place.

[brettw, 2012-03-02 05:31:18]

http://codereview.chromium.org/9568031/diff/1/ipc/ipc_channel_win.cc
File ipc/ipc_channel_win.cc (right):

http://codereview.chromium.org/9568031/diff/1/ipc/ipc_channel_win.cc#newcode459
ipc/ipc_channel_win.cc:459: input_state_.is_pending = false;
I was going to make AsyncReadComplete cross-platform, and since this is
platform-specific, it needs to go here.

[rvargas (doing something else), 2012-03-02 18:44:16]

http://codereview.chromium.org/9568031/diff/1/ipc/ipc_channel_win.cc
File ipc/ipc_channel_win.cc (right):

http://codereview.chromium.org/9568031/diff/1/ipc/ipc_channel_win.cc#newcode459
ipc/ipc_channel_win.cc:459: input_state_.is_pending = false;
On 2012/03/02 05:31:18, brettw wrote:
> I was going to make AsyncReadComplete cross-platform, and since this is
> platform-specific, it needs to go here.

OK.

Does that mean that AsyncReadComplete is going to be part of the interface that
this class implements? The description on the header was not clear about how one
would use this method, but I assumed that it was going to be private to the
windows version (so it didn't matter that much). I guess when the rest of the
code appear it would be clear how to use it :)

[commit-bot: I haz the power, 2012-03-03 04:41:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/brettw@chromium.org/9568031/6001

[commit-bot: I haz the power, 2012-03-03 08:55:46]

Change committed as 124848