Thread annotation of rtc::CriticalSection.

Effectively re-lands r5516 which was reverted because talk/-only
checkouts existed. This now resides in webrtc/base/, so no talk/-only
checkouts should be possible.

This change also enables -Wthread-safety for talk/ and fixes a bug in
talk/media/webrtc/webrtcvideoengine2.cc where a guarded variable was
read without taking the corresponding lock.

R=andresp@webrtc.org, mflodman@webrtc.org, pthatcher@webrtc.org
BUG=

Review URL: https://webrtc-codereview.appspot.com/27569004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7284 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/build/common.gypi

@@ -91,6 +91,7 @@
               # LateBindingSymbolTable::TableInfo from
               # latebindingsymboltable.cc.def and remove below flag.
               '-Wno-address-of-array-temporary',
+              '-Wthread-safety',
             ],
           }],
         ],

talk/media/webrtc/webrtcvideoengine2.cc

@@ -1381,14 +1381,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::InputFrame(
                   << frame->GetHeight();
   // Lock before copying, can be called concurrently when swapping input source.
   rtc::CritScope frame_cs(&frame_lock_);
-  if (!muted_) {
+  ConvertToI420VideoFrame(*frame, &video_frame_);
-    ConvertToI420VideoFrame(*frame, &video_frame_);
+
-  } else {
-    // Create a black frame to transmit instead.
-    CreateBlackFrame(&video_frame_,
-                     static_cast<int>(frame->GetWidth()),
-                     static_cast<int>(frame->GetHeight()));
-  }
   rtc::CritScope cs(&lock_);
   if (stream_ == NULL) {
     LOG(LS_WARNING) << "Capturer inputting frames before send codecs are "
@@ -1400,6 +1394,12 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::InputFrame(
     LOG(LS_VERBOSE) << "VideoFormat 0x0 set, Dropping frame.";
     return;
   }
+  if (muted_) {
+    // Create a black frame to transmit instead.
+    CreateBlackFrame(&video_frame_,
+                     static_cast<int>(frame->GetWidth()),
+                     static_cast<int>(frame->GetHeight()));
+  }
   // Reconfigure codec if necessary.
   SetDimensions(
       video_frame_.width(), video_frame_.height(), capturer->IsScreencast());

talk/media/webrtc/webrtcvideoengine2.h

@@ -335,10 +335,12 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
     };
 
     void SetCodecAndOptions(const VideoCodecSettings& codec,
-                            const VideoOptions& options);
+                            const VideoOptions& options)
-    void RecreateWebRtcStream();
+        EXCLUSIVE_LOCKS_REQUIRED(lock_);
+    void RecreateWebRtcStream() EXCLUSIVE_LOCKS_REQUIRED(lock_);
     // When |override_max| is false constrain width/height to codec dimensions.
-    void SetDimensions(int width, int height, bool override_max);
+    void SetDimensions(int width, int height, bool override_max)
+        EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
     webrtc::Call* const call_;
     WebRtcVideoEncoderFactory2* const encoder_factory_;

talk/session/media/mediamonitor.h

@@ -34,6 +34,7 @@
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/sigslot.h"
 #include "webrtc/base/thread.h"
