Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(643)

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

Created:
11 years, 7 months ago by Mike Belshe
Modified:
9 years, 5 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) Patch
M net/net.gyp View 1 2 3 4 5 6 1 chunk +34 lines, -0 lines 0 comments Download
A net/tools/fetch/fetch_client.cc View 1 2 3 4 5 6 7 1 chunk +185 lines, -0 lines 0 comments Download
A net/tools/fetch/fetch_server.cc View 1 2 3 4 5 1 chunk +54 lines, -0 lines 0 comments 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 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 Download
A net/tools/fetch/http_server.h View 1 2 3 4 5 1 chunk +25 lines, -0 lines 0 comments Download
A net/tools/fetch/http_server.cc View 1 2 3 4 5 6 7 1 chunk +13 lines, -0 lines 0 comments Download
A net/tools/fetch/http_server_request_info.h View 1 2 3 4 5 1 chunk +24 lines, -0 lines 0 comments Download
A net/tools/fetch/http_server_response_info.h View 1 2 1 chunk +40 lines, -0 lines 0 comments Download
A net/tools/fetch/http_session.h View 1 2 3 4 5 6 7 1 chunk +27 lines, -0 lines 0 comments Download
A net/tools/fetch/http_session.cc View 1 2 3 4 5 6 7 1 chunk +34 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Mike Belshe
Please take a look; it doesn't compile on Mac&Linux yet, but I'm working on that. ...
11 years, 7 months ago (2009-05-04 00:13:00 UTC) #1
wtc
Mike, Thanks a lot for working on this. My comments below are mostly style nits. ...
11 years, 7 months ago (2009-05-04 19:22:38 UTC) #2
Mike Belshe
Should be ready for another look. Thanks!
11 years, 7 months ago (2009-05-04 23:50:57 UTC) #3
wtc
11 years, 7 months ago (2009-05-05 00:46:37 UTC) #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

Powered by Google App Engine
This is Rietveld 408576698