Closed Bug 969165 Opened 6 years ago Closed 6 years ago

Change Atomic<T> users to Atomic<bool> where that's what they wanted, now that Atomic<bool> works

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: Waldo, Assigned: Waldo)

References

Details

Attachments

(8 files)

I did a skim of all instances of Atomic< in the tree and quickly fixed the bool-ish ones to be Atomic<bool> (and interactions with them to be bool-ish, too).  Relatively simple patches shortly.
Attached patch content/Splinter Review
Attachment #8371950 - Flags: review?(bent.mozilla)
Attached patch image/Splinter Review
Attachment #8371951 - Flags: review?(bobbyholley)
Attached patch js/src/Splinter Review
Attachment #8371952 - Flags: review?(shu)
Attached patch security/Splinter Review
Attachment #8371954 - Flags: review?(brian)
Attached patch xpcom/Splinter Review
Attachment #8371956 - Flags: review?(nfroyd)
Attached patch dom/Splinter Review
Attachment #8371957 - Flags: review?(bent.mozilla)
Attached patch widget/Splinter Review
Attachment #8371958 - Flags: review?(vladimir)
Attached patch ipc/Splinter Review
Attachment #8371959 - Flags: review?(continuation)
Attachment #8371959 - Flags: review?(continuation) → review?(bent.mozilla)
Comment on attachment 8371951 [details] [diff] [review]
image/

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

I'm technically not an ImageLib peer anymore, but...
Attachment #8371951 - Flags: review?(bobbyholley) → review+
Comment on attachment 8371956 [details] [diff] [review]
xpcom/

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

Bonus points for whitespace cleanup.
Attachment #8371956 - Flags: review?(nfroyd) → review+
Comment on attachment 8371952 [details] [diff] [review]
js/src/

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

Missed a few places but looks good to me, I trust they'll be patched.

::: js/src/vm/Runtime.h
@@ +685,2 @@
>       */
> +    mozilla::Atomic<bool, mozilla::Relaxed> interrupt;

Should also change the writes of interrupt to be true or false over 0 and 1, like in various places in Runtime.cpp and js_InvokeOperationCallback in jscntxt.cpp

@@ +1285,2 @@
>       */
> +    mozilla::Atomic<bool, mozilla::ReleaseAcquire> gcMallocGCTriggered;

I wonder why this is ReleaseAcquire?
Attachment #8371952 - Flags: review?(shu) → review+
Attachment #8371957 - Flags: review?(bent.mozilla) → review+
Attachment #8371959 - Flags: review?(bent.mozilla) → review+
Comment on attachment 8371950 [details] [diff] [review]
content/

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

There are two unrelated whitespace changes here that shouldn't get checked in.
Attachment #8371950 - Flags: review?(bent.mozilla) → review+
Attachment #8371954 - Flags: review?(brian) → review+
https://hg.mozilla.org/mozilla-central/rev/51260f68e21f
https://hg.mozilla.org/mozilla-central/rev/06a9e307dcc1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.