base/synchronization: Add more annotations to base::Lock.

This adds more annotations to base::Lock. This has ripple effects on the
codebase, namely code can now be checked, and is failing the static
checker. Depending on the case, either disable thread checking for the
function, or add the proper annotations.

Thread checking has to be disabled in cases where there is a
control-flow dependency on mutex locking. The most common case is where
the lock is a pointer, and can be nullptr.

While in the neighborhood, fix a few "git cl lint" warnings, mostly
"include what you use".

Bug: 831825
Change-Id: I66605f5ce1ab7ab35449dab6e120bef844ad9b57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036003
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749086}

base/debug/activity_tracker_unittest.cc

@@ -5,6 +5,7 @@
 #include "base/debug/activity_tracker.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -302,6 +303,7 @@ TEST_F(ActivityTrackerTest, LockTest) {
   // Check no activity when only "trying" a lock.
   EXPECT_TRUE(lock.Try());
   EXPECT_EQ(pre_version, tracker->GetDataVersionForTesting());
+  lock.AssertAcquired();
   lock.Release();
   EXPECT_EQ(pre_version, tracker->GetDataVersionForTesting());
 

base/metrics/persistent_histogram_allocator.cc

@@ -4,7 +4,8 @@
 
 #include "base/metrics/persistent_histogram_allocator.h"
 
-#include <memory>
+#include <limits>
+#include <utility>
 
 #include "base/atomicops.h"
 #include "base/files/file_path.h"
