Rewrite guts of ResourceLoader and BufferedResourceHandler to suck less.

Rewrite guts of ResourceLoader and BufferedResourceHandler to
suck less.

The key change is to the pause/resume behavior.  ResourceHandlers
are now expected to consume events they receive.  They can only
defer subsequent processing.  It used to be the case that pausing
during OnReadCompleted would lead to repeating the call to
OnReadCompleted when resumed.  This was quite problematic for
chained ResourceHandlers.

BufferedResourceHandler is now the ResourceController for its
downstream ResourceHandler.  This enables the ResourceLoader and
the BufferedResourceHandler to be more cleanly decoupled.  The
BufferedResourceHandler buffers network events, determines the
mime type of the response, selects the appropriate downstream
ResourceHandler, and then it replays the network events.  It can
now do this independently of the ResourceLoader state machine.

OnReadCompleted no longer has an in/out bytes_read parameter.
This was used before in a very complicated way by the
BufferedResourceHandler to effect the bytes_read parameter of
future calls to OnReadCompleted (see the now deleted
ResourceLoader::paused_bytes_read_ member variable).

ResourceLoader gets a lot simpler and loses a lot of state
variables.  It no longer needs to track things like pause_count_
or paused_read_bytes_.  is_paused_, called_on_response_started_
and has_started_reading_ are all subsumed by deferred_stage_.

BufferedResourceLoader is modified extensively to hopefully
separate out the states more logically.  It now advances through
buffering (withholds events from the downstream handler),
processing (detects mime type and selects downstream handler),
replaying (feeding downstream handler with buffered events) and
finally streaming (acts like a simple pass-through) states.

R=jam@chromium.org

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

[darin (slow to review), 2012-06-28 21:49:53]

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
File content/browser/renderer_host/async_resource_handler.cc (right):

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
content/browser/renderer_host/async_resource_handler.cc:316: if
(++pending_data_count_ >= kMaxPendingDataMessages) {
NOTE:  This change results from not repeating the OnReadCompleted call, which
led to WillSendData being called, upon Resume.  Now, instead of withholding the
data chunk that put us over the edge, we instead withhold future data chunks.

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
File content/browser/renderer_host/buffered_resource_handler.cc (right):

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
content/browser/renderer_host/buffered_resource_handler.cc:62: // Used to write
into an existing IOBuffer at a given offset.
NOTE: The previous code would allocate a new IOBuffer if the first chunk of data
received over the wire was not big enough to confidently sniff a mime type.  It
would then memcpy that IOBuffer to the tail of the existing IOBuffer
(read_buffer_).  Now, we just use DependentIOBuffer to point to the available
space at the end of read_buffer_.  We have separate checks to ensure that we
never write beyond the end of read_buffer_.

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
File content/browser/renderer_host/resource_handler.h (right):

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
content/browser/renderer_host/resource_handler.h:79: // If the handler returns
false, then the request is cancelled.  Otherwise,
NOTE: This change makes OnWillRead's return value work more like the return
value of other methods.  The old meaning of the return value wasn't that useful,
and I found at least one case where it wasn't being respected properly.

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
File content/browser/renderer_host/resource_loader.cc (right):

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
content/browser/renderer_host/resource_loader.cc:412: // XXX Perhaps we should
really be looking to see if the status is IO_PENDING?
Whoops, I need to change this to a TODO(darin).

http://codereview.chromium.org/10578055/diff/38006/content/browser/renderer_h...
content/browser/renderer_host/resource_loader.cc:467: void
ResourceLoader::StartReading(bool is_continuation) {
NOTE: The old code tried to avoid going back to the MessageLoop in some cases:
1) When reading the first chunk of data, and 2) when reading EOF.  This
is_continuation flag and the bytes_read <= 0 check down below are used to
preserve that behavior.  I'm not sure yet if it is the best behavior, so I just
decided to preserve it.

[jam, 2012-06-28 23:27:07]

lgtm

[commit-bot: I haz the power, 2012-06-28 23:57:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/darin@chromium.org/10578055/27012

[commit-bot: I haz the power, 2012-06-29 00:54:10]

Try job failure for 10578055-27012 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-06-29 03:33:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/darin@chromium.org/10578055/27012

[commit-bot: I haz the power, 2012-06-29 04:45:04]

Change committed as 144873