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

Issue 119346: Introduce HttpStream and HttpBasicStream. (Closed)

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

Description

Introduce HttpStream and HttpBasicStream.
This is the beginning of the http pipelining work. Introduce HttpStream, an interface for reading and writing to http streams. Provide a basic implementation with HttpBasicStream. Switch HttpNetworkTransaction to reading/writing via HttpStream rather than directly to the socket.
Note that the interface will have to change later on. Read/Write() is the wrong interface, since a pipelining HttpStream implementation will have to detect the end of an http response, rather than having the client (HttpNetworkTransaction) do that. This is just the first step.
For information of the general roadmap for http pipelining, please refer to the bug.
BUG=http://crbug.com/8991
TEST=none

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

Patch Set 1 #

Patch Set 2 : Fix lint error. #

Patch Set 3 : Fix dumb errors. #

Total comments: 4

Patch Set 4 : Add comments. #

Total comments: 1

Patch Set 5 : sync #

Patch Set 6 : Missed these on sync. #

Patch Set 7 : Add comments to HttpBasicStream. Switch declaration order of Read/Write. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+113 lines, -5 lines) Lint Patch
A net/http/http_basic_stream.h View 1 2 3 4 6 1 chunk +44 lines, -0 lines 0 comments 1 errors Download
M net/http/http_network_transaction.h View 2 chunks +2 lines, -0 lines 0 comments 0 errors Download
M net/http/http_network_transaction.cc View 1 2 3 4 6 chunks +7 lines, -5 lines 0 comments 0 errors Download
A net/http/http_stream.h View 1 2 3 1 chunk +60 lines, -0 lines 0 comments 1 errors Download
Trybot results:
Commit:

Messages

Total messages: 4
willchan
4 years, 10 months ago #1
wtc
LGTM. Many of the things you said in the CL's description should be moved the ...
4 years, 10 months ago #2
willchan
http://codereview.chromium.org/119346/diff/10/1006 File net/http/http_basic_stream.h (right): http://codereview.chromium.org/119346/diff/10/1006#newcode8 Line 8: #include "base/basictypes.h" On 2009/06/09 18:58:22, wtc wrote: > ...
4 years, 10 months ago #3
eroman
4 years, 10 months ago #4
are their any design docs for the pipelining?

in particular i am wandering how cancellation of a transaction works when
sharing an httpstream.

otherwise LG.

http://codereview.chromium.org/119346/diff/16/1013
File net/http/http_basic_stream.h (right):

http://codereview.chromium.org/119346/diff/16/1013#newcode14
Line 14: class HttpBasicStream : public HttpStream {
a class comment would be good.
Sign in to reply to this message.

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