Chromium Code Reviews
Help | Chromium Project | Sign in
(259)

Issue 99333: A utility driver for doing client/server HTTP transaction... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 11 months ago by Mike Belshe
Modified:
2 years, 9 months ago
Reviewers:
wtc
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

A utility driver for doing client/server HTTP transaction
tests.

This is an initial codebase - there is a lot of work to do.
But I wanted to get an initial version checked in.

http://crbug.com/6754

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

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Total comments: 22

Patch Set 6 : '' #

Patch Set 7 : '' #

Total comments: 7

Patch Set 8 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+732 lines, -0 lines) Lint Patch
M net/net.gyp View 1 2 3 4 5 6 1 chunk +34 lines, -0 lines 0 comments 0 errors Download
A net/tools/fetch/fetch_client.cc View 1 2 3 4 5 6 7 1 chunk +185 lines, -0 lines 0 comments 5 errors Download
A net/tools/fetch/fetch_server.cc View 1 2 3 4 5 1 chunk +54 lines, -0 lines 0 comments 3 errors Download
A net/tools/fetch/http_listen_socket.h View 1 2 3 4 5 6 7 1 chunk +55 lines, -0 lines 0 comments 8 errors Download
A net/tools/fetch/http_listen_socket.cc View 1 2 3 4 5 6 7 1 chunk +241 lines, -0 lines 0 comments 9 errors Download
A net/tools/fetch/http_server.h View 1 2 3 4 5 1 chunk +25 lines, -0 lines 0 comments 4 errors Download
A net/tools/fetch/http_server.cc View 1 2 3 4 5 6 7 1 chunk +13 lines, -0 lines 0 comments 1 errors Download
A net/tools/fetch/http_server_request_info.h View 1 2 3 4 5 1 chunk +24 lines, -0 lines 0 comments 2 errors Download
A net/tools/fetch/http_server_response_info.h View 1 2 1 chunk +40 lines, -0 lines 0 comments 3 errors Download
A net/tools/fetch/http_session.h View 1 2 3 4 5 6 7 1 chunk +27 lines, -0 lines 0 comments 5 errors Download
A net/tools/fetch/http_session.cc View 1 2 3 4 5 6 7 1 chunk +34 lines, -0 lines 0 comments 4 errors Download
Commit:

Messages

Total messages: 4
Mike Belshe
Please take a look; it doesn't compile on Mac&Linux yet, but I'm working on that. ...
4 years, 11 months ago #1
wtc
Mike, Thanks a lot for working on this. My comments below are mostly style nits. ...
4 years, 11 months ago #2
Mike Belshe
Should be ready for another look. Thanks!
4 years, 11 months ago #3
wtc
4 years, 11 months ago #4
LGTM.  Please fix the function declaration and
function call formatting issues before you check this in.
I don't need to review it again.

http://codereview.chromium.org/99333/diff/101/1100
File net/net.gyp (right):

http://codereview.chromium.org/99333/diff/101/1100#newcode624
Line 624: 'target_name': 'fetch_client',
So you decided to build fetch_client and fetch_server on
Windows only first?  I agree this is the right move.

Just curious: where did the msvs_guid values come from?

http://codereview.chromium.org/99333/diff/101/1096
File net/tools/fetch/fetch_client.cc (right):

http://codereview.chromium.org/99333/diff/101/1096#newcode21
Line 21: printf("usage: %s --url=<url>  [--n=<clients>] [--stats]
[--use_cache]\n",
Nit: the function calls should be formatted following the
guidelines at
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Calls

In this case the 'program_name' argument should be aligned
with the '(' on this line.

http://codereview.chromium.org/99333/diff/101/1096#newcode122
Line 122: &client_limit);
Nit: function call formatting (see above).

http://codereview.chromium.org/99333/diff/101/1097
File net/tools/fetch/http_listen_socket.h (right):

http://codereview.chromium.org/99333/diff/101/1097#newcode20
Line 20: virtual void OnRequest(HttpListenSocket* connection,
Nit: fix function declaration formatting in this file:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Decla...

http://codereview.chromium.org/99333/diff/101/1097#newcode25
Line 25: HttpListenSocket::Delegate* del);
Nit: change 'del' to 'delegate' in this file.

http://codereview.chromium.org/99333/diff/101/1095
File net/tools/fetch/http_server.cc (right):

http://codereview.chromium.org/99333/diff/101/1095#newcode10
Line 10: session_(new HttpSession(ip, port))) {
Nit: function call (ALLOW_THIS_IN_INITIALIZER_LIST)
formatting:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Calls

I suggest:

    : ALLOW_THIS_IN_INITIALIZER_LIST(
          session_(new HttpSession(ip, port))) {

http://codereview.chromium.org/99333/diff/101/1093
File net/tools/fetch/http_session.h (right):

http://codereview.chromium.org/99333/diff/101/1093#newcode19
Line 19: HttpServerRequestInfo* info);
Nit: function declaration formatting
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6