+#include "webrtc/base/thread_annotations.h"
 
 namespace cricket {
 
@@ -77,7 +78,7 @@ class MediaMonitorT : public MediaMonitor {
     media_info_.Clear();
     media_channel_->GetStats(&media_info_);
   }
-  virtual void Update() {
+  virtual void Update() EXCLUSIVE_LOCKS_REQUIRED(crit_) {
     MI stats(media_info_);
     crit_.Leave();
     SignalUpdate(media_channel_, stats);

webrtc/base/criticalsection.h

@@ -12,6 +12,7 @@
 #define WEBRTC_BASE_CRITICALSECTION_H__
 
 #include "webrtc/base/constructormagic.h"
+#include "webrtc/base/thread_annotations.h"
 
 #if defined(WEBRTC_WIN)
 #include "webrtc/base/win32.h"
@@ -34,7 +35,7 @@
 namespace rtc {
 
 #if defined(WEBRTC_WIN)
-class CriticalSection {
+class LOCKABLE CriticalSection {
  public:
   CriticalSection() {
     InitializeCriticalSection(&crit_);
@@ -44,18 +45,18 @@ class CriticalSection {
   ~CriticalSection() {
     DeleteCriticalSection(&crit_);
   }
-  void Enter() {
+  void Enter() EXCLUSIVE_LOCK_FUNCTION() {
     EnterCriticalSection(&crit_);
     TRACK_OWNER(thread_ = GetCurrentThreadId());
   }
-  bool TryEnter() {
+  bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
     if (TryEnterCriticalSection(&crit_) != FALSE) {
       TRACK_OWNER(thread_ = GetCurrentThreadId());
       return true;
     }
     return false;
   }
-  void Leave() {
+  void Leave() UNLOCK_FUNCTION() {
     TRACK_OWNER(thread_ = 0);
     LeaveCriticalSection(&crit_);
   }
@@ -71,7 +72,7 @@ class CriticalSection {
 #endif // WEBRTC_WIN 
 
 #if defined(WEBRTC_POSIX)
-class CriticalSection {
+class LOCKABLE CriticalSection {
  public:
   CriticalSection() {
     pthread_mutexattr_t mutex_attribute;
@@ -84,18 +85,18 @@ class CriticalSection {
   ~CriticalSection() {
     pthread_mutex_destroy(&mutex_);
   }
-  void Enter() {
+  void Enter() EXCLUSIVE_LOCK_FUNCTION() {
     pthread_mutex_lock(&mutex_);
     TRACK_OWNER(thread_ = pthread_self());
   }
-  bool TryEnter() {
+  bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
     if (pthread_mutex_trylock(&mutex_) == 0) {
       TRACK_OWNER(thread_ = pthread_self());
       return true;
     }
     return false;
   }
-  void Leave() {
+  void Leave() UNLOCK_FUNCTION() {
     TRACK_OWNER(thread_ = 0);
     pthread_mutex_unlock(&mutex_);
   }
@@ -111,13 +112,13 @@ class CriticalSection {
 #endif // WEBRTC_POSIX
 
 // CritScope, for serializing execution through a scope.
-class CritScope {
+class SCOPED_LOCKABLE CritScope {
  public:
-  explicit CritScope(CriticalSection *pcrit) {
+  explicit CritScope(CriticalSection *pcrit) EXCLUSIVE_LOCK_FUNCTION(pcrit) {
     pcrit_ = pcrit;
     pcrit_->Enter();
   }
-  ~CritScope() {
+  ~CritScope() UNLOCK_FUNCTION() {
     pcrit_->Leave();
   }
  private:

webrtc/base/sharedexclusivelock.h

@@ -19,14 +19,14 @@ namespace rtc {
 
 // This class provides shared-exclusive lock. It can be used in cases like
 // multiple-readers/single-writer model.
-class SharedExclusiveLock {
+class LOCKABLE SharedExclusiveLock {
  public:
   SharedExclusiveLock();
 
   // Locking/unlocking methods. It is encouraged to use SharedScope or
   // ExclusiveScope for protection.
-  void LockExclusive();
+  void LockExclusive() EXCLUSIVE_LOCK_FUNCTION();
-  void UnlockExclusive();
+  void UnlockExclusive() UNLOCK_FUNCTION();
   void LockShared();
   void UnlockShared();
 
@@ -39,15 +39,14 @@ class SharedExclusiveLock {
   DISALLOW_COPY_AND_ASSIGN(SharedExclusiveLock);
 };
 
-class SharedScope {
+class SCOPED_LOCKABLE SharedScope {
  public:
-  explicit SharedScope(SharedExclusiveLock* lock) : lock_(lock) {
+  explicit SharedScope(SharedExclusiveLock* lock) SHARED_LOCK_FUNCTION(lock)
+      : lock_(lock) {
     lock_->LockShared();
   }
 
-  ~SharedScope() {
+  ~SharedScope() UNLOCK_FUNCTION() { lock_->UnlockShared(); }
-    lock_->UnlockShared();
-  }
 
  private:
   SharedExclusiveLock* lock_;
@@ -55,15 +54,15 @@ class SharedScope {
   DISALLOW_COPY_AND_ASSIGN(SharedScope);
 };
 
-class ExclusiveScope {
+class SCOPED_LOCKABLE ExclusiveScope {
  public:
-  explicit ExclusiveScope(SharedExclusiveLock* lock) : lock_(lock) {
+  explicit ExclusiveScope(SharedExclusiveLock* lock)
+      EXCLUSIVE_LOCK_FUNCTION(lock)
+      : lock_(lock) {
     lock_->LockExclusive();
   }
 
-  ~ExclusiveScope() {
+  ~ExclusiveScope() UNLOCK_FUNCTION() { lock_->UnlockExclusive(); }
-    lock_->UnlockExclusive();
-  }
 
  private:
   SharedExclusiveLock* lock_;

webrtc/base/signalthread.h

@@ -115,16 +115,17 @@ class SignalThread
     DISALLOW_IMPLICIT_CONSTRUCTORS(Worker);
   };
 
-  class EnterExit {
+  class SCOPED_LOCKABLE EnterExit {
    public:
-    explicit EnterExit(SignalThread* t) : t_(t) {
+    explicit EnterExit(SignalThread* t) EXCLUSIVE_LOCK_FUNCTION(t->cs_)
+        : t_(t) {
       t_->cs_.Enter();
       // If refcount_ is zero then the object has already been deleted and we
       // will be double-deleting it in ~EnterExit()! (shouldn't happen)
       ASSERT(t_->refcount_ != 0);
       ++t_->refcount_;
     }
-    ~EnterExit() {
+    ~EnterExit() UNLOCK_FUNCTION() {
       bool d = (0 == --t_->refcount_);
       t_->cs_.Leave();
       if (d)