Closed Bug 99107 Opened 19 years ago Closed 19 years ago

nsViewSourceChannel needs to implement nsICachingChannel

Categories

(Core :: Networking, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(2 files)

To be able to load view source from cache, view source channels need to
implement nsICachingChannel.
Chak?  review?
Blocks: 55583
Keywords: patch, review
Are these changes going to fix bug 55583 or will there be something more to do?
[BTW: if yes, is nsICachingChannel something new?!]
There will be more.  Here's the full list for bug 55583:

1)  Check this in
2)  Check in interface changes to allow passing postdata and cachkeys to loadUri
    (or something equivalent)
3)  Check in JS changes to pass these in when doing viewsource

nsDocShell.cpp does the following at the moment:

open a channel;
if (channel can QI to nsIHttpChannel) {
  rewind postdata if we have it; // needed to properly repost if we repost
  if (channel can QI to nsICachingChannel && cache key passed in) {
    use cachekey to load document (prompting for repost if we have post data
    and can't load from cache);
  }}

So making nsIViewSourceChannel load from cache involves being able to get into
the middle part of that loop.
r=chak on the first patch 
Comment on attachment 48905 [details] [diff] [review]
proposed patch -- add nsIHttpChannel and nsICachingChannel so the docshell code will do the right things

looks like it would be nice if nsIHttpChannel and nsIViewSourceChannel 
didn't inherit from nsIChannel (sigh)

also, are you sure it is correct to have the view source channel appear to 
implement nsIHttpChannel and nsICachingChannel when mHttpChannel and/or 
mCachingChannel are NULL?
Well... I've checked all the callers and they do error-checking properly.  But
on third thought that does make me a little queasy.  Another approach coming up.
ccing darin for his comments
yeah, that seems better... i wonder if it is "OK" to hard code knowledge of the
inner workings of the QI macros.  i think it is worth it, considering the
benefit of not having to completely write your own QI impl, but perhaps you
should seek out a second opinion.
I talked with Shaver on IRC before posting that patch.  He was OK with it.  In
fact he vastly preferred it to the first attempt.
well then, r/sr=darin
checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.