For the AArch64 Windows port, I wrote some dodgy code to use the x86 code from MSVC for chromium atomics. As you might imagine, x86(-64) and AArch64 have completely dissimilar memory models, so while my changes might have compiled, they were most likely semantically incorrect. Rather than trying to puzzle out what the exact AArch64 instruction sequences are, and inflicting the same on some poor reviewer, let's just blow up the need for the chromium atomics in the first place.

Their uses are easily replaced with simpler classes.

C++11 provides guaranteed thread-safe static initialization, so we can use that instead of ipc's baroque Singleton class.

C++11 provides guaranteed thread-safe static initialization, so we can use that instead of ipc's baroque Singleton class.

C++11 provides guaranteed thread-safe static initialization, so we can use that instead of ipc's baroque Singleton class.

C++11 provides guaranteed thread-safe static initialization, so we can use that instead of ipc's baroque Singleton class.

When we remove singleton.h, some webrtc code will be unhappy, because of cargo-culted definitions from singleton.h or included headers. Let's fix those first

C++11 provides guaranteed thread-safe static initialization, so we can use that instead of ipc's baroque Singleton class.

These headers are no longer used. Try is telling me there may be some small additional fixes required in webrtc code.

The only consumer of this code was Singleton, which we previously removed, and everything that this code accomplished can be done more simply and more foolproof-y by standard constructs these days.

Comment on attachment 9013810 [details] [diff] [review] remove ipc/'s copies of *AtomicSequenceNum classes Review of attachment 9013810 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/chromium/src/chrome/common/ipc_channel.cc @@ +17,4 @@ > namespace { > > // Global atomic used to guarantee channel IDs are unique. > +mozilla::Atomic<int> g_last_id; Should this be Atomic<unsigned>, given that it's printf'ed as %u?

Thanks for getting rid of this code.

jesup indicated in an IRC conversation that touching imported WebRTC code isn't really desirable. In light of that, I think what we want to do is: 1) Keep the C++11 statics changes for our own code, since IMHO it's clearer. 2) Redo singleton.h using C++11 atomics, which should be relatively straightforward. It's *possible* that step 2 will, since it removes the need for Chromium atomics, still require touching WebRTC headers. Would that be acceptable if it happened? Jed, WDYT about the above plan? Are you comfortable reviewing the patch for part 2?

I could review that patch, but that Singleton dependency isn't from upstream WebRTC; it's something we changed locally in bug 1219339. WebRTC has its own renamed import of the Chromium base library, so generally they shouldn't need to borrow our Chromium bits like that unless it's our change. Also, the upstream repo doesn't have static_instance.h at all anymore — they removed it a year ago in https://webrtc.googlesource.com/src/+/779d3657ef12b65eb55455c6f415b35522c07c97 — so we might not even need to merge it the next time it's updated.

Comment on attachment 9013816 [details] [diff] [review] use C++11 statics in static_instance.h Review of attachment 9013816 [details] [diff] [review]: ----------------------------------------------------------------- r+; removed from upstream so we don't have to carry this

Comment on attachment 9013815 [details] [diff] [review] fix webrtc header cargo culting Review of attachment 9013815 [details] [diff] [review]: ----------------------------------------------------------------- already fixed in upstream, r+

Needed this to fix Windows builds. upstream seems to have already done this: https://webrtc.googlesource.com/src/+/master/modules/video_capture/windows/device_info_ds.h#14

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2296a2feb192 remove ipc/'s copies of *AtomicSequenceNum classes; r=jld https://hg.mozilla.org/integration/mozilla-inbound/rev/cdd41eaece0f use C++11 statics in CamerasChild; r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/6fc7db6d7326 use C++11 statics in ipc_channel_posix.cc; r=jld https://hg.mozilla.org/integration/mozilla-inbound/rev/d95dd4c9cdca use C++11 statics for Faulty instance; r=decoder https://hg.mozilla.org/integration/mozilla-inbound/rev/06a1c7fa3ba9 use C++11 statics in time_win.cc; r=jld https://hg.mozilla.org/integration/mozilla-inbound/rev/1fe17ae969d6 fix webrtc header cargo culting; r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/1815bfcf7984 use C++11 statics in static_instance.h; r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/a2a04672f049 fix windows includes of singleton.h; r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/323979f32e2d remove singleton headers from ipc/; r=jld https://hg.mozilla.org/integration/mozilla-inbound/rev/36356f99018a remove chromium atomics code; r=jld

https://hg.mozilla.org/mozilla-central/rev/2296a2feb192 https://hg.mozilla.org/mozilla-central/rev/cdd41eaece0f https://hg.mozilla.org/mozilla-central/rev/6fc7db6d7326 https://hg.mozilla.org/mozilla-central/rev/d95dd4c9cdca https://hg.mozilla.org/mozilla-central/rev/06a1c7fa3ba9 https://hg.mozilla.org/mozilla-central/rev/1fe17ae969d6 https://hg.mozilla.org/mozilla-central/rev/1815bfcf7984 https://hg.mozilla.org/mozilla-central/rev/a2a04672f049 https://hg.mozilla.org/mozilla-central/rev/323979f32e2d https://hg.mozilla.org/mozilla-central/rev/36356f99018a

We've landed this, I guess ni? on jesup is silly now.