Support move-only types for MozPromise

RESOLVED FIXED in Firefox 55

Status

()

Core
XPCOM
P1
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

5 months ago
It would be quite useful to support move-only types when IsExclusive [1] is true for there is at most only one consumer and it is safe to move the ResolveOrRejectValue stored in the promise.

[1] http://searchfox.org/mozilla-central/rev/8b70b0a5038ef9472fe7c53e04a70c978bb06aed/xpcom/threads/MozPromise.h#130
(Assignee)

Updated

5 months ago
Depends on: 1362912
(Assignee)

Updated

5 months ago
Depends on: 1363676
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

5 months ago
Attachment #8866665 - Flags: review?(gsquelart)
Attachment #8866666 - Flags: review?(gsquelart)
Attachment #8866667 - Flags: review?(gsquelart)
(Assignee)

Updated

5 months ago
Assignee: nobody → jwwang
Priority: -- → P1

Comment 4

5 months ago
mozreview-review
Comment on attachment 8866665 [details]
Bug 1362910. P1 - enable move when IsExclusive is true.

https://reviewboard.mozilla.org/r/138272/#review141466
Attachment #8866665 - Flags: review?(gsquelart) → review+

Comment 5

5 months ago
mozreview-review
Comment on attachment 8866666 [details]
Bug 1362910. P2 - fix callers.

https://reviewboard.mozilla.org/r/138274/#review141470
Attachment #8866666 - Flags: review?(gsquelart) → review+

Comment 6

5 months ago
mozreview-review
Comment on attachment 8866667 [details]
Bug 1362910. P3 - add a gtest to test move-only types with MozPromise.

https://reviewboard.mozilla.org/r/138276/#review141478

::: dom/media/gtest/TestMozPromise.cpp:340
(Diff revision 1)
> +      EXPECT_FALSE(aVal.IsNothing());
> +      EXPECT_TRUE(aVal.IsResolve());
> +      EXPECT_FALSE(aVal.IsReject());
> +      EXPECT_EQ(87, *aVal.ResolveValue());
> +
> +      // Move() shouldn't change the resovle/reject state of aVal.

resovle -> resolve
Attachment #8866667 - Flags: review?(gsquelart) → review+
(Assignee)

Comment 7

5 months ago
Thanks for the review!
Comment hidden (mozreview-request)

Comment 9

5 months ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3e212b01bec
P1 - enable move when IsExclusive is true. r=gerald
https://hg.mozilla.org/integration/autoland/rev/f3590d95b1a6
P2 - fix callers. r=gerald
https://hg.mozilla.org/integration/autoland/rev/a9520acf01ad
P3 - add a gtest to test move-only types with MozPromise. r=gerald

Comment 10

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b3e212b01bec
https://hg.mozilla.org/mozilla-central/rev/f3590d95b1a6
https://hg.mozilla.org/mozilla-central/rev/a9520acf01ad
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1364736
No longer depends on: 1364736
You need to log in before you can comment on or make changes to this bug.