@@ -118,8 +119,6 @@ PersistentSparseHistogramDataManager::UseSampleMapRecords(uint64_t id,
 PersistentSampleMapRecords*
 PersistentSparseHistogramDataManager::GetSampleMapRecordsWhileLocked(
     uint64_t id) {
-  lock_.AssertAcquired();
-
   auto found = sample_records_.find(id);
   if (found != sample_records_.end())
     return found->second.get();

base/metrics/persistent_histogram_allocator.h

@@ -2,11 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef BASE_METRICS_HISTOGRAM_PERSISTENCE_H_
-#define BASE_METRICS_HISTOGRAM_PERSISTENCE_H_
+#ifndef BASE_METRICS_PERSISTENT_HISTOGRAM_ALLOCATOR_H_
+#define BASE_METRICS_PERSISTENT_HISTOGRAM_ALLOCATOR_H_
 
 #include <map>
 #include <memory>
+#include <string>
+#include <vector>
 
 #include "base/atomicops.h"
 #include "base/base_export.h"
@@ -64,9 +66,9 @@ class BASE_EXPORT PersistentSparseHistogramDataManager {
  private:
   friend class PersistentSampleMapRecords;
 
-  // Gets the object holding records for a given sample-map id when |lock_|
-  // has already been acquired.
-  PersistentSampleMapRecords* GetSampleMapRecordsWhileLocked(uint64_t id);
+  // Gets the object holding records for a given sample-map id.
+  PersistentSampleMapRecords* GetSampleMapRecordsWhileLocked(uint64_t id)
+      EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
   // Loads sample-map records looking for those belonging to the specified
   // |load_id|. Records found for other sample-maps are held for later use
@@ -80,13 +82,12 @@ class BASE_EXPORT PersistentSparseHistogramDataManager {
   PersistentMemoryAllocator* allocator_;
 
   // Iterator within the allocator for finding sample records.
-  PersistentMemoryAllocator::Iterator record_iterator_;
+  PersistentMemoryAllocator::Iterator record_iterator_ GUARDED_BY(lock_);
 
   // Mapping of sample-map IDs to their sample records.
   std::map<uint64_t, std::unique_ptr<PersistentSampleMapRecords>>
-      sample_records_;
+      sample_records_ GUARDED_BY(lock_);
 
-  // A lock used for synchronizing changes to sample_records_.
   base::Lock lock_;
 
   DISALLOW_COPY_AND_ASSIGN(PersistentSparseHistogramDataManager);
@@ -503,4 +504,4 @@ class BASE_EXPORT GlobalHistogramAllocator
 
 }  // namespace base
 
-#endif  // BASE_METRICS_HISTOGRAM_PERSISTENCE_H_
+#endif  // BASE_METRICS_PERSISTENT_HISTOGRAM_ALLOCATOR_H__

base/synchronization/lock.h

@@ -25,17 +25,14 @@ class LOCKABLE BASE_EXPORT Lock {
   Lock() : lock_() {}
   ~Lock() {}
 
-  // TODO(lukasza): https://crbug.com/831825: Add EXCLUSIVE_LOCK_FUNCTION
-  // annotation to Acquire method and similar annotations to Release and Try
-  // methods (here and in the #else branch).
-  void Acquire() { lock_.Lock(); }
-  void Release() { lock_.Unlock(); }
+  void Acquire() EXCLUSIVE_LOCK_FUNCTION() { lock_.Lock(); }
+  void Release() UNLOCK_FUNCTION() { lock_.Unlock(); }
 
   // If the lock is not held, take it and return true. If the lock is already
   // held by another thread, immediately return false. This must not be called
   // by a thread already holding the lock (what happens is undefined and an
   // assertion may fail).
-  bool Try() { return lock_.Try(); }
+  bool Try() EXCLUSIVE_TRYLOCK_FUNCTION(true) { return lock_.Try(); }
 
   // Null implementation if not debug.
   void AssertAcquired() const ASSERT_EXCLUSIVE_LOCK() {}
@@ -46,16 +43,16 @@ class LOCKABLE BASE_EXPORT Lock {
   // NOTE: We do not permit recursive locks and will commonly fire a DCHECK() if
   // a thread attempts to acquire the lock a second time (while already holding
   // it).
-  void Acquire() {
+  void Acquire() EXCLUSIVE_LOCK_FUNCTION() {
     lock_.Lock();
     CheckUnheldAndMark();
   }
-  void Release() {
+  void Release() UNLOCK_FUNCTION() {
     CheckHeldAndUnmark();
     lock_.Unlock();
   }
 
-  bool Try() {
+  bool Try() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
     bool rv = lock_.Try();
     if (rv) {
       CheckUnheldAndMark();

base/synchronization/lock_unittest.cc

@@ -97,8 +97,11 @@ class TryLockTestThread : public PlatformThread::Delegate {
   explicit TryLockTestThread(Lock* lock) : lock_(lock), got_lock_(false) {}
 
   void ThreadMain() override {
-    got_lock_ = lock_->Try();
-    if (got_lock_)
+    // The local variable is required for the static analyzer to see that the
+    // lock is properly released.
+    bool got_lock = lock_->Try();
+    got_lock_ = got_lock;
+    if (got_lock)
       lock_->Release();
   }
 
@@ -115,7 +118,7 @@ TEST(LockTest, TryLock) {
   Lock lock;
 
   ASSERT_TRUE(lock.Try());
-  // We now have the lock....
+  lock.AssertAcquired();
 
   // This thread will not be able to get the lock.
   {
@@ -143,6 +146,7 @@ TEST(LockTest, TryLock) {
     ASSERT_TRUE(thread.got_lock());
     // But it released it....
     ASSERT_TRUE(lock.Try());
+    lock.AssertAcquired();
   }
 
   lock.Release();
@@ -155,7 +159,7 @@ TEST(LockTest, TryTrackedLock) {
   Lock lock;
 
   ASSERT_TRUE(lock.Try());
-  // We now have the lock....
+  lock.AssertAcquired();
 
   // This thread will not be able to get the lock.
   {
@@ -183,6 +187,7 @@ TEST(LockTest, TryTrackedLock) {
     ASSERT_TRUE(thread.got_lock());
     // But it released it....
     ASSERT_TRUE(lock.Try());
+    lock.AssertAcquired();
   }
 
   lock.Release();

base/synchronization/waitable_event_mac.cc

@@ -48,7 +48,8 @@ void WaitableEvent::Reset() {
   PeekPort(receive_right_->Name(), true);
 }
 
-void WaitableEvent::Signal() {
+// NO_THREAD_SAFETY_ANALYSIS: Runtime dependent locking.
+void WaitableEvent::Signal() NO_THREAD_SAFETY_ANALYSIS {
   // If using the slow watch-list, copy the watchers to a local. After
   // mach_msg(), the event object may be deleted by an awoken thread.
   const bool use_slow_path = UseSlowWatchList(policy_);

base/synchronization/waitable_event_posix.cc

@@ -246,8 +246,9 @@ cmp_fst_addr(const std::pair<WaitableEvent*, unsigned> &a,
 }
 
 // static
+// NO_THREAD_SAFETY_ANALYSIS: Complex control flow.
 size_t WaitableEvent::WaitMany(WaitableEvent** raw_waitables,
-                               size_t count) {
+                               size_t count) NO_THREAD_SAFETY_ANALYSIS {
   DCHECK(count) << "Cannot wait on no events";
   internal::ScopedBlockingCallWithBaseSyncPrimitives scoped_blocking_call(
       FROM_HERE, BlockingType::MAY_BLOCK);
@@ -337,9 +338,10 @@ size_t WaitableEvent::WaitMany(WaitableEvent** raw_waitables,
 //   was signaled with the lowest input index from the original WaitMany call.
 // -----------------------------------------------------------------------------
 // static
+// NO_THREAD_SAFETY_ANALYSIS: Complex control flow.
 size_t WaitableEvent::EnqueueMany(std::pair<WaitableEvent*, size_t>* waitables,
                                   size_t count,
-                                  Waiter* waiter) {
+                                  Waiter* waiter) NO_THREAD_SAFETY_ANALYSIS {
   size_t winner = count;
   size_t winner_index = count;
   for (size_t i = 0; i < count; ++i) {

base/task/common/checked_lock_impl.h

@@ -32,8 +32,8 @@ class BASE_EXPORT CheckedLockImpl {
 
   static void AssertNoLockHeldOnCurrentThread();
 
-  void Acquire();
-  void Release();
+  void Acquire() EXCLUSIVE_LOCK_FUNCTION(lock_);
+  void Release() UNLOCK_FUNCTION(lock_);
 
   void AssertAcquired() const;
 

base/task/common/checked_lock_unittest.cc

@@ -121,7 +121,10 @@ TEST(CheckedLockTest, AcquirePredecessor) {
   predecessor.Release();
 }
 
-TEST(CheckedLockTest, AcquirePredecessorWrongOrder) {
+// Here and below, disable thread safety analysis, otherwise our death tests do
+// not compile (the issues are caught at compile time).
+TEST(CheckedLockTest, AcquirePredecessorWrongOrder)
+NO_THREAD_SAFETY_ANALYSIS {
   CheckedLock predecessor;
   CheckedLock lock(&predecessor);
   EXPECT_DCHECK_DEATH({
@@ -130,7 +133,7 @@ TEST(CheckedLockTest, AcquirePredecessorWrongOrder) {
   });
 }
 
-TEST(CheckedLockTest, AcquireNonPredecessor) {
+TEST(CheckedLockTest, AcquireNonPredecessor) NO_THREAD_SAFETY_ANALYSIS {
   CheckedLock lock1;
   CheckedLock lock2;
   EXPECT_DCHECK_DEATH({
@@ -161,7 +164,8 @@ TEST(CheckedLockTest, AcquireMultipleLocksInTheMiddleOfAChain) {
   lock2.Release();
 }
 
-TEST(CheckedLockTest, AcquireMultipleLocksNoTransitivity) {
+TEST(CheckedLockTest, AcquireMultipleLocksNoTransitivity)
+NO_THREAD_SAFETY_ANALYSIS {
   CheckedLock lock1;
   CheckedLock lock2(&lock1);
   CheckedLock lock3(&lock2);
@@ -299,7 +303,8 @@ TEST(CheckedLockTest, AcquireLockAfterUniversalPredecessor) {
   universal_predecessor.Release();
 }
 
-TEST(CheckedLockTest, AcquireMultipleLocksAfterUniversalPredecessor) {
+TEST(CheckedLockTest, AcquireMultipleLocksAfterUniversalPredecessor)
+NO_THREAD_SAFETY_ANALYSIS {
   // Acquisition of a universal-predecessor lock does not affect acquisition
   // rules for locks beyond the one acquired directly after it.
   CheckedLock universal_predecessor((UniversalPredecessor()));
@@ -321,7 +326,8 @@ TEST(CheckedLockTest, AcquireMultipleLocksAfterUniversalPredecessor) {
   });
 }
 
-TEST(CheckedLockTest, AcquireUniversalPredecessorAfterLock) {
+TEST(CheckedLockTest, AcquireUniversalPredecessorAfterLock)
+NO_THREAD_SAFETY_ANALYSIS {
   // A universal-predecessor lock may not be acquired after any other lock.
   CheckedLock universal_predecessor((UniversalPredecessor()));
   CheckedLock lock;
@@ -332,7 +338,8 @@ TEST(CheckedLockTest, AcquireUniversalPredecessorAfterLock) {
   });
 }
 
-TEST(CheckedLockTest, AcquireUniversalPredecessorAfterUniversalPredecessor) {
+TEST(CheckedLockTest, AcquireUniversalPredecessorAfterUniversalPredecessor)
+NO_THREAD_SAFETY_ANALYSIS {
   // A universal-predecessor lock may not be acquired after any other lock, not
   // even another universal predecessor.
   CheckedLock universal_predecessor((UniversalPredecessor()));

base/trace_event/trace_log.cc

@@ -6,7 +6,9 @@
 
 #include <algorithm>
 #include <cmath>
+#include <limits>
 #include <memory>
+#include <unordered_set>
 #include <utility>
 
 #include "base/base_switches.h"
@@ -189,10 +191,12 @@ class TraceLog::OptionalAutoLock {
       lock_->Release();
   }
 
-  void EnsureAcquired() {
+  void EnsureAcquired() EXCLUSIVE_LOCK_FUNCTION(lock_) {
     if (!locked_) {
       lock_->Acquire();
       locked_ = true;
+    } else {
+      lock_->AssertAcquired();
     }
   }
 
@@ -1220,7 +1224,7 @@ TraceEventHandle TraceLog::AddTraceEventWithThreadIdAndTimestamp(
     int thread_id,
     const TimeTicks& timestamp,
     TraceArguments* args,
-    unsigned int flags) {
+    unsigned int flags) NO_THREAD_SAFETY_ANALYSIS {
   TraceEventHandle handle = {0, 0, 0};
   if (!ShouldAddAfterUpdatingState(phase, category_group_enabled, name, id,
                                    thread_id, args)) {
@@ -1297,6 +1301,7 @@ TraceEventHandle TraceLog::AddTraceEventWithThreadIdAndTimestamp(
       trace_event = AddEventToThreadSharedChunkWhileLocked(&handle, true);
     }
 
+    // NO_THREAD_SAFETY_ANALYSIS: Conditional locking above.
     if (trace_event) {
       if (filtered_trace_event) {
         *trace_event = std::move(*filtered_trace_event);
@@ -1603,7 +1608,8 @@ TraceEvent* TraceLog::GetEventByHandle(TraceEventHandle handle) {
 }
 
 TraceEvent* TraceLog::GetEventByHandleInternal(TraceEventHandle handle,
-                                               OptionalAutoLock* lock) {
+                                               OptionalAutoLock* lock)
+    NO_THREAD_SAFETY_ANALYSIS {
   if (!handle.chunk_seq)
     return nullptr;
 
@@ -1620,6 +1626,7 @@ TraceEvent* TraceLog::GetEventByHandleInternal(TraceEventHandle handle,
 
   // The event has been out-of-control of the thread local buffer.
   // Try to get the event from the main buffer with a lock.
+  // NO_THREAD_SAFETY_ANALYSIS: runtime-dependent locking here.
   if (lock)
     lock->EnsureAcquired();
 

cc/layers/video_frame_provider_client_impl.cc

@@ -85,14 +85,12 @@ VideoFrameProviderClientImpl::AcquireLockAndCurrentFrame() {
 
 void VideoFrameProviderClientImpl::PutCurrentFrame() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  provider_lock_.AssertAcquired();
   provider_->PutCurrentFrame();
   needs_put_current_frame_ = false;
 }
 
 void VideoFrameProviderClientImpl::ReleaseLock() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  provider_lock_.AssertAcquired();
   provider_lock_.Release();
 }
 

cc/layers/video_frame_provider_client_impl.h

@@ -43,9 +43,13 @@ class CC_EXPORT VideoFrameProviderClientImpl
   // Must be called on the impl thread while the main thread is blocked.
   void Stop();
 
-  scoped_refptr<media::VideoFrame> AcquireLockAndCurrentFrame();
-  void PutCurrentFrame();
-  void ReleaseLock();
+  scoped_refptr<media::VideoFrame> AcquireLockAndCurrentFrame()
+      EXCLUSIVE_LOCK_FUNCTION(provider_lock_);
+  void PutCurrentFrame() EXCLUSIVE_LOCKS_REQUIRED(provider_lock_);
+  void ReleaseLock() UNLOCK_FUNCTION(provider_lock_);
+  void AssertLocked() const ASSERT_EXCLUSIVE_LOCK(provider_lock_) {
+    provider_lock_.AssertAcquired();
+  }
   bool HasCurrentFrame();
 
   // VideoFrameController implementation.

cc/layers/video_layer_impl.cc

@@ -6,6 +6,9 @@
 
 #include <stddef.h>
 
+#include <memory>
+#include <utility>
+
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ -78,7 +81,8 @@ void VideoLayerImpl::DidBecomeActive() {
 }
 
 bool VideoLayerImpl::WillDraw(DrawMode draw_mode,
-                              viz::ClientResourceProvider* resource_provider) {
+                              viz::ClientResourceProvider* resource_provider)
+    NO_THREAD_SAFETY_ANALYSIS {
   if (draw_mode == DRAW_MODE_RESOURCELESS_SOFTWARE)
     return false;
 
@@ -98,6 +102,7 @@ bool VideoLayerImpl::WillDraw(DrawMode draw_mode,
     // Drop any resources used by the updater if there is no frame to display.
     updater_ = nullptr;
 
+    // NO_THREAD_SAFETY_ANALYSIS: Releasing the lock in some return paths only.
     provider_client_impl_->ReleaseLock();
     return false;
   }
@@ -165,6 +170,7 @@ void VideoLayerImpl::AppendQuads(viz::RenderPass* render_pass,
 }
 
 void VideoLayerImpl::DidDraw(viz::ClientResourceProvider* resource_provider) {
+  provider_client_impl_->AssertLocked();
   LayerImpl::DidDraw(resource_provider);
 
   DCHECK(frame_.get());

cc/tiles/gpu_image_decode_cache.cc

@@ -1276,7 +1276,7 @@ void GpuImageDecodeCache::DrawWithImageFinished(
   RunPendingContextThreadOperations();
 }
 
-void GpuImageDecodeCache::ReduceCacheUsage() {
+void GpuImageDecodeCache::ReduceCacheUsage() NO_THREAD_SAFETY_ANALYSIS {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "GpuImageDecodeCache::ReduceCacheUsage");
   base::AutoLock lock(lock_);
@@ -1285,6 +1285,8 @@ void GpuImageDecodeCache::ReduceCacheUsage() {
   // This is typically called when no tasks are running (between scheduling
   // tasks). Try to lock and run pending operations if possible, but don't
   // block on it.
+  //
+  // NO_THREAD_SAFETY_ANALYSIS: runtime-dependent locking.
   if (context_->GetLock() && !context_->GetLock()->Try())
     return;
 

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -555,7 +555,8 @@ base::Lock& GetLockForBlockingDefaultFileTaskRunner() {
 
 // Ensures the default HTML filesystem API blocking task runner is blocked for a
 // test.
-void BlockFileTaskRunner(Profile* profile) {
+void BlockFileTaskRunner(Profile* profile)
+    EXCLUSIVE_LOCK_FUNCTION(GetLockForBlockingDefaultFileTaskRunner()) {
   GetLockForBlockingDefaultFileTaskRunner().Acquire();
 
   content::BrowserContext::GetDefaultStoragePartition(profile)
@@ -567,7 +568,8 @@ void BlockFileTaskRunner(Profile* profile) {
 }
 
 // Undo the effects of |BlockFileTaskRunner()|.
-void UnblockFileTaskRunner() {
+void UnblockFileTaskRunner()
+    UNLOCK_FUNCTION(GetLockForBlockingDefaultFileTaskRunner()) {
   GetLockForBlockingDefaultFileTaskRunner().Release();
 }
 
@@ -1633,9 +1635,12 @@ void FileManagerBrowserTestBase::RunTestMessageLoop() {
   }
 }
 
+// NO_THREAD_SAFETY_ANALYSIS: Locking depends on runtime commands, the static
+// checker cannot assess it.
 void FileManagerBrowserTestBase::OnCommand(const std::string& name,
                                            const base::DictionaryValue& value,
-                                           std::string* output) {
+                                           std::string* output)
+    NO_THREAD_SAFETY_ANALYSIS {
   base::ScopedAllowBlockingForTesting allow_blocking;
 
   if (name == "isInGuestMode") {

chromeos/components/nearby/lock_impl.cc

@@ -18,7 +18,9 @@ LockImpl::~LockImpl() {
 #endif
 }
 
-void LockImpl::lock() {
+// NO_THREAD_SAFETY_ANALYSIS: Whether this locks or not depends on rumtime
+// properties.
+void LockImpl::lock() NO_THREAD_SAFETY_ANALYSIS {
   {
     base::AutoLock al(bookkeeping_lock_);
     if (num_acquisitions_ > 0u &&
@@ -46,7 +48,9 @@ void LockImpl::lock() {
   }
 }
 
-void LockImpl::unlock() {
+// NO_THREAD_SAFETY_ANALYSIS: Whether this unlocks or not depends on rumtime
+// properties.
+void LockImpl::unlock() NO_THREAD_SAFETY_ANALYSIS {
   base::AutoLock al(bookkeeping_lock_);
   CHECK_GT(num_acquisitions_, 0u);
   DCHECK_EQ(base::PlatformThread::CurrentId(), owning_thread_id_);

chromeos/components/nearby/lock_impl.h

@@ -14,7 +14,7 @@ namespace chromeos {
 
 namespace nearby {
 
-// Concrete location::nearby::Lock implementation.
+// Concrete location::nearby::Lock implementation. Recursive lock.
 class LockImpl : public LockBase {
  public:
   LockImpl();

components/sync/syncable/syncable_base_transaction.cc

@@ -19,14 +19,16 @@ Directory* BaseTransaction::directory() const {
   return directory_;
 }
 
-void BaseTransaction::Lock() {
+void BaseTransaction::Lock()
+    EXCLUSIVE_LOCK_FUNCTION(directory_->kernel()->transaction_mutex) {
   TRACE_EVENT2("sync_lock_contention", "AcquireLock", "src_file",
                from_here_.file_name(), "src_func", from_here_.function_name());
 
   directory_->kernel()->transaction_mutex.Acquire();
 }
 
-void BaseTransaction::Unlock() {
+void BaseTransaction::Unlock()
+    UNLOCK_FUNCTION(directory_->kernel()->transaction_mutex) {
   directory_->kernel()->transaction_mutex.Release();
 }
 

components/viz/common/gpu/context_cache_controller.cc

@@ -5,6 +5,7 @@
 #include "components/viz/common/gpu/context_cache_controller.h"
 
 #include <chrono>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -161,7 +162,8 @@ void ContextCacheController::InvalidatePendingIdleCallbacks() {
   ++current_idle_generation_;
 }
 
-void ContextCacheController::OnIdle(uint32_t idle_generation) {
+void ContextCacheController::OnIdle(uint32_t idle_generation)
+    NO_THREAD_SAFETY_ANALYSIS {
   // First check if we should run our idle callback at all. If we have become
   // busy since scheduling, just schedule another idle callback and return.
   {
@@ -175,6 +177,8 @@ void ContextCacheController::OnIdle(uint32_t idle_generation) {
   // Try to acquire the context lock - if we can't acquire it then we've become
   // busy since checking |current_idle_generation_| above. In this case, just
   // re-post our idle callback and return.
+  //
+  // NO_THREAD_SAFETY_ANALYSIS: Locking depends on runtime properties.
   if (context_lock_ && !context_lock_->Try()) {
     base::AutoLock hold(current_idle_generation_lock_);
     PostIdleCallback(current_idle_generation_);

content/browser/renderer_host/text_input_client_mac.h

@@ -121,10 +121,10 @@ class CONTENT_EXPORT TextInputClientMac {
   // These methods lock the internal condition for use before the asynchronous
   // message is sent to the renderer to lookup the required information. These
   // are only used on the UI thread.
-  void BeforeRequest();
+  void BeforeRequest() EXCLUSIVE_LOCK_FUNCTION(lock_);
   // Called at the end of a critical section. This will release the lock and
   // condition.
-  void AfterRequest();
+  void AfterRequest() UNLOCK_FUNCTION(lock_);
 
   uint32_t character_index_;
   gfx::Rect first_rect_;

content/utility/in_process_utility_thread.cc

@@ -37,7 +37,8 @@ void InProcessUtilityThread::Init() {
                                 base::Unretained(this)));
 }
 
-void InProcessUtilityThread::CleanUp() {
+void InProcessUtilityThread::CleanUp()
+    UNLOCK_FUNCTION(g_one_utility_thread_lock.Get()) {
   child_process_.reset();
 
   // See comment in RendererMainThread.
@@ -45,7 +46,8 @@ void InProcessUtilityThread::CleanUp() {
   g_one_utility_thread_lock.Get().Release();
 }
 
-void InProcessUtilityThread::InitInternal() {
+void InProcessUtilityThread::InitInternal()
+    EXCLUSIVE_LOCK_FUNCTION(g_one_utility_thread_lock.Get()) {
   g_one_utility_thread_lock.Get().Acquire();
   child_process_.reset(new ChildProcess());
   child_process_->set_main_thread(new UtilityThreadImpl(params_));

gpu/ipc/client/command_buffer_proxy_impl.cc

@@ -5,8 +5,6 @@
 #include "gpu/ipc/client/command_buffer_proxy_impl.h"
 
 #include <memory>
-#include <utility>
-#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ -658,7 +656,6 @@ void CommandBufferProxyImpl::ReturnFrontBuffer(const gpu::Mailbox& mailbox,
 
 bool CommandBufferProxyImpl::Send(IPC::Message* msg) {
   DCHECK(channel_);
-  last_state_lock_.AssertAcquired();
   DCHECK_EQ(gpu::error::kNoError, last_state_.error);
 
   last_state_lock_.Release();
@@ -714,7 +711,6 @@ CommandBufferProxyImpl::AllocateAndMapSharedMemory(size_t size) {
 void CommandBufferProxyImpl::SetStateFromMessageReply(
     const gpu::CommandBuffer::State& state) {
   CheckLock();
-  last_state_lock_.AssertAcquired();
   if (last_state_.error != gpu::error::kNoError)
     return;
   // Handle wraparound. It works as long as we don't have more than 2B state
@@ -727,7 +723,6 @@ void CommandBufferProxyImpl::SetStateFromMessageReply(
 
 void CommandBufferProxyImpl::TryUpdateState() {
   CheckLock();
-  last_state_lock_.AssertAcquired();
   if (last_state_.error == gpu::error::kNoError) {
     shared_state()->Read(&last_state_);
     if (last_state_.error != gpu::error::kNoError)
@@ -736,7 +731,6 @@ void CommandBufferProxyImpl::TryUpdateState() {
 }
 
 void CommandBufferProxyImpl::TryUpdateStateThreadSafe() {
-  last_state_lock_.AssertAcquired();
   if (last_state_.error == gpu::error::kNoError) {
     shared_state()->Read(&last_state_);
     if (last_state_.error != gpu::error::kNoError) {
@@ -749,7 +743,6 @@ void CommandBufferProxyImpl::TryUpdateStateThreadSafe() {
 }
 
 void CommandBufferProxyImpl::TryUpdateStateDontReportError() {
-  last_state_lock_.AssertAcquired();
   if (last_state_.error == gpu::error::kNoError)
     shared_state()->Read(&last_state_);
 }
@@ -779,7 +772,6 @@ void CommandBufferProxyImpl::OnBufferPresented(
 
 void CommandBufferProxyImpl::OnGpuSyncReplyError() {
   CheckLock();
-  last_state_lock_.AssertAcquired();
   last_state_.error = gpu::error::kLostContext;
   last_state_.context_lost_reason = gpu::error::kInvalidGpuMessage;
   // This method may be inside a callstack from the GpuControlClient (we got a
@@ -792,7 +784,6 @@ void CommandBufferProxyImpl::OnGpuAsyncMessageError(
     gpu::error::ContextLostReason reason,
     gpu::error::Error error) {
   CheckLock();
-  last_state_lock_.AssertAcquired();
   last_state_.error = error;
   last_state_.context_lost_reason = reason;
   // This method only occurs when receiving IPC messages, so we know it's not in
@@ -804,7 +795,6 @@ void CommandBufferProxyImpl::OnGpuAsyncMessageError(
 
 void CommandBufferProxyImpl::OnGpuStateError() {
   CheckLock();
-  last_state_lock_.AssertAcquired();
   DCHECK_NE(gpu::error::kNoError, last_state_.error);
   // This method may be inside a callstack from the GpuControlClient (we
   // encountered an error while trying to perform some action). So avoid
@@ -814,7 +804,6 @@ void CommandBufferProxyImpl::OnGpuStateError() {
 
 void CommandBufferProxyImpl::OnClientError(gpu::error::Error error) {
   CheckLock();
-  last_state_lock_.AssertAcquired();
   last_state_.error = error;
   last_state_.context_lost_reason = gpu::error::kUnknown;
   // This method may be inside a callstack from the GpuControlClient (we
@@ -825,7 +814,6 @@ void CommandBufferProxyImpl::OnClientError(gpu::error::Error error) {
 
 void CommandBufferProxyImpl::DisconnectChannelInFreshCallStack() {
   CheckLock();
-  last_state_lock_.AssertAcquired();
   // Inform the GpuControlClient of the lost state immediately, though this may
   // be a re-entrant call to the client so we use the MaybeReentrant variant.
   if (gpu_control_client_)

gpu/ipc/client/command_buffer_proxy_impl.h

@@ -13,6 +13,8 @@
 #include <queue>
 #include <string>
 #include <unordered_map>
+#include <utility>
+#include <vector>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
@@ -169,7 +171,7 @@ class GPU_EXPORT CommandBufferProxyImpl : public gpu::CommandBuffer,
   // Send an IPC message over the GPU channel. This is private to fully
   // encapsulate the channel; all callers of this function must explicitly
   // verify that the context has not been lost.
-  bool Send(IPC::Message* msg);
+  bool Send(IPC::Message* msg) EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
 
   std::pair<base::UnsafeSharedMemoryRegion, base::WritableSharedMemoryMapping>
   AllocateAndMapSharedMemory(size_t size);
@@ -189,33 +191,38 @@ class GPU_EXPORT CommandBufferProxyImpl : public gpu::CommandBuffer,
 
   // Try to read an updated copy of the state from shared memory, and calls
   // OnGpuStateError() if the new state has an error.
-  void TryUpdateState();
+  void TryUpdateState() EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
   // Like above but calls the error handler and disconnects channel by posting
   // a task.
-  void TryUpdateStateThreadSafe();
+  void TryUpdateStateThreadSafe() EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
   // Like the above but does not call the error event handler if the new state
   // has an error.
-  void TryUpdateStateDontReportError();
+  void TryUpdateStateDontReportError()
+      EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
   // Sets the state, and calls OnGpuStateError() if the new state has an error.
-  void SetStateFromMessageReply(const CommandBuffer::State& state);
+  void SetStateFromMessageReply(const CommandBuffer::State& state)
+      EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
 
   // Loses the context after we received an invalid reply from the GPU
   // process.
-  void OnGpuSyncReplyError();
+  void OnGpuSyncReplyError() EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
 
   // Loses the context when receiving a message from the GPU process.
   void OnGpuAsyncMessageError(gpu::error::ContextLostReason reason,
-                              gpu::error::Error error);
+                              gpu::error::Error error)
+      EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
 
   // Loses the context after we receive an error state from the GPU process.
-  void OnGpuStateError();
+  void OnGpuStateError() EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
 
   // Sets an error on the last_state_ and loses the context due to client-side
   // errors.
-  void OnClientError(gpu::error::Error error);
+  void OnClientError(gpu::error::Error error)
+      EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
 
   // Helper methods, don't call these directly.
-  void DisconnectChannelInFreshCallStack();
+  void DisconnectChannelInFreshCallStack()
+      EXCLUSIVE_LOCKS_REQUIRED(last_state_lock_);
   void LockAndDisconnectChannel();
   void DisconnectChannel();
 

net/proxy_resolution/proxy_resolver_mac.cc

@@ -149,13 +149,15 @@ void SynchronizedRunLoopObserver::RemoveFromCurrentRunLoop(
 
 void SynchronizedRunLoopObserver::RunLoopObserverCallBack(
     CFRunLoopObserverRef observer,
-    CFRunLoopActivity activity) {
+    CFRunLoopActivity activity) NO_THREAD_SAFETY_ANALYSIS {
   DCHECK(thread_checker_.CalledOnValidThread());
   // Acquire the lock when a source has been signaled and going to be fired.
   // In the context of the proxy resolver that happens when the proxy for a
   // given URL has been resolved and the callback function that handles the
   // result is going to be fired.
   // Release the lock when all source events have been handled.
+  //
+  // NO_THREAD_SAFETY_ANALYSIS: Runtime dependent locking.
   switch (activity) {
     case kCFRunLoopBeforeSources:
       if (!lock_acquired_) {

ppapi/shared_impl/proxy_lock.cc

@@ -35,7 +35,8 @@ base::Lock* ProxyLock::Get() {
 // locking).
 
 // static
-void ProxyLock::Acquire() {
+void ProxyLock::Acquire() NO_THREAD_SAFETY_ANALYSIS {
+  // NO_THREAD_SAFETY_ANALYSIS: Runtime dependent locking.
   base::Lock* lock = Get();
   if (lock) {
     // This thread does not already hold the lock.
@@ -48,7 +49,8 @@ void ProxyLock::Acquire() {
 }
 
 // static
-void ProxyLock::Release() {
+void ProxyLock::Release() NO_THREAD_SAFETY_ANALYSIS {
+  // NO_THREAD_SAFETY_ANALYSIS: Runtime dependent locking.
   base::Lock* lock = Get();
   if (lock) {
     // This thread currently holds the lock.

third_party/blink/renderer/platform/webrtc/peer_connection_remote_audio_source.cc

@@ -4,6 +4,9 @@
 
 #include "third_party/blink/renderer/platform/webrtc/peer_connection_remote_audio_source.h"
 
+#include <string>
+#include <utility>
+
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
@@ -161,7 +164,8 @@ void PeerConnectionRemoteAudioSource::OnData(const void* audio_data,
   MediaStreamAudioSource::DeliverDataToTracks(*audio_bus_, playout_time);
 
 #ifndef NDEBUG
-  single_audio_thread_guard_.Release();
+  if (is_only_thread_here)
+    single_audio_thread_guard_.Release();
 #endif
 }