244855 - Impossible to find out which browser instance created a nsIHttpChannel

Status: NEW

(Core :: DOM: Navigation, defect)

Description

[Simon Detheridge]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040517
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040517

A bit fuzzy about whether this is an embedding bug, or networking bug. Sorry.

When writing an embedded application that contains multiple instances of a
nsIWebBrowser, and adding an observer that does something with http channels, I
found it impossible to know which instance of the browser created the instance
of the http channel you are interested in.

This makes it difficult to create a program that has multiple nsIWebBrowser
instances that behave differently in this regard.

I tried interrogating the nsIChannel::owner, but this seemed to be set to null
whenever I needed it. Also, the nsILoadGroup couldn't tell me what I needed to know.

The URI stored in nsIHttpChannelInternal would have been useful for this, except
that I want the instances of the browser to be able to behave differently, even
if they are both looking at the same web page.

The only thing I could come up with, was a dirty hack to store a pointer to the
relevant nsIWebBrowser in the nsIHttpChannelInternal, when it's created by the
docshell. (This doesn't work for images, but it serves my purposes.) - There
must be a better way to do this. Is this what nsIChannel::owner is for? I'm
confused about it's purpose.

Reproducible: Always
Steps to Reproduce:

Comment 1

[Simon Detheridge]

Attachment: Nasty hack to store nsIWebBrowser pointer in nsIHttpChannelInternal

This is a horrible hack to enable an http channel to know about the browser
instance that created it, so this information can be used to differentiate
between them later on, when the channel gets to an observer that doesn't
otherwise have this information.

Comment 2

[Boris Zbarsky [:bzbarsky]]

nsIChannel::owner should be a principal object (so should QI to nsIPrincipal).

It sounds like you're trying to use a global observer to observe all browser
instances; wouldn't it be more appropriate to have an observer per browser
instance?  The observer would then know what instance it was dealing with...

I don't think we can really have necko knowing about nsIWebBrowser, since that's
not even present in Mozilla proper (a side-effect of us not using our own
embedding framework!)... We could make use the the aContext param, I suppose...
but aren't people using that for something already?

Comment 3

[Simon Detheridge]

An observer that could operate per browser instance would be ideal, but I 
couldn't figure out a way to make that happen. If I could add an observer that 
fired on http-on-modify-request, and other http related events, but was 
associated with a single browser instance, I'd be happy. 
 
From the docs, I could only see a way to do this with the observer service, 
which is global. 

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #2)
> We could make use the the aContext param, I suppose...
> but aren't people using that for something already?

uriloader currently uses null for the context, but I want to change that. I wish
I remembered what I wanted to change it to.

(In reply to comment #3)
> An observer that could operate per browser instance would be ideal

http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsIWebBrowser.idl#60
?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, hmm.. yeah, necko doesn't really know anything about docshells, and
nsIWebBrowser seems to correspond to a toplevel docshell...

You could cheat by getting the notification callbacks from the channel/loadgroup
-- that's a pointer to the docshell that opened the channel, probably.  You'd
still need to figure out a way to get from the docshell (possibly a subframe
one) to the nsIWebBrowser.

I agree that we should make that easier, somehow.

Comment 6

[Simon Detheridge]

(In reply to comment #4) 
> (In reply to comment #3) 
> > An observer that could operate per browser instance would be ideal 
>  
> 
http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsIWebBrowser.idl#60 
> ? 
 
Specifies nsIWebProgressListener, and nsIHistoryListener. No mention of adding 
an observer that might suppot http-on-modify-request, et al. 
 
However, if I could use this function to register any observer that I could 
normally use with the observer service, in the context of a single web browser 
instance, that would be A Good Thing. 
 

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #6)
> an observer that might suppot http-on-modify-request, et al. 

oh, hmm, you want http-on-modify-request, and take different actions depending
on the nsiwebbrowser?

> However, if I could use this function to register any observer that I could 
> normally use with the observer service, in the context of a single web browser 
> instance, that would be A Good Thing. 

can't work, as (like bz said) http-on-modify-request is sent by necko, and that
knows nothing about browsers.

Comment 8

[Simon Detheridge]

(In reply to comment #7) 
> oh, hmm, you want http-on-modify-request, and take different actions 
depending 
> on the nsiwebbrowser? 
 
Yes. I don't see why I shouldn't be able to work out where a request came 
from. Call it nsIChannel::creator if you like, leave it as an nsISupports and 
if I can QI to an nsIDocShell then I can figure out where it came from. 
 
I'm not sure exactly, but when writing an app that embedded more than one 
instance of a web browser, I found it really frustrating that I couldn't 
figure out where everything was coming from and tweak accordingly.  
 
In the context of my app, it's important to be able to do this. (hence the 
hack/patch above) 

Comment 9

[Boris Zbarsky [:bzbarsky]]

> I don't see why I shouldn't be able to work out where a request came from.

Mostly because necko doesn't care, and hence doesn't know...  :(

Comment 10

[Simon Detheridge]

(In reply to comment #9) 
> > I don't see why I shouldn't be able to work out where a request came from. 
>  
> Mostly because necko doesn't care, and hence doesn't know...  :( 
 
Just because necko doesn't care, doesn't mean the observer won't. 
 
Whomever _created_ the channel that necko is operating on, knows who/what 
itself is. If 'I' am a docshell and 'I' am creating an http channel to do 
whatever, all I need to do is set a parameter on it pointing to myself, and 
all obervers that fire on it can find out who I am. If they can't qi that 
parameter to a docshell (or whatever they're expecting) then obviously it came 
from somewhere else in the framework so they're probably not interested in it. 
 
Basically, if everything that creates a channel specifies 'this' as a 
parameter somewhere (or nsnull if it's feeling particularly humble) then 
whenever that channel is observed further down the line, the observer has more 
information about what context that channel came from, and can potentially do 
more Clever Things as a result. 
 
Does that make sense? 

Comment 11

[Boris Zbarsky [:bzbarsky]]

It makes sense.  Unfortunately, some things have no idea who "they are".

If the image loader service creates a channel because it was asked to load an
image who is "this"?  The image loader service?  The thing that called the image
loader service (say a style rule)?  What happens when the style rule in question
is from a stylesheet not associated with any docshells or documents (eg a user
stylesheet)?  Similar questions for other services that perform network access
of some sort...

I'm just trying to establish where between "loads started directly by a docshell
should have a pointer to that docshell" and "all loads should have a pointer to
something nebulous that would be useful" we're targeting.  Then we get to figure
out how to do it within frozen necko and embedding apis...

Also, what about non-HTTP channels?  You're not really worried about those,
since you're doing HTTP-specific stuff, but is this something we need to address
for all channels or just HTTP ones?

Comment 12

[Simon Detheridge]

(In reply to comment #11) 
> It makes sense.  Unfortunately, some things have no idea who "they are". 
>  
> If the image loader service creates a channel because it was asked to load 
an 
> image who is "this"?  The image loader service?  The thing that called the 
image 
> loader service (say a style rule)?  What happens when the style rule in 
question 
> is from a stylesheet not associated with any docshells or documents (eg a 
user 
> stylesheet)?  Similar questions for other services that perform network 
access 
> of some sort... 
 
That's a good point. I guess though, that everything that could possibly have 
had something to do with creating a channel, will end up being a child of a 
nsIWebBrowser at some level? Which therefore points back to the idea of 
storing an nsIWebBrowser pointer in the channel, rather than an arbitrary 
'creator' pointer. - I can't think of any situation whereby a channel would be 
created that was not related to one of the web browser instances in an 
application. Or at least, not in _my_ application anyway. 
 
I suppose one could always use a null pointer for a channel that really *was* 
global, and not related to a specific browser instance. 
 
> I'm just trying to establish where between "loads started directly by a 
docshell 
> should have a pointer to that docshell" and "all loads should have a pointer 
to 
> something nebulous that would be useful" we're targeting.  Then we get to 
figure 
> out how to do it within frozen necko and embedding apis... 
 
Understood. For now, my application does not need to any processing of the 
image-related channels related to a document (not therefore directly 
instantiated by a docshell, although related to said document and underlying 
browser instance all the same) - but I can see how it would be useful to have 
this information in the future. 
 
> Also, what about non-HTTP channels?  You're not really worried about those, 
> since you're doing HTTP-specific stuff, but is this something we need to 
address 
> for all channels or just HTTP ones? 
 
Again, for *my* purposes, all I'm interested in right now is http. But also 
for my purposes, the patch I posted works fine... 
 
From the purposes of building a flexible system however, it seems more sense 
to have all channels who ultimately owe their existence to a specific web 
browser instance, to store some kind of reference to that instance. 

Comment 13

[Boris Zbarsky [:bzbarsky]]

> I can't think of any situation whereby a channel would be created that was not
> related to one of the web browser instances

Comment 11 gave an example of such a situation.  I could probably come up with a
few more if necessary (pac files, XUL stuff, XBL service, etc).  Any time a
resource is a singleton or shared across multiple documents...

Note also that comment 5 mentioned the notification callbacks, which _are_ the
sort of pointer you mention and are set in all gecko-internal cases when it's
feasible to set them (if they're not set, things like HTTP authentication will
fail).

Also note that channels can be opened outside necko/gecko (eg from javascript,
extensions, your embedding app, etc).  That also complicates things, since
requiring that something be set on channels is a de-facto change to the channel
API....

Comment 14

[Simon Detheridge]

(In reply to comment #13)  
> > I can't think of any situation whereby a channel would be created that was  
not  
> > related to one of the web browser instances  
>   
> Comment 11 gave an example of such a situation.  
 
Presumably the image loader service is loading an image on behalf of some 
document or other? 
 
> I could probably come up  
with a  
> few more if necessary (pac files, XUL stuff, XBL service, etc).  Any time a  
> resource is a singleton or shared across multiple documents...  
 
Hmm. "shared across multiple documents" I hadn't considered. In the case of a 
singleton though, it would be feasible to use a null pointer as it's not 
related to a browser. (Any observer that was expecting it to be could see a 
null pointer and either do it's default processing, or leave it alone 
depending on what was needed) 
 
> Note also that comment 5 mentioned the notification callbacks, which _are_  
the  
> sort of pointer you mention and are set in all gecko-internal cases when  
it's  
> feasible to set them (if they're not set, things like HTTP authentication  
will  
> fail).  
 
Ohh... It's possible then to get a docshell from the nsIInterfaceRequestor in 
the cases I'm interested in? (how?) I didn't realise that this was possible 
from looking at the docs. 
 
> Also note that channels can be opened outside necko/gecko (eg from  
javascript,  
> extensions, your embedding app, etc).  That also complicates things, since  
> requiring that something be set on channels is a de-facto change to the  
channel  
> API.... 
 
See above about null pointers... 

Comment 15

[Boris Zbarsky [:bzbarsky]]

> Presumably the image loader service is loading an image on behalf of some 
> document or other? 

No, not necessarily.  Comment 11 described an explicit situation where it's not,
in fact.

> It's possible then to get a docshell from the nsIInterfaceRequestor in 
> the cases I'm interested in? (how?)

QueryInterface should do the trick.  ;)  Assuming the load was started by a
docshell.  People outside gecko starting channel loads are free to point that
pointer to whatever they want to, as I said.

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #14)
> Ohh... It's possible then to get a docshell from the nsIInterfaceRequestor in 
> the cases I'm interested in? (how?) I didn't realise that this was possible 
> from looking at the docs. 

that's because nothing guarantees that you will be able to get a docshell from that.

hm.. maybe we should specify that the notificationCallbacks will provide access
to the docshell via GetInterface, if such a docshell is available? it seems like
that would only require changing nsDocShell (or maybe not even that, in case it
has a return QueryInterface(aIID, aRetval) in its GetInterface impl)

Comment 17

[Boris Zbarsky [:bzbarsky]]

> maybe we should specify that the notificationCallbacks will provide access
> to the docshell via GetInterface,

That's an API change to nsIChannel, effectively, again as I said.

> it seems like that would only require changing nsDocShell 

And also xslt (can maybe hack in docshells), webbrowserpersist (no docshell
there!), uri checker, sync load service, the directory viewer, and whatever
embeddors are opening channels and using the _frozen_ nsIChannel API to set
notification callbacks.

Comment 18

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

could also use nsIPropertyBag2 these days.