24711 - HTTP needs to arbitrarily retrieve data

Status: CLOSED FIXED

(Core :: Networking, defect, P1)

Description

[Judson Valeski]

This bug is being created in lieu of 24250.

HTTP needs to ensure that *identical* stream listener callbacks are fired to the 
stream listener specified in it's orignal nsHTTPChannel::AsyncRead() method.

Currently, if there's a cache entry, the end consumer will receive two 
OnStopRequest() callbacks because ResponseCompleted() is still called after the 
304 response is handled and a cache entry is created and used.

Perhaps HTTP should generalize it's stream listener layer to accommodate 
receiving On*() callbacks regardless of transport type (cache or socket), or an 
additional listener should be implemented to handle the cache callbacks.

The bug basically amounts to the consumer re-targeting no working.

Comment 1

[Doug Turner (:dougt)]

I think that this problem causes SMP machines to crash.  The stack crawl that I
am setting on Dawn machine sounds alot like this bug

see 21556

Comment 2

[Gagan]

I am hoping to clean this a little bit. And get rpotts blessing on the same...

Comment 3

[leger]

PDT+

Comment 4

[Gagan]

cc'ing rick now that he is back and wants something to do :)

Comment 5

[Gagan]

->rick

Comment 6

[rpotts (gone)]

I've checked in a partial fix for this problem.  If a HTTP channel was deferred 
because no socket transports were available, then when it resumed it would stall 
if it's data had already been loaded into the cache...

An example of this problem was seen on http://www.abcnews.com

I am almost done with the fixes for the other part of this bug - where the 
CacheChannel was being passed out as the protocol channel (instead of the HTTP 
channel...)

Comment 7

[leger]

Need ETA date on final fix in Status Whiteboard please.

Comment 8

[rpotts (gone)]

I've finally checked in the fixes...

Comment 9

[Scott MacGregor]

Ack! Shame on you Rick. You didn't test mailnews. This fix broke displaying imap
messages when we store the messages in the memory cache because the cache
channel no longer implements GetContentType!

Comment 10

[Tom Everingham]

Discussed this with Rick.  Primarily architecture changes that are not 
verifiable.  SMP crash is a different bug and www.abcnew.com part of the bug 
works.  

Comment 11

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/8e41612eba0e
[fenix] For https://github.com/mozilla-mobile/fenix/issues/24711: Remove wrapper from recent tabs metrics.