Closed Bug 561830 Opened 14 years ago Closed 14 years ago

e10s HTTP: nsIDocShellTreeItem crashes

Categories

(Core :: Networking: HTTP, defect)

Other Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: lusian, Unassigned)

References

()

Details

Attachments

(1 file)

HttpChannelParent needs to implement nsIDocShellTreeItem.

Accessing http://bugzilla.mozilla.org kills the browser, too.
Blocks: fennecko
Er... why does something in _necko_ need to implement nsIDocshellTreeItem?
I was testing Jason's patch in Bug 536292, and the browser died on the web sites.  Please advise.
Well, for starters not all HTTP channels have an associated nsIDocShellTreeItem.  Or more precisely, some HTTP channel callbacks don't hand one of those out from GetInterface for security reasons.
OK, so it sounds like our issue here is that something is trying to get a nsIDocShellTreeItem from HttpChannelParent via GetInterface().  Boris, does it makes sense to try to hand out a nsIDocShellTreeItem to something in chrome?  Jae, what's the code that's asking for the nsIDocShellTreeItem?  I'm guessing it may need to be changed.
> something is trying to get a nsIDocShellTreeItem from HttpChannelParent via
> GetInterface().

Quite likely, yes.  Can we hunt down that something?

> does it makes sense to try to hand out a nsIDocShellTreeItem to something in
> chrome? 

Only marginally.  Can we locate the caller and see what it really wants here?
Attached file stack trace
mozilla::net::HttpChannelParent::GetInterface Line 271 << dies here.
nsInterfaceRequestorAgg::GetInterface Line 62 + 0x24 bytes
nsHttpConnection::GetInterface Line 828 + 0x21 bytes
That's not very enlightening; I wonder who posts that event...
A fair bit of extension code expects to be able to get a docshell from HTTP requests so that it can decide whether to allow or prevent the request. I don't know whether product code makes similar assumptions. I suspect we are going to have to, at some point, be able to go from httpchannel -> PIFrameEmbedding or something like that, in order for security-UI to be implemented entirely on the chrome side.

I don't actually know what this request is, though... sounds like we need a longer stacktrace or better data.
> A fair bit of extension code expects to be able to get a docshell from HTTP
> requests so that it can decide whether to allow or prevent the request.

Yes, but we don't _have_ a docshell in the chrome process.  Do we just want to hand out a proxy?

And note that such code is broken and should be using nsILoadContext (which we should change as needed), not docshell.
Oh, I agree it's broken, or at least will be unavoidably broken when we have remote tabs. I just think we should design the replacement to actually meet our needs in-app as well as extensions.
Absolutely.  The idea of nsILoadContext was that we could modify it as needed to meet our needs as we did e10s...  Dunno that it succeeded at that.  ;)
from nsHttpConnection.cpp:

// not called on the socket transport thread
NS_IMETHODIMP
nsHttpConnection::GetInterface(const nsIID &iid, void **result)
{
    // NOTE: This function is only called on the UI thread via sync proxy from
    //       the socket transport thread.  If that weren't the case, then we'd
    //       have to worry about the possibility of mTransaction going away
    //       part-way through this function call.  See CloseTransaction.
Summary: e10s HTTP: Implement nsIDocShellTreeItem → e10s HTTP: nsIDocShellTreeItem crashes
from nsHttpConnection.cpp:

// not called on the socket transport thread
NS_IMETHODIMP
nsHttpConnection::GetInterface(const nsIID &iid, void **result)
{
    // NOTE: This function is only called on the UI thread via sync proxy from
    //       the socket transport thread.  If that weren't the case, then we'd
    //       have to worry about the possibility of mTransaction going away
    //       part-way through this function call.  See CloseTransaction.
To reproduce:  goto either http://wiki.mozilla.org or http://bugzilla.mozilla.org.
Fixed as part of bug 536292.

http://hg.mozilla.org/projects/electrolysis/rev/11682e4090a9
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: