1443956 - Support directly using nsIURI and nsIPrincipal in IPDL

Status: RESOLVED FIXED

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

Description

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

With bug 1443954, the ability to directly use refcounted types in IPDL will be added.

We should use this capability to allow nsIURI and nsIPrincipal to be embedded directly in IPDL structs and used as IPDL arguments.

Comment 1

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

Attachment: Part 1: Support serializing nsIURI directly over IPDL

MozReview-Commit-ID: 1QWdw1qzcmR

Comment 2

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

Attachment: Part 2: Support serializing nsIPrincipal directly over IPDL

MozReview-Commit-ID: 5XnyVyMceuy

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8957009 [details] [diff] [review]
Part 2: Support serializing nsIPrincipal directly over IPDL

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

::: dom/ipc/PermissionMessageUtils.cpp
@@ -22,3 @@
>    nsCString principalString;
> -  nsCOMPtr<nsISerializable> serializable = do_QueryInterface(aParam.mPrincipal);
> -  if (serializable) {

Hah, we've been doing this completely unnecessary QI check for sooo long. :(

::: dom/ipc/PermissionMessageUtils.h
@@ +20,5 @@
> +  static bool Read(const Message* aMsg, PickleIterator* aIter, RefPtr<nsIPrincipal>* aResult);
> +};
> +
> +/**
> + * Legacy IPC::Principal type. Use nsIPrincipal directly in new IPDL code.

Do we have a bug on file to nuke this?

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Don't we need these types to be threadsafe first?

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Also, is this serializing CSP data?  We have explicitly chosen *not* to propagate this across IPC in PrincipalInfo because we have problems with the data spreading where it should not.

Comment 6

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

(In reply to Ben Kelly [Part-time, not reviewing][:bkelly] from comment #4)
> Don't we need these types to be threadsafe first?

We don't need these types to be threadsafe to use them on the main thread.

---

(In reply to Ben Kelly [Part-time, not reviewing][:bkelly] from comment #5)
> Also, is this serializing CSP data?  We have explicitly chosen *not* to
> propagate this across IPC in PrincipalInfo because we have problems with the
> data spreading where it should not.

This serializes the exact same data as IPC::Principal, which happens to include CSP data in its serialization. I think this will probably be changing in bug 965637.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/390bb0f184d1
Part 1: Support serializing nsIURI directly over IPDL, r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/30c42e2e0944
Part 2: Support serializing nsIPrincipal directly over IPDL, r=froydnj

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/390bb0f184d1
https://hg.mozilla.org/mozilla-central/rev/30c42e2e0944