Switch MessagePumpForIO to use completion ports on Windows....

Switch MessagePumpForIO to use completion ports on Windows.

Cleanup the separation between MessagePumpForUI and 
MessagePumpForIO, and convert the latter to use Completion
Ports instead of MsgWaitForMultipleobjects to sleep
when idle.

Remove all traces of Windows messages from MessagePumpForIO,
remove the transitional API of completion port notifications
and remove WatchObject API.

Modify all callers of RegisterIOHandler so that they are no
longer using RegisterIOContext, and also handle properly
the new semantics of completion ports (notifications even when
the IO completes immediately).

Add a new interface to allow proper cleanup of disk cache (to
replace code that was waiting for pending APCs from the destructor).

Add a way for the message pump to perform cleanup of abandoned IO.


BUG=B/1344358, 3497, 3630
TESt=unit tests
R=darin



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

[rvargas (doing something else), 2008-10-24 19:12:04]

Darin,

Given the large movement of code on the message pump,  I added a version of the
files that (hopefully) just moves the code around without changing anything (rev
1) so verifying movement vs actual change should be easier.

[rvargas (doing something else), 2008-10-31 01:05:10]

I have a new version, this time blocking on the destructors. Please take a look.

[darin (slow to review), 2008-10-31 04:20:44]

LGTM.  nice work!

http://codereview.chromium.org/8156/diff/231/235
File base/message_pump_win.cc (right):

http://codereview.chromium.org/8156/diff/231/235#newcode430
Line 430: // ProcessNextIOCompletion(), there is a good chance there are still
more
this comment needs to be updated.

http://codereview.chromium.org/8156/diff/231/245
File chrome/common/ipc_channel.cc (right):

http://codereview.chromium.org/8156/diff/231/245#newcode54
Line 54: Time start = Time::Now();
please add a note about why this may be an interesting metric to histogram.

http://codereview.chromium.org/8156/diff/231/237
File net/base/file_input_stream_win.cc (right):

http://codereview.chromium.org/8156/diff/231/237#newcode73
Line 73: Time start = Time::Now();
ditto

http://codereview.chromium.org/8156/diff/231/238
File net/disk_cache/file_win.cc (right):

http://codereview.chromium.org/8156/diff/231/238#newcode100
Line 100: // Now that we are not using APCs anymore we can not fake synchronous
requests
this comment may seem a bit confusing without the context of the code being
removed.

[rvargas (doing something else), 2008-11-06 02:13:56]

Could you take another look.

Changes after merging with the latest code:
1. Re-enable nested IO message loop support (now needed by ipc_sync_channel).
2. Limit the new histograms to when we actually wait.
3. Fix ipc_channel to avoid reentrancy or issuing more async IO after Close().

On 2008/10/31 04:20:44, darin wrote:
> LGTM.  nice work!
> 
> http://codereview.chromium.org/8156/diff/231/235
> File base/message_pump_win.cc (right):
> 
> http://codereview.chromium.org/8156/diff/231/235#newcode430
> Line 430: // ProcessNextIOCompletion(), there is a good chance there are still
> more
> this comment needs to be updated.
> 
> http://codereview.chromium.org/8156/diff/231/245
> File chrome/common/ipc_channel.cc (right):
> 
> http://codereview.chromium.org/8156/diff/231/245#newcode54
> Line 54: Time start = Time::Now();
> please add a note about why this may be an interesting metric to histogram.
> 
> http://codereview.chromium.org/8156/diff/231/237
> File net/base/file_input_stream_win.cc (right):
> 
> http://codereview.chromium.org/8156/diff/231/237#newcode73
> Line 73: Time start = Time::Now();
> ditto
> 
> http://codereview.chromium.org/8156/diff/231/238
> File net/disk_cache/file_win.cc (right):
> 
> http://codereview.chromium.org/8156/diff/231/238#newcode100
> Line 100: // Now that we are not using APCs anymore we can not fake
synchronous
> requests
> this comment may seem a bit confusing without the context of the code being
> removed.

[darin (slow to review), 2008-11-07 23:23:02]

OK, LGTM