Replace Acquire_Store calls with Release_Store as these stores should have the release ordering/sem…

chromecast/media/cma/ipc/media_message_fifo.cc

Index: chromecast/media/cma/ipc/media_message_fifo.cc
diff --git a/chromecast/media/cma/ipc/media_message_fifo.cc b/chromecast/media/cma/ipc/media_message_fifo.cc
index f81bb4c49c23561675c2c998a48fffe4d6e64bcf..4adbb67e0b4ec4bff7074408e24d858d263b9504 100644
--- a/chromecast/media/cma/ipc/media_message_fifo.cc
+++ b/chromecast/media/cma/ipc/media_message_fifo.cc
@@ -123,8 +123,8 @@ MediaMessageFifo::MediaMessageFifo(
     desc->size = size_;
     internal_rd_offset_ = 0;
     internal_wr_offset_ = 0;
-    base::subtle::Acquire_Store(rd_offset_, 0);
-    base::subtle::Acquire_Store(wr_offset_, 0);
+    base::subtle::Release_Store(rd_offset_, 0);

[servolk, 2014/10/07 19:43:37]

The bug referenced in the description
(https://code.google.com/p/chromium/issues/detail?id=420970) talks about
removing BOTH Acquire_Store and Release_Store and replacing them with proper
C++11 atomics. So why not use C++11 atomics here (std::atomic_store, I suppose)?
Or is that something that Chromecast can't support yet due to being on older
GCC?

[lcwu1, 2014/10/07 21:30:17]

No, the bug was talking about removing Acquire_Store and Release_Load (not store
here) because the new C++ memory model doesn't define their behavior. (C++
memory model only defines the behavior of Release_Store and Acquire_Load, both
of which are most commonly used.)

Basically the upstream is working on replacing the implementation of
base/atomicops operations with C++11's new std::atomics so that the existing
calls to base/atomicops don't have to change. But our code would stop to work if
we don't upgrade to a new toolchain that supports C++11 atomics.

+    base::subtle::Release_Store(wr_offset_, 0);
   } else {
     size_ = desc->size;
     CHECK_LE(size_, max_size);
@@ -372,7 +372,6 @@ void MediaMessageFifo::CommitRead(size_t new_rd_offset) {
 
   // Make sure the read pointer has been updated before sending a notification.

[damienv1, 2014/10/07 19:22:31]

The comment should be updated (just to mention it's safe to send the
notification: the other thread will get the latest value for the read offset
through an Acquire_Load).

[lcwu1, 2014/10/07 21:30:17]

Done.

   if (!read_event_cb_.is_null()) {
-    base::subtle::MemoryBarrier();
     read_event_cb_.Run();
   }
 }
@@ -384,7 +383,6 @@ void MediaMessageFifo::CommitWrite(size_t new_wr_offset) {
 
   // Make sure the write pointer has been updated before sending a notification.

[damienv1, 2014/10/07 19:22:30]

Same: update the comment.
Also, could you please make sure that running TSAN on the unit test is still
fine.

[lcwu1, 2014/10/07 21:30:17]

Ran Tsan on cast_media_unittests and everything is fine. (To make tsan run, I
had to fix a couple of issues in ozone, which I will prepare another CL for.)

   if (!write_event_cb_.is_null()) {
-    base::subtle::MemoryBarrier();
     write_event_cb_.Run();
   }
 }