1400501 - Store a list of ancestor principals in document objects

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Boris Zbarsky [:bzbarsky]]

There are various things like bug 1305237 and probably bug 1085214 that will want to know the list of "ancestor principals" for a document.  The way I would define that is that this list is empty for a toplevel document.  For a non-toplevel document X that is nested through document Y (in the https://html.spec.whatwg.org/multipage/browsers.html#browsing-context-nested-through sense), the list for Y is the list for X, with the principal of X prepended.

If we actually had an accessor for the "nested through" concept in the spec, we could probably use that.  GetParentDocument() doesn't work for this purpose, because we null that out when the child's docshell is navigated.  We could build something on top of getting the docshell and then getting the window and then getting the frameElement and then the ownerDocument.  But this is also moderately fragile, especially the bit about "getting the docshell", if we're talking about a navigated-away-from document.  I'd rather not rely on fragile things for security state.

Hence the proposal to just snapshot this information at document creation.  This has the benefit of playing nicer with things like out-of-process iframes too: instead of querying things sync cross-process, we just have to ship over the one immutable list once.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Store a list of ancestor principals on docshell and document

Right now every document in a docshell makes a copy of the list.  In practice,
this list is usually pretty short (limited by depth of iframe nesting), so this
is probably not a problem.  We could add a bit of complexity and have a
refcounted struct that contains the list... I wish we had something as simple
as Rust's Arc that we could use here.

MozReview-Commit-ID: 8jGIlkhp1DU

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8908959 [details] [diff] [review]
Store a list of ancestor principals on docshell and document

Review of attachment 8908959 [details] [diff] [review]:
-----------------------------------------------------------------

It wouldn't be hard to add something like rust's Arc/Rc if we thought it would have usage - probably as a wrapper around RefPtr:

template<typename T>
using Arc = RefPtr<ArcBox<T>>;

template<typename T>
class ArcBox : public T {
  //  ... constructors etc...
  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ArcBox)
};

// .. Probably a MakeArc free function to construct one?

::: docshell/base/nsDocShell.h
@@ +1130,5 @@
>    uint32_t mTouchEventsOverride;
>  
> +  // Our list of ancestor principals.
> +  nsTArray<nsCOMPtr<nsIPrincipal>> mAncestorPrincipals;
> +  

nit: whitespace

::: dom/base/nsIDocument.h
@@ +456,5 @@
> +   * ancestor list for the document's docshell the last time SetContainer() was called
> +   * with a non-null argument. See the documentation for the corresponding getter in
> +   * docshell for how this list is determined.
> +   */
> +  const nsTArray<nsCOMPtr<nsIPrincipal>>& AncestorPrincipals() const

I don't see a way that this can be mutated in the DocShell right now, so is there a reason we don't just fetch this value from the DocShell when we need it rather than performing a copy into nsIDocument?

I'm assuming there's a reason (as you did mention that you were considering using refcounting here), but I don't see it immediately.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to Michael Layzell [:mystor] from comment #2)
> I don't see a way that this can be mutated in the DocShell right now, so is
> there a reason we don't just fetch this value from the DocShell when we need
> it rather than performing a copy into nsIDocument?
> 
> I'm assuming there's a reason (as you did mention that you were considering
> using refcounting here), but I don't see it immediately.

Nevermind - I should read comment 0 ^.^

Comment 4

[Boris Zbarsky [:bzbarsky]]

> nit: whitespace

Fixed.

> Nevermind - I should read comment 0 ^.^

No, I should have better code comments.  Added those:

   * We store a copy of the list, because we may lose the ability to reach our docshell
   * before people stop asking us for this information.

Filed bug 1401008 on Arc/Rc.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe05bde7b58f
Store a list of ancestor principals on docshell and document.  r=mystor

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fe05bde7b58f