976107 - Add creating document to every channel

Status: RESOLVED INVALID

(Core Graveyard :: Security: UI, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

Adding the creating document to every channel would allow us to fix two bugs:

1) We could us this document as the requestingContext for Mixed Content Blocking when evaluating channel redirects [Bug 418354]. In the current patch for Bug 418354, we set the requesting context on every http channel where shouldLoad is called, which seems error prone. I guess we could clean up that patch by providing a clean solution for setting the document on every channel in this bug. (Please keep in mind that we would also need the loadType to be set on the channel).

2) We could use the document to ignore requests which where not triggered by the current top level URL (address bar URL). At the moment nsSecureBrowserUIImpl.cpp gets confused whenever pasting in a new URL in the address bar where requests from the previous page are still arriving and cause a misclassification, therefore the grey triangle to be displayed incorrectly [Bug 947079].

Now the qustion is, what is the best approach to accomplish this setting of the document on every channel?

Comment 1

[Boris Zbarsky [:bzbarsky]]

So some general thoughts I've had so far:

1)  We could hang a document off something like the object I want to add in bug 965413.  But doing that without leaks might be hard...  In particular, the channel would have to make sure to drop the ref to the document at the same time as it drops the refs to its stream listener and the like.

2)  We could try to have per-document loadgroups, not just per-docshell ones.  The load that leads to the creation of a new document would have to be moved to the new loadgroup once it exists.

Ccing some people who may have other ideas.  So far, I like #2 a lot more than #1, though it seems like a bit more up-front work.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is comment 0 really asking to know which document loads something, or which "document that is being displayed to the user".

I think the latter is what we generally care about. Especially since we generally forbid data-documents and svg-not-as-document-but-as-image from loading things.

Comment 3

[Boris Zbarsky [:bzbarsky]]

What is really desired for mixed content stuff is "which inner Window is this load associated with", since what mixed content stuff cares about is comparing the scheme of the thing being loaded to whoever is doing the loading...

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Right. We don't need nested loadgroups for that. But I agree that we need a more reasonable way for going from channels to the principal/inner-window that we should do security checks against.

Comment 5

[Patrick McManus [:mcmanus]]

(In reply to Boris Zbarsky [:bz] (reviews will be slow; ask someone else) from comment #1)
> \
> 
> 2)  We could try to have per-document loadgroups, not just per-docshell
> ones.  The load that leads to the creation of a new document would have to
> be moved to the new loadgroup once it exists.

its kind of obvious to say, but if we did that they would need to be chained together somehow as some of the necko scheduler is more interested in the tab or potentially viewable area..

I don't think that would be a problem
> 
> Ccing some people who may have other ideas.  So far, I like #2 a lot more
> than #1, though it seems like a bit more up-front work.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Right, I was figuring the document loadgroups would be in the docshell loadgroup.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

This bug became invalid since we filed Bug 1063197 and also Bug 1006868.