IPDL: Replace nsTArray with InfallibleTArray

RESOLVED FIXED

Status

()

Core
IPC
--
enhancement
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(3 attachments)

The arrays used to track managed actors and shmems are, ahem, already sort of used infallibly.  s/nsTArray/InfallibleTArray/.
After starting to implement this, I realized it's not quite so straightforward as I thought at first.  There are two high-level decisions to make

 (1) Which array type (fallible/infallible) should be used for IPDL actors' mManagedPFoo and mShmem?

 (2) Which array type should be used for Send(Foo[]) and Recv(Foo[])?

Comment 0 referered to question (1), because those arrays are already treated infallibly.  Switching those over to infallible is trivial, and we could also (less-trivially) switch back to fallible arrays if we modified the IPDL-generated code that mucks with them to error check.

(2) is perhaps a more interesting question.  We can't templatize Send/Call(Foo[]) on the array allocator type, because that would require an inline definition, which would cause circular-class-definition errors.  So we have to choose.

In dom/plugins code, mainly NPRuntime, we construct argument arrays for Call/Send the length of which are controlled by plugins (and on the browser side by content script).  We should use fallible arrays for these, definitely.  But, which type should the IPDL-generated C++ Send/CallFoo() accept?

I argue for SendFoo(InfallibleTArray).  Reasoning
  (a) It will be uncommon that argument arrays are under direct control by web content, and InfallibleTArray is a friendlier interface
  (b) Code that needs to construct fallible argument arrays can use nsTArray<fallible> and then Swap() to an InfallibleTArray just before sending, for free.

I think we should deserialize InfallibleTArrays "fallibly", meaning that we deserialize to an nsTArray<fallible> and then Swap() to an InfallibleTArray.  (This doesn't matter too much, it's just so that we can keep a uniform Send/Recv interface.)
Created attachment 442306 [details] [diff] [review]
Followup to bug 550611: Allow TArrays with different allocators to interoperate and implement auto arrays for the convenience subclasses  

I'll qfold this patch into bug 550611 before pushing, but I decided to post it here to show the context that motivated the changes.
Assignee: nobody → jones.chris.g
Attachment #442306 - Flags: review?(benjamin)
Attachment #442306 - Flags: review?(benjamin) → superreview?(benjamin)
Created attachment 442307 [details] [diff] [review]
Use InfallibleTArray exclusively in IPDL-generated code

Most of this was mechanical, the only interesting part is the change to ProtectedVariantArray.
Attachment #442307 - Flags: review?(bent.mozilla)

Updated

8 years ago
Attachment #442306 - Flags: superreview?(benjamin) → superreview+
Comment on attachment 442307 [details] [diff] [review]
Use InfallibleTArray exclusively in IPDL-generated code

Looks good!
Attachment #442307 - Flags: review?(bent.mozilla) → review+
Created attachment 487500 [details] [diff] [review]
Update IPDL-InfallibleTArray to trunk

Mechanical update, so not requesting individual r? on various modules.
Attachment #487500 - Flags: superreview?(benjamin)
Suggest blocking-final or blocking-fennec-final for the same reasons as infallible-tarray blocks for the HTML5 parser; all the IPDL code was written assuming infallible TArray.
blocking2.0: --- → ?
tracking-fennec: --- → ?
(In reply to comment #6)
> Suggest blocking-final or blocking-fennec-final for the same reasons as
> infallible-tarray blocks for the HTML5 parser; all the IPDL code was written
> assuming infallible TArray.

YES PLEASE!

Updated

7 years ago
blocking2.0: ? → final+

Updated

7 years ago
Attachment #487500 - Flags: superreview?(benjamin) → superreview+
http://hg.mozilla.org/mozilla-central/rev/15765a60e203
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.