tc-malloc:
  third_party/tcmalloc/chromium/src/base/low_level_alloc.cc : the only annotation here is static int Random(). A racy random number is still random enough :)
  third_party/tcmalloc/vendor/src/base/low_level_alloc.cc : identical code. THis code is not built in chrome, we keep as a reference to diff against the forked copy of tcmalloc.
  Personally no concern about removing the annotation.

trace_event:
defer base/trace_event/trace_event_synthetic_delay.cc to skyostil@ who is already cc here.

> A racy random number is still random enough

You underestimate effects of data races. Please read this:
https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong
Language standard clearly states that behavior of a program with data races is undefined.

Re #2. Hmm if I can be honest that article isn't really too pragmatic (I've seen more convincing examples on the topic than "launch_nuclear_missile()").
Regardless, really, I fully buy the argument that data races can cause very subtle effect that are extremely hard to reason about.

That article makes a great point about using std::atomic, and I am fully onboard with that.
I would love to use std::atomic and std::memory_order_relaxed, but I have a very realistic and pragmatic problem with it. 
Unfortunately, as I found out at my own expense in  Issue 593344 , today std::memory_order_relaxed causes a double mfence to be emitted for no good reason (tracking bug  Issue 592903 ).
I am quite reluctant to introduce two mfence in an allocator, especially considering that:
- that code has been there for years and is hit, at the peak, 400 K times / second *  several hundreds million users (so, realistically, if there was a bug today, that would have popped up)
- Those extra mfences in the allocator were the cause a performance regression that showed up in our perf waterfall (which is how I opened that can of worms). 

I will be extremely happy to switch to std::atomic std::memory_order_relaxed, once our toolchain will emit the right assembly.

Chromium has base::subtle::NoBarrier_Load/Store for this.

Yup, I know. Unfortunately, I might be misreading the code, but that doesn't seem to be any precedent for third_party/tcmalloc to use chromium's base (unfortunately, tcmalloc has its own base/)

We also have NoBarrier operations in third_party/tcmalloc/chromium/src/base/atomicops.h, however it doesn't seem to support TSan.

"static" strings in blink will have their reference counters written to, but the value is never used for any meaningful logic. I think this is the reason why their "benign" race was considered ok in the past. Reading up on this, it seems like it is not the case and it is a real bug due to things like storage reuse indicated on the linked article.

Strings in Blink are very performance sensitive. I think our only alternative is to ensure static strings do not do any ref counting whatsoever, without resorting to adding additional conditional branches in StringImpl. This probably requires templating StringImpl. +esprehn.

This may have additional benefits if static strings are used in perf sensitive places that aggressively ref bump. Plus, if ref/deref are inlined in many places we could see a binary size reduction.

I don't think we can templatize the StringImpl and keep the generality that String and static strings are the same thing in the code which is important for the ergonomics of it. I read those papers and I'm aware of many of the issues, but the ref count on a static string is basically garbage on purpose so we don't really care what the compiler is doing with that value in memory.

If someone has a way to make the binary literally the same number of bytes and the same instructions inside StringImpl and the usage of Strings look literally identical then we can change the code of course.

In the absence of a bug where someone can point to this actually being a problem I don't want to chase a ghost. The code has been this way and produced correct code for many years. :)

Un-cc-ing me from all bugs on my final day.