DevTools: Fixed suspected bug in port forwarding

DevTools: Fixed suspected bug in port forwarding

Port forwarding tunnel test revealed a bug in stream_listen_socket on Windows. The last packet was not delivered to DidRead, DidClose was called instead.

BUG=356617
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271985

[pfeldman, 2014-05-18 06:17:05]

https://codereview.chromium.org/285263003/diff/20001/chrome/browser/devtools/...
File chrome/browser/devtools/device/port_forwarding_controller.cc (right):

https://codereview.chromium.org/285263003/diff/20001/chrome/browser/devtools/...
chrome/browser/devtools/device/port_forwarding_controller.cc:143: pending_io_--;
To recap offline discussion: we don't have read timeout, which means we can wait
forever on pending reads. It seems like every solution that makes this object
live longer fixes the test problem.

[pfeldman, 2014-05-20 12:02:47]

lgtm.

@mmenke: you will say we should migrate from this stream_listen_socket and I
agree. So I filed https://code.google.com/p/chromium/issues/detail?id=375230 for
that.

[vkuzkokov, 2014-05-20 12:38:36]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-05-20 12:39:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/285263003/40001

[pfeldman, 2014-05-20 12:40:18]

The CQ bit was unchecked by pfeldman@chromium.org

[pfeldman, 2014-05-20 12:40:43]

We are missing OWNERS stamp here, to early to cq it.

[mmenke, 2014-05-20 16:17:18]

On 2014/05/20 12:40:43, pfeldman wrote:
> We are missing OWNERS stamp here, to early to cq it.

Could you please expand the description?

Looking at the net/ changes, I assume the issue is that we're getting the
FD_CLOSE and FD_READ for the final bytes at the same time, but I have no idea
what that has to do with port forwarding.

[pfeldman, 2014-05-20 19:15:11]

> Looking at the net/ changes, I assume the issue is that we're getting the
> FD_CLOSE and FD_READ for the final bytes at the same time, but I have no idea
> what that has to do with port forwarding.

it is just that the test for port forwarding was revealing the bug on Windows.
Updated the description.

[pfeldman, 2014-05-20 19:15:39]

And yes, your assessment of FD_CLOSE and FD_READ being there at the same time is
correct.

[pfeldman, 2014-05-21 10:01:46]

Gentle owners ping.

[mmenke, 2014-05-21 14:48:13]

On 2014/05/21 10:01:46, pfeldman wrote:
> Gentle owners ping.

Sorry, had been waiting for a comment from vkuzkokov that the description was
updated.

LGTM!

[pfeldman, 2014-05-21 15:05:44]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2014-05-21 19:56:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/285263003/40001

[commit-bot: I haz the power, 2014-05-21 22:24:42]

Change committed as 271985