Closed Bug 1367176 Opened 5 years ago Closed 4 years ago

The MozPromise typedef generated for async message returns should be public

Categories

(Core :: IPC, enhancement)

55 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: haik, Assigned: kanru)

Details

Attachments

(1 file)

With the new support to return MozPromises from async messages, the generated receiver typedef for the MozPromise should be public instead of private so the type can be reused in other classes we need to pass the MozPromise to.

An example is 

  http://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/_ipdlheaders/mozilla/dom/PContentParent.h#1029

  1029 typedef MozPromise<PipelineId, PromiseRejectReason, false>::Private AllocPipelineIdPromise;
(In reply to Haik Aftandilian [:haik] from comment #0)
> With the new support to return MozPromises from async messages, the
> generated receiver typedef for the MozPromise should be public instead of
> private ...

That should read "public instead of protected".
Comment on attachment 8870916 [details]
Bug 1367176 - Expose promise typedef to public so it can be used outside of actor.

https://reviewboard.mozilla.org/r/142486/#review146208

::: ipc/ipdl/ipdl/lower.py:2723
(Diff revision 1)
> +                self.cls.addstmt(
> +                    Typedef(_makePromise(md.returns, self.side, resolver=True),
> +                            md.promiseName()))
> +        self.cls.addstmt(Whitespace.NL)
> +
> +        self.cls.addstmt(Label.PUBLIC)

Do we need another public:?
Attachment #8870916 - Flags: review?(wmccloskey) → review+
Comment on attachment 8870916 [details]
Bug 1367176 - Expose promise typedef to public so it can be used outside of actor.

https://reviewboard.mozilla.org/r/142486/#review146208

> Do we need another public:?

It looks nicer in the generated code. I think I can put them together with the standard typedefs so we don't need another public:
(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #5)
> Comment on attachment 8870916 [details]
> Bug 1367176 - Expose promise typedef to public so it can be used outside of
> actor.
> 
> https://reviewboard.mozilla.org/r/142486/#review146208
> 
> > Do we need another public:?
> 
> It looks nicer in the generated code. I think I can put them together with
> the standard typedefs so we don't need another public:

The typedef has to be defined before the Recv methods so I have to move the code to the original place and add more public: and protected: labels :(
Pushed by kchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8db29d17d984
Expose promise typedef to public so it can be used outside of actor. r=billm
https://hg.mozilla.org/mozilla-central/rev/8db29d17d984
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee: nobody → kanru
You need to log in before you can comment on or make changes to this bug.