1547218 - IPDLParamTraits<nsTArray<T>> doesn't work if T = RefPtr<U>

Status: RESOLVED FIXED

(Core :: IPC, defect, P2)

Description

[Andreas Farre [:farre]]

BrowsingContext is refcounted, and using BrowsingContext[] would trigger us using IPDLParamTraits<nsTArray<RefPtr<BrowsingContext>>::WriteInternal which moves the elements of the array into WriteIPDLParam which in turn tries to invoke BrowsingContext::~delete and that is deleted due to it being refcounted.

That is, if we have:

template <typename T>
struct Strip {
  typedef T Type;
};

template <typename T>
struct Strip<RefPtr<T>> {
  typedef T Type;
};

template <typename T>
static void Baz(T* a) {

}

template <typename T>
struct Foo {
  template <typename U>
  static void Bar(U&& a) {
    Baz<typename Strip<T>::Type>(std::forward<U>(a));
  }
};

static void Qux() {
  RefPtr<BrowsingContext> c;
  Foo<RefPtr<BrowsingContext>>::Bar(std::move(c));
}

stuff breaks.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Seems like a defect to me if valid/reasonable IPDL results in invalid C++.

Comment 2

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

I think this is caused by some unfortunate fallout from the way we perform IPDLParamTraits selection.

I have a local patch which flips around how we handle refcounted type selection here, which should make the behaviour for nsTArray<T> and other compound types perform better.

Comment 3

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

Attachment: Bug 1547218 - Part 1: Handle container IPDLParamTraits types more consistently,

Comment 4

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

Attachment: Bug 1547218 - Part 2: Stop special casing pointer types in ParamTraits specialization,

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jld, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

This has r+'ed patches; is there anything else that needs to happen to land this?

Comment 7

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

This bug is currently on the stack after my changes in bug 1540731, which until very recently were blocked on :froydnj's review. My current intention is to not land any of these changes until after the merge, to minimize the chance of fallout.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jld, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15b4b7f9d97a
Part 1: Handle container IPDLParamTraits types more consistently, r=froydnj
https://hg.mozilla.org/integration/autoland/rev/a50b68e0deb2
Part 2: Stop special casing pointer types in ParamTraits specialization, r=froydnj

Comment 10

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/15b4b7f9d97a
https://hg.mozilla.org/mozilla-central/rev/a50b68e0deb2