1453633 - Make nsHostObjectURI hold a threadsafe wrapper of nsIPrincipal

Status: RESOLVED FIXED

(Core :: DOM: File, enhancement)

Description

[Valentin Gosu [:valentin] (he/him)]

The problem of principals was pointed out in bug 1448328. They are not threadsafe yet, and addref-ing/releasing them OMT is not possible at the moment.
The easy solution to make nsHostObjectURI usable/clonable OMT is to make it hold a threadsafe wrapper containing the principal object.
Once created on the main thread, it can be addref-ed on any thread, and when its refcount reaches 0 we dispatch to the main thread to release the principal.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1453633 - Make nsHostObjectURI hold its principal wrapped in a nsMainThreadPtrHandle

nsIPrincipal is not yet threadsafe (bug 1443925). This is a problem in the context of threadsafe nsIURI, where cloning an nsIURI would also AddRef the principal.
To get around this problem, we use nsMainThreadPtrHandle<nsIPrincipal>. This means cloning the URI would not AddRef the principal (it addrefs the nsMainThreadPtrHolder that holds the principal). When the last ref is dropped, the principal is released on the main thread.

We should get rid of this once principals become thread-safe

Review commit: https://reviewboard.mozilla.org/r/236068/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/236068/

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8967391 [details]
Bug 1453633 - Make nsHostObjectURI hold its principal wrapped in a nsMainThreadPtrHandle

https://reviewboard.mozilla.org/r/236068/#review241820

::: dom/file/nsHostObjectURI.h:103
(Diff revision 1)
>      }
>  
>      MOZ_MUST_USE NS_IMETHOD
>      SetPrincipal(nsIPrincipal *aPrincipal) override
>      {
>          if (!mURI) {

MOZ_ASSERT(NS_IsMainThread())

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8967391 [details]
Bug 1453633 - Make nsHostObjectURI hold its principal wrapped in a nsMainThreadPtrHandle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/236068/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/165ff300ac95
Make nsHostObjectURI hold its principal wrapped in a nsMainThreadPtrHandle r=baku

Comment 5

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/165ff300ac95