Created attachment 339637 [details] [diff] [review] patch v1 Currently, streamMessage will stream from the network even if we're offline or we don't want to. This isn't always desirable. The patch aims to fix this. The patch basically does two things -- - it allows streaming over the network in case This is a fairly straightforward patch, with some code shuffled around and a bit of the control flow changed. There is only one cache lookup now. David -- is this worth getting in for beta 1? Also, right now I've tried to keep as similar a behaviour as possible for the search component. Jeff mentioned that there should be a UI option whether to allow the component to download over the network. What do you think of such a thing post beta 1? (There are some changes that are just whitespace, diff -w didn't figure them out though)
Thx for doing this! In general it looks good. I know this is code that just moved, but you only need to check NS_SUCCEEDED or aStreamListener here., not both: + nsCOMPtr<nsIStreamListener> aStreamListener = do_QueryInterface(aDisplayConsumer, &rv); + if (NS_SUCCEEDED(rv) && aStreamListener) Am I right that with this patch, no one is passing in PR_TRUE for aLocalOnly? If not, I don't see a good reason to get this in for b1, other than getting some testing, or allowing extensions to try it...
(In reply to comment #1) > Am I right that with this patch, no one is passing in PR_TRUE for aLocalOnly? > If not, I don't see a good reason to get this in for b1, other than getting > some testing, or allowing extensions to try it... The only thing I'm concerned about is that streamMessage in its current state might attempt to generate network traffic even if we're offline. Otherwise, the patch doesn't really provide any critically useful features for extensions, either. So do you think this should land post b1? (Sorry about the half-finished thought there -- the full thought is: The patch basically does two things -- - it allows streaming over the network in case we aren't offline and aLocalOnly is false - it doesn't allow it otherwise, and returns immediately with a failure instead of much later)
Comment on attachment 339637 [details] [diff] [review] patch v1 thx for doing this. One other nit: I think we should remove the "now is where" part of this comment, since we don't have context anymore. + // now is where our behavior differs....if the consumer is the docshell then we want to + // run the url in the webshell in order to display it. If it isn't a docshell then just I think I'd like this to land after b1, since no one will be taking advantage of the new functionality until after b1
Created attachment 341932 [details] [diff] [review] patch for checkin carrying forward r+sr. I also fixed an nsresult to read NS_IMETHODIMP.
Checked in, changeset id 529:70420d2ee44c