Steve, would you be willing to find the day that this broke using http://harthur.github.com/mozregression/ ? I can totally understand if you don't want to commit to that, by the way. Just let me know.
I was bored, so I fiddled with the mozregression tool. It looks like this broke in the nightlies between 2012-03-23 and 2012-03-24... Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ab2ff3b5611f&tochange=df1f94b2bdee
Thanks! for Inbound this is: Last good nightly: 2012-03-22 First bad nightly: 2012-03-23 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dd6e82e5edac&tochange=d2693e86769d
HG Bisect tells: The first bad revision is: changeset: 89771:5b61e3d75735 user: Michal Novotny <firstname.lastname@example.org> date: Thu Mar 22 23:53:55 2012 +0100 summary: Bug 722033 - Use asyncOpenCacheEntry() in nsHttpChannel::OpenOfflineCacheEntryForWriting() (Had to apply add2ed43cc05 on each Run due to Build Failures)
cc'ing Michal on this - are we looking at a forward fix here or a backout of bug 722033 to make sure we don't ship 14 with this regression?
Created attachment 638761 [details] [diff] [review] fix We call ContinueConnect() in OnCacheEntryAvailableInternal() even if the channel was canceled. This patch is a quick fix for this issue. I still need to check all other failures in OnNormalCacheEntryAvailable(), OnOfflineCacheEntryAvailable() and OnOfflineCacheEntryForWritingAvailable() whether we correctly cancel the request or continue without the entry...
Comment on attachment 638761 [details] [diff] [review] fix Review of attachment 638761 [details] [diff] [review]: ----------------------------------------------------------------- r=honzab checked this aborts the channel and calls OnStart/OnStopRequest just ones for this case ::: netwerk/protocol/http/nsHttpChannel.cpp @@ +5522,5 @@ > } > return NS_ERROR_DOCUMENT_NOT_CACHED; > } > + if (mCanceled) > + return rv; Definitely needs a comment what this causes and why it is here.
Verified using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120705030540. Thanks for the quickly Fix!
Comment on attachment 638761 [details] [diff] [review] fix [Approval Request Comment] Bug caused by (feature/regressing bug #): 722033 User impact if declined: some resources would be fetched from the net multiple times Testing completed (on m-c, etc.): https://tbpl.mozilla.org/?tree=Try&rev=dad6ce8b9a25 Risk to taking this patch (and alternatives if risky): probably none String or UUID changes made by this patch: none
Comment on attachment 638761 [details] [diff] [review] fix [Triage Comment] Approving for all branches in case the http://www.crazy-factory.com/ user experience is a canary in the coal mine for other web regressions caused by bug 722033.
(In reply to Michal Novotny (:michal) from comment #13) > https://hg.mozilla.org/releases/mozilla-aurora/rev/ebea5096017e > https://hg.mozilla.org/releases/mozilla-beta/rev/cd89f913b87b Thanks Michal!