Revert "Revert of Replace StreamListenSocket with StreamSocket in HttpServer. (patchset #29 of http…

Revert "Revert of Replace StreamListenSocket with StreamSocket in HttpServer. (patchset #29 of https://codereview.chromium.org/296053012/)"

This reverts commit 0b2f33f4a88efbd203b0623324ad4114e3bb9d23.

This is relanding CL of https://codereview.chromium.org/296053012/, which broke http server unittests because http server doesn't send response synchronously any more.
This CL fixes unittests by reading responses completely.

Patch set #1 is same to the original CL.
Patch set #2 is the diff.

BUG=371906
TBR=pfeldman@chromium.org,darin@chromium.org,gunsch@chromium.org,mnaganov@chromium.org

Committed: https://crrev.com/38c3ae72c9743dbe172779477917bf24bc25ab97
Cr-Commit-Position: refs/heads/master@{#291784}

[byungchul, 2014-08-22 23:18:49]

Fixed http server unittests.

[mmenke, 2014-08-25 18:19:54]

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
File net/server/http_server_unittest.cc (right):

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:115: return false;
I don't think this is needed.

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:119: return false;
optional:  May be simpler to read one byte at a time until we have the headers,
and then to call Read with the content-length.  Or not, haven't really thought
about it.

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:156: bool IsCompleteResponse(const
std::string response) {
nit:  std::string&

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:173: }
I don't think this is needed?  We only read in one byte at a time.  Suggest just
a DCHECK_GT instead, with a comment that only one byte of the headers is read at
a time by ReadResponse.

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:180: DCHECK_GT(end_of_http_headers,
content_length_value_pos);
scoped_refptr<HttpResponseHeaders> headers(
    new HttpResponseHeaders(HttpUtil::AssembleRawHeaders(
        response.c_str(), content_length_header_pos));
int64 content_length = headers->GetContentLength();

[byungchul, 2014-08-25 21:11:26]

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
File net/server/http_server_unittest.cc (right):

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:115: return false;
On 2014/08/25 18:19:54, mmenke wrote:
> I don't think this is needed.

I wanted to read data directly in message first. If you mind strongly, will
remove it.

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:119: return false;
On 2014/08/25 18:19:54, mmenke wrote:
> optional:  May be simpler to read one byte at a time until we have the
headers,
> and then to call Read with the content-length.  Or not, haven't really thought
> about it.

Then, we need another Read() to read only the exact number of expected bytes
which is somewhat duplicated. I think it would be better to read as much as it
can, and make unexpected extra data cause failure.

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:156: bool IsCompleteResponse(const
std::string response) {
On 2014/08/25 18:19:54, mmenke wrote:
> nit:  std::string&

Done.

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:173: }
On 2014/08/25 18:19:54, mmenke wrote:
> I don't think this is needed?  We only read in one byte at a time.  Suggest
just
> a DCHECK_GT instead, with a comment that only one byte of the headers is read
at
> a time by ReadResponse.

Not necessary any more.

https://codereview.chromium.org/487013003/diff/20001/net/server/http_server_u...
net/server/http_server_unittest.cc:180: DCHECK_GT(end_of_http_headers,
content_length_value_pos);
On 2014/08/25 18:19:54, mmenke wrote:
> scoped_refptr<HttpResponseHeaders> headers(
>     new HttpResponseHeaders(HttpUtil::AssembleRawHeaders(
>         response.c_str(), content_length_header_pos));
> int64 content_length = headers->GetContentLength();

Done.

[mmenke, 2014-08-25 21:28:09]

Much simpler.  LGTM!

[byungchul, 2014-08-25 21:33:48]

The CQ bit was checked by byungchul@chromium.org

[commit-bot: I haz the power, 2014-08-25 21:35:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/byungchul@chromium.org/487013003/60001

[commit-bot: I haz the power, 2014-08-25 22:42:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-25 23:27:58]

Committed patchset #4 (60001) as a0413b18912bdf24e548c68bbc2d5aac3551e709

[commit-bot: I haz the power, 2014-09-10 02:38:25]

Patchset 4 (id:??) landed as
https://crrev.com/38c3ae72c9743dbe172779477917bf24bc25ab97
Cr-Commit-Position: refs/heads/master@{#291